git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 2/3] ref-filter: handle CRLF at end-of-line more gracefully

From
Philippe Blain via GitGitGadget <gitgitgadget@gmail.com>
Date
Oct 12, 2020, 18:09 UTC
Message-ID
<11d044a4f7feccdf20da6364a1f9bbe934e9981f.1602526169.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.576.v3.git.1602526169.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>

The ref-filter code does not correctly handle commit or tag messages that use CRLF as the line terminator. Such messages can be created with the `--verbatim` option of `git commit` and `git tag`, or by using `git commit-tree` directly.

The function `find_subpos` in ref-filter.c looks for two consecutive LFs to find the end of the subject line, a sequence which is absent in messages using CRLF. This results in the whole message being parsed as the subject line (`%(contents:subject)`), and the body of the message (`%(contents:body)`) being empty.

Moreover, in `copy_subject`, which wants to return the subject as a single line, '\n' is replaced by space, but '\r' is untouched.

This impacts the output of `git branch`, `git tag` and `git for-each-ref`.

This bug is a regression for `git branch --verbose`, which bisects down to 949af0684c (branch: use ref-filter printing APIs, 2017-01-10).

Fix this bug in ref-filter by hardening the logic in `copy_subject` and `find_subpos` to correctly parse messages containing CRLF.

Add tests for `branch`, `tag` and `for-each-ref` using lib-crlf-messages.sh.

The 'make commits' test at the beginning of t3203-branch-output.sh does not use `test_tick` and thus the commit hashes are not reproducible. For simplicity, use `test_commit` to create the commits, as the content and name of the files created in this setup test are irrelevant to the rest of the test script.

Use `test_cleanup_crlf_refs` in t3203-branch-output.sh and t7004-tag.sh to avoid having to modify the expected output in later tests.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
 ref-filter.c             | 36 +++++++++++++++++++++---------------
 t/t3203-branch-output.sh | 31 ++++++++++++++++++++++++++-----
 t/t6300-for-each-ref.sh  |  5 +++++
 t/t7004-tag.sh           |  7 +++++++
 4 files changed, 59 insertions(+), 20 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index c62f6b4822..92d8ca5340 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1097,14 +1097,18 @@ static const char *copy_email(const char *buf, struct used_atom *atom)
 
 static char *copy_subject(const char *buf, unsigned long len)
 {
-	char *r = xmemdupz(buf, len);
-	int i;
+	struct strbuf sb = STRBUF_INIT;
 
-	for (i = 0; i < len; i++)
-		if (r[i] == '\n')
-			r[i] = ' ';
+	for (int i = 0; i < len; i++) {
+		if (buf[i] == '\r' && i + 1 < len && buf[i + 1] == '\n')
+			continue; /* ignore CR in CRLF */
 
-	return r;
+		if (buf[i] == '\n')
+			strbuf_addch(&sb, ' ');
+		else
+			strbuf_addch(&sb, buf[i]);
+	}
+	return strbuf_detach(&sb, NULL);
 }
 
 static void grab_date(const char *buf, struct atom_value *v, const char *atomname)
@@ -1228,20 +1232,22 @@ static void find_subpos(const char *buf,
 
 	/* subject is first non-empty line */
 	*sub = buf;
-	/* subject goes to first empty line */
-	while (buf < *sig && *buf && *buf != '\n') {
-		eol = strchrnul(buf, '\n');
-		if (*eol)
-			eol++;
-		buf = eol;
-	}
+	/* subject goes to first empty line before signature begins */
+	if ((eol = strstr(*sub, "\n\n"))) {
+		eol = eol < *sig ? eol : *sig;
+	/* check if message uses CRLF */
+	} else if (! (eol = strstr(*sub, "\r\n\r\n"))) {
+		/* treat whole message as subject */
+		eol = strrchr(*sub, '\0');
+	}
+	buf = eol;
 	*sublen = buf - *sub;
 	/* drop trailing newline, if present */
-	if (*sublen && (*sub)[*sublen - 1] == '\n')
+	while (*sublen && ((*sub)[*sublen - 1] == '\n' || (*sub)[*sublen - 1] == '\r'))
 		*sublen -= 1;
 
 	/* skip any empty lines */
-	while (*buf == '\n')
+	while (*buf == '\n' || *buf == '\r')
 		buf++;
 	*body = buf;
 	*bodylen = strlen(buf);
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 71818b90f0..c06eca774f 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -3,13 +3,11 @@
 test_description='git branch display tests'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-terminal.sh
+. "$TEST_DIRECTORY"/lib-crlf-messages.sh
 
 test_expect_success 'make commits' '
-	echo content >file &&
-	git add file &&
-	git commit -m one &&
-	echo content >>file &&
-	git commit -a -m two
+	test_commit one &&
+	test_commit two
 '
 
 test_expect_success 'make branches' '
@@ -95,6 +93,29 @@ test_expect_success 'git branch --ignore-case --list -v pattern shows branch sum
 	awk "{print \$NF}" <tmp >actual &&
 	test_cmp expect actual
 '
+test_create_crlf_refs
+
+test_expect_success 'git branch -v works with CRLF input' '
+	cat >expect <<-EOF &&
+	  two
+	  one
+	  Subject first line
+	  Subject first line
+	  Subject first line Subject second line
+	  Subject first line Subject second line
+	  Subject first line Subject second line
+	  Subject first line Subject second line
+	EOF
+	git branch -v >tmp &&
+	# Remove first two columns, and the line for the currently checked out branch
+	current=$(git branch --show-current) &&
+	grep -v $current <tmp | awk "{\$1=\$2=\"\"}1"  >actual &&
+	test_cmp expect actual
+'
+
+test_crlf_subject_body_and_contents branch --list crlf*
+
+test_cleanup_crlf_refs
 
 test_expect_success 'git branch -v pattern does not show branch summaries' '
 	test_must_fail git branch -v branch*
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index b359023189..c30940cf7a 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -8,6 +8,7 @@ test_description='for-each-ref test'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-gpg.sh
 . "$TEST_DIRECTORY"/lib-terminal.sh
+. "$TEST_DIRECTORY"/lib-crlf-messages.sh
 
 # Mon Jul 3 23:18:43 2006 +0000
 datestamp=1151968723
@@ -1030,4 +1031,8 @@ test_expect_success 'for-each-ref --ignore-case works on multiple sort keys' '
 	test_cmp expect actual
 '
 
+test_create_crlf_refs
+
+test_crlf_subject_body_and_contents for-each-ref refs/heads/crlf*
+
 test_done
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 05f411c821..cda735dab4 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -10,6 +10,7 @@ Tests for operations with tags.'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-gpg.sh
 . "$TEST_DIRECTORY"/lib-terminal.sh
+. "$TEST_DIRECTORY"/lib-crlf-messages.sh
 
 # creating and listing lightweight tags:
 
@@ -1970,6 +1971,12 @@ test_expect_success '--format should list tags as per format given' '
 	test_cmp expect actual
 '
 
+test_create_crlf_refs
+
+test_crlf_subject_body_and_contents tag --list tag-crlf*
+
+test_cleanup_crlf_refs
+
 test_expect_success "set up color tests" '
 	echo "<RED>v1.0<RESET>" >expect.color &&
 	echo "v1.0" >expect.bare &&

-- 
gitgitgadget
Previous: Philippe Blain via GitGitGadgetNext: Junio C Hamano
Message 15 of 38 in “Teach ref-filter API to correctly handle CRLF in messages”
  1. 0/3 Teach ref-filter API to correctly handle CRLF in messagesPhilippe Blain via GitGitGadget, Mar 8, 2020
  2. 3/3 log: add tests for messages containing CRLF to t4202Philippe Blain via GitGitGadget, Mar 8, 2020
  3. 1/3 t: add lib-crlf-messages.sh for messages containing CRLFPhilippe Blain via GitGitGadget, Mar 8, 2020
  4. 2/3 ref-filter: teach the API to correctly handle CRLFPhilippe Blain via GitGitGadget, Mar 8, 2020
  5. Junio C HamanoMar 9, 2020
  6. Philippe BlainMar 10, 2020
  7. 0/3 Teach ref-filter API to correctly handle CRLF in messagesPhilippe Blain via GitGitGadget, Mar 10, 2020
  8. 1/3 t: add lib-crlf-messages.sh for messages containing CRLFPhilippe Blain via GitGitGadget, Mar 10, 2020
  9. 2/3 ref-filter: fix the API to correctly handle CRLFPhilippe Blain via GitGitGadget, Mar 10, 2020
  10. Junio C HamanoMar 10, 2020
  11. 3/3 log: add tests for messages containing CRLF to t4202Philippe Blain via GitGitGadget, Mar 10, 2020
  12. Junio C HamanoMar 10, 2020
  13. Junio C HamanoMar 10, 2020
  14. 0/3 ref-filter: handle CRLF at end-of-line more gracefullyPhilippe Blain via GitGitGadget, Oct 12, 2020
  15. 2/3 ref-filter: handle CRLF at end-of-line more gracefullyPhilippe Blain via GitGitGadget, Oct 12, 2020
  16. Junio C HamanoOct 12, 2020
  17. Philippe BlainOct 14, 2020
  18. 1/3 t: add lib-crlf-messages.sh for messages containing CRLFPhilippe Blain via GitGitGadget, Oct 12, 2020
  19. Junio C HamanoOct 12, 2020
  20. Philippe BlainOct 14, 2020
  21. Eric SunshineOct 12, 2020
  22. Philippe BlainOct 14, 2020
  23. Eric SunshineOct 14, 2020
  24. Philippe BlainOct 14, 2020
  25. Eric SunshineOct 14, 2020
  26. Philippe BlainOct 22, 2020
  27. 3/3 log, show: add tests for messages containing CRLFPhilippe Blain via GitGitGadget, Oct 12, 2020
  28. 0/2 ref-filter: handle CRLF at end-of-line more gracefullyPhilippe Blain via GitGitGadget, Oct 22, 2020
  29. 2/2 log, show: add tests for messages containing CRLFPhilippe Blain via GitGitGadget, Oct 22, 2020
  30. Philippe BlainOct 22, 2020
  31. 1/2 ref-filter: handle CRLF at end-of-line more gracefullyPhilippe Blain via GitGitGadget, Oct 22, 2020
  32. Junio C HamanoOct 23, 2020
  33. Philippe BlainOct 23, 2020
  34. Junio C HamanoOct 28, 2020
  35. Philippe BlainOct 29, 2020
  36. 0/2 ref-filter: handle CRLF at end-of-line more gracefullyPhilippe Blain via GitGitGadget, Oct 29, 2020
  37. 1/2 ref-filter: handle CRLF at end-of-line more gracefullyPhilippe Blain via GitGitGadget, Oct 29, 2020
  38. 2/2 log, show: add tests for messages containing CRLFPhilippe Blain via GitGitGadget, Oct 29, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.