On Mon, Apr 9, 2018 at 4:41 PM, Ben Toews <mastahyeti@xxxxxxxxx> wrote: > From: Jeff King <peff@xxxxxxxx> > > A signed tag has a detached signature like this: > > object ... > [...more header...] > > This is the tag body. > > -----BEGIN PGP SIGNATURE----- > [opaque gpg data] > -----END PGP SIGNATURE----- > > Our parser finds the _first_ line that appears to start a > PGP signature block, meaning we may be confused by a > signature (or a signature-like line) in the actual body. > Let's keep parsing and always find the final block, which > should be the detached signature over all of the preceding > content. > --- > diff --git a/gpg-interface.c b/gpg-interface.c > @@ -110,11 +110,17 @@ static int is_gpg_start(const char *line) > size_t parse_signature(const char *buf, size_t size) > { > size_t len = 0; > - while (len < size && !is_gpg_start(buf + len)) { > - const char *eol = memchr(buf + len, '\n', size - len); > + size_t match = size; If no GPG-start line is found then 'size' will be returned, which matches the logic before this change. Okay. > + while (len < size) { > + const char *eol; > + > + if (is_gpg_start(buf + len)) > + match = len; Otherwise, the position of the final GPG-start line will be remembered and returned. Makes sense. > + eol = memchr(buf + len, '\n', size - len); > len += eol ? eol - (buf + len) + 1 : size - len; > } > - return len; > + return match; > } > diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh > @@ -1059,6 +1059,17 @@ test_expect_success GPG \ > +test_expect_success GPG 'signed tag with embedded PGP message' ' > + cat >msg <<-\EOF && > + -----BEGIN PGP MESSAGE----- > + > + this is not a real PGP message > + -----END PGP MESSAGE----- > + EOF This bogus PGP message is just in the body... > + git tag -s -F msg confusing-pgp-message && and "git tag -s" adds the real PGP message at the end... > + git tag -v confusing-pgp-message and the new logic finds the real PGP message rather than the bogus one, so "git tag -v" exits successfully. Looks good. > +'