Re: [PATCH 6/6] ssh signing: fmt-merge-msg/check_signature with tag date

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Oct 22, 2021, 18:12 UTC
Message-ID: <211022.86a6j0kjoi.gmgdl@evledraar.gmail.com>
In-Reply-To: <20211022150949.1754477-7-fs@gigacodes.de>

On Fri, Oct 22 2021, Fabian Stelzer wrote:

Show 15 quoted lines

>  			buf = payload.buf;
>  			len = payload.len;
> -			if (check_signature(payload.buf, payload.len, 0, NULL,
> +
> +			if (parse_signed_buffer_metadata(payload.buf, "tagger",
> +							 &payload_timestamp,
> +							 &payload_signer))
> +				strbuf_addstr(&sig,
> +					_("failed to parse timestamp and signer info from payload"));
> +
> +			if (check_signature(payload.buf, payload.len,
> +					    payload_timestamp, &payload_signer,
>  					    sig.buf, sig.len, &sigc) &&
>  			    !sigc.output)
>  				strbuf_addstr(&sig, "gpg verification failed.\n");

I haven't tested this, but your addition to &sig here lacks a \n, compared to the &sig seen right above here in the diff context.

Isn't one or the other either missing a \n, or shouldn't have one? *Looks ath the surrounding code*, yeah if I'm not wrong it's the \n in the new code here that's missing.

The whole business of seemingly mixing error messages and a signature payload in the same variable seems a bit odd, but maybe I'm misreading it. In any case it seems to pre-date this series...