Re: Signed commit regression?
On Fri, Feb 28 2020, Junio C Hamano wrote:
> I'd expect that there may be another round of attempt to update the
> GPG interface. Let's make sure we won't lose info given to the
> end-users while doing so.
The regression was introduced by this botched chunk in 72b006f4bf:
@@ -521,18 +522,19 @@ static int show_one_mergetag(struct commit *commit,
gpg_message_offset = verify_message.len;
payload_size = parse_signature(extra->value, extra->len);
status = -1;
if (extra->len > payload_size) {
/* could have a good signature */
- if (!verify_signed_buffer(extra->value, payload_size,
- extra->value + payload_size,
- extra->len - payload_size,
- &verify_message, NULL))
+ if (!check_signature(extra->value, payload_size,
+ extra->value + payload_size,
+ extra->len - payload_size, &sigc)) {
+ strbuf_addstr(&verify_message, sigc.gpg_output);
+ signature_check_clear(&sigc);
status = 0; /* good */
- else if (verify_message.len <= gpg_message_offset)
+ } else if (verify_message.len <= gpg_message_offset)
strbuf_addstr(&verify_message, "No signature\n");
/* otherwise we couldn't verify, which is shown as bad */
}
There are two ridiculous bugs in my original patch:
1. The output from GPG is only added to `verify_message' if a signature
verifies successfully.
2. On verification failure, the "No signature" message is always added
to `verify_message'. This is because, again, no output from GPG is
added to `verify_message' on failure, so its length will always equal
`gpg_message_offset' (see the initial assignment) when
`check_signature()' returns non-zero, sigh...
Not sure of the proper way of fixing a reverted commit, but I'll send v1
based on pu that includes regression tests.
I'm sorry for my fuckup and the headache it caused!
--
hji
Hans Jerry Illikainen <hji@dyntopia.com>