Re* [IGNORETHIS/PATCH] Choosing the sha1 prefix of your commits
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 20, 2011, 18:36 UTC
- Message-ID
- <7vehy7a4sf.fsf_-_@alter.siamese.dyndns.org>
- In-Reply-To
- <20111020071356.GA14945@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 9 quoted lines
> It's not that the commit is bad or the source of problems. My point is > that the assumption that commit messages are NUL-terminated has been > there for a really long time, so there are lots of spots in the code > that sloppily run string functions on them. Every one of those needs to > be found and fixed (e.g., I remember seeing this in > for-each-ref.c:find_subpos recently). > > It's not impossible, of course, or even really that hard. It's just a > giant pain, and I wonder if the effort is worth it.
True.
It probably is not worth it for most applications, but this fix-up to a fairly recent one is worth doing, I would suspect.
-- >8 -- Subject: parse_signed_commit: really use the entire commit log message
... even beyond the first NUL in the buffer, when checking the commit against the detached signature in the header.
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- commit.c | 11 +++++------ t/t7510-signed-commit.sh | 21 ++++++++++++++++----- 2 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/commit.c b/commit.c index 93045a2..6ec49fa 100644 --- a/commit.c +++ b/commit.c @@ -854,28 +854,27 @@ int parse_signed_commit(const unsigned char *sha1, unsigned long size; enum object_type type; char *buffer = read_sha1_file(sha1, &type, &size); - int in_header, saw_signature = -1; + int saw_signature = -1; char *line; if (!buffer || type != OBJ_COMMIT) goto cleanup; line = buffer; - in_header = 1; saw_signature = 0; - while (*line) { + while (line < buffer + size) { char *next = strchrnul(line, '\n'); if (*next) next++; - if (in_header && !prefixcmp(line, gpg_sig_header)) { + if (!prefixcmp(line, gpg_sig_header)) { const char *sig = line + gpg_sig_header_len; strbuf_add(signature, sig, next - sig); saw_signature = 1; } else { + if (*line == '\n') + next = buffer + size; /* dump the whole remainder */ strbuf_add(payload, line, next - line); } - if (*line == '\n') - in_header = 0; line = next; } cleanup: diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh index 5c7475d..30401ce 100755 --- a/t/t7510-signed-commit.sh +++ b/t/t7510-signed-commit.sh @@ -50,11 +50,22 @@ test_expect_success GPG 'show signatures' ' test_expect_success GPG 'detect fudged signature' ' git cat-file commit master >raw && - sed -e "s/fourth signed/4th forged/" raw >forged && - git hash-object -w -t commit forged >forged.commit && - git show --pretty=short --show-signature $(cat forged.commit) >actual && - grep "BAD signature from" actual && - ! grep "Good signature from" actual + + sed -e "s/fourth signed/4th forged/" raw >forged1 && + git hash-object -w -t commit forged1 >forged1.commit && + git show --pretty=short --show-signature $(cat forged1.commit) >actual1 && + grep "BAD signature from" actual1 && + ! grep "Good signature from" actual1 +' + +test_expect_success GPG 'detect fudged signature with NUL' ' + git cat-file commit master >raw && + cat raw >forged2 && + echo Qwik | tr "Q" "\000" >>forged2 && + git hash-object -w -t commit forged2 >forged2.commit && + git show --pretty=short --show-signature $(cat forged2.commit) >actual2 && + grep "BAD signature from" actual2 && + ! grep "Good signature from" actual2 ' test_done