mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <>
To: Philippe Blain <>
Cc: Martin Englund <>,
Subject: [PATCH 2/2] ref-filter: fix parsing of signatures with CRLF and no body
Date: Wed, 2 Nov 2022 03:44:00 -0400	[thread overview]
Message-ID: <Y2IfwL96Ku/> (raw)
In-Reply-To: <>

This commit fixes a bug when parsing tags that have CRLF line endings, a
signature, and no body, like this (the "^M" are marking the CRs):

  this is the subject^M
  ...some stuff...^M

When trying to find the start of the body, we look for a blank line
separating the subject and body. In this case, there isn't one. But we
search for it using strstr(), which will find the blank line in the

In the non-CRLF code path, we check whether the line we found is past
the start of the signature, and if so, put the body pointer at the start
of the signature (effectively making the body empty). But the CRLF code
path doesn't catch the same case, and we end up with the body pointer in
the middle of the signature field. This has two visible problems:

  - printing %(contents:subject) will show part of the signature, too,
    since the subject length is computed as (body - subject)

  - the length of the body is (sig - body), which makes it negative.
    Asking for %(contents:body) causes us to cast this to a very large
    size_t when we feed it to xmemdupz(), which then complains about
    trying to allocate too much memory.

These are essentially the same bugs fixed in the previous commit, except
that they happen when there is a CRLF blank line in the signature,
rather than no blank line at all. Both are caused by the refactoring in
9f75ce3d8f (ref-filter: handle CRLF at end-of-line more gracefully,

We can fix this by doing the same "sigstart" check that we do in the
non-CRLF case. And rather than repeat ourselves, we can just use
short-circuiting OR to collapse both cases into a single conditional.
I.e., rather than:

  if (strstr("\n\n"))
    ...found blank, check if it's in signature...
  else if (strstr("\r\n\r\n"))
    ...found blank, check if it's in signature...
  else blank line found...

we can collapse this to:

  if (strstr("\n\n")) ||
    ...found blank, check if it's in signature...
  else blank line found...

The tests show the problem and the fix. Though it wasn't broken, I
included contents:signature here to make sure it still behaves as
expected, but note the shell hackery needed to make it work. A
less-clever option would be to skip using test_atom and just "append_cr
>expected" ourselves.

Signed-off-by: Jeff King <>
 ref-filter.c            |  6 +++---
 t/ | 25 +++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 6c2148c01e..9dc2cd1451 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1375,10 +1375,10 @@ static void find_subpos(const char *buf,
 	/* subject is first non-empty line */
 	*sub = buf;
 	/* subject goes to first empty line before signature begins */
-	if ((eol = strstr(*sub, "\n\n"))) {
+	if ((eol = strstr(*sub, "\n\n")) ||
+	    (eol = strstr(*sub, "\r\n\r\n"))) {
 		eol = eol < sigstart ? eol : sigstart;
-	/* check if message uses CRLF */
-	} else if (! (eol = strstr(*sub, "\r\n\r\n"))) {
+	} else {
 		/* treat whole message as subject */
 		eol = sigstart;
diff --git a/t/ b/t/
index d7e70027e6..fa38b87441 100755
--- a/t/
+++ b/t/
@@ -1421,4 +1421,29 @@ test_atom refs/tags/fake-sig-no-blanks contents:subject 'this is the subject'
 test_atom refs/tags/fake-sig-no-blanks contents:body ''
 test_atom refs/tags/fake-sig-no-blanks contents:signature "$sig"
+test_expect_success 'set up tag with CRLF signature' '
+	append_cr <<-\EOF |
+	this is the subject
+	not a real signature, but we just care about
+	the subject/body parsing. It is important here
+	that there is a blank line separating this
+	from the signature header.
+	git tag -F - --cleanup=verbatim fake-sig-crlf
+test_atom refs/tags/fake-sig-crlf contents:subject 'this is the subject'
+test_atom refs/tags/fake-sig-crlf contents:body ''
+# CRLF is retained in the signature, so we have to pass our expected value
+# through append_cr. But test_atom requires a shell string, which means command
+# substitution, and the shell will strip trailing newlines from the output of
+# the substitution. Hack around it by adding and then removing a dummy line.
+sig_crlf="$(printf "%s" "$sig" | append_cr; echo dummy)"
+test_atom refs/tags/fake-sig-crlf contents:signature "$sig_crlf"

  parent reply	other threads:[~2022-11-02  7:44 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-28 22:29 Git Bug Report: out of memory using git tag Martin Englund
2022-11-01 12:22 ` Jeff King
2022-11-02  0:41   ` Philippe Blain
2022-11-02  7:39     ` Jeff King
2022-11-02  7:42       ` [PATCH 1/2] ref-filter: fix parsing of signatures without blank lines Jeff King
2022-11-02  7:44       ` Jeff King [this message]
2022-11-02  8:14       ` Git Bug Report: out of memory using git tag Elijah Newren
2022-11-02  9:13         ` gigantic commit messages, was " Jeff King
2022-11-02 14:26           ` Ævar Arnfjörð Bjarmason
2022-11-02 15:43             ` Elijah Newren
2022-11-02  8:24       ` Eric Sunshine
2022-11-02 12:13       ` Philippe Blain
2022-11-03  4:32         ` Jeff King
2022-11-03  0:42       ` Taylor Blau
2022-11-02  0:42   ` Philippe Blain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y2IfwL96Ku/ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).