@matheusbsbs / matheus-bagliano / commits / 9f75ce3d8f

ref-filter: handle CRLF at end-of-line more gracefully

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 `--cleanup=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 behaviour is a regression for `git branch --verbose`, which bisects down to 949af0684c (branch: use ref-filter printing APIs, 2017-01-10). Adjust the ref-filter code to be more lenient by hardening the logic in `copy_subject` and `find_subpos` to correctly parse messages containing CRLF. Add a new test script, 't3920-crlf-messages.sh', to test the behaviour of commands using either the ref-filter or the pretty APIs with messages using CRLF line endings. The function `test_crlf_subject_body_and_contents` can be used to test that the `--format` option of `branch`, `tag`, `for-each-ref`, `log` and `show` correctly displays the subject, body and raw content of commit and tag messages using CRLF. Test the output of `branch`, `tag` and `for-each-ref` with such commits. Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Philippe Blain committed Oct 29, 2020 at 12:48 UTC 9f75ce3d8f2666ea82913a88d7068f28893b23a8
2 files changed +130 -14
ref-filter.c
+22 -14
@@ -1097,14 +1097,19 @@ static const char *copy_email(const char *buf, struct used_atom *atom)
1097
1098 static char *copy_subject(const char *buf, unsigned long len)
1099 {
1100 - char *r = xmemdupz(buf, len);
1100 + struct strbuf sb = STRBUF_INIT;
1101 int i;
1102
1103 - for (i = 0; i < len; i++)
1104 - if (r[i] == '\n')
1105 - r[i] = ' ';
1103 + for (i = 0; i < len; i++) {
1104 + if (buf[i] == '\r' && i + 1 < len && buf[i + 1] == '\n')
1105 + continue; /* ignore CR in CRLF */
1106
1107 - return r;
1107 + if (buf[i] == '\n')
1108 + strbuf_addch(&sb, ' ');
1109 + else
1110 + strbuf_addch(&sb, buf[i]);
1111 + }
1112 + return strbuf_detach(&sb, NULL);
1113 }
1114
1115 static void grab_date(const char *buf, struct atom_value *v, const char *atomname)
@@ -1228,20 +1233,23 @@ static void find_subpos(const char *buf,
1233
1234 /* subject is first non-empty line */
1235 *sub = buf;
1231 - /* subject goes to first empty line */
1232 - while (buf < *sig && *buf && *buf != '\n') {
1233 - eol = strchrnul(buf, '\n');
1234 - if (*eol)
1235 - eol++;
1236 - buf = eol;
1237 - }
1236 + /* subject goes to first empty line before signature begins */
1237 + if ((eol = strstr(*sub, "\n\n"))) {
1238 + eol = eol < *sig ? eol : *sig;
1239 + /* check if message uses CRLF */
1240 + } else if (! (eol = strstr(*sub, "\r\n\r\n"))) {
1241 + /* treat whole message as subject */
1242 + eol = strrchr(*sub, '\0');
1243 + }
1244 + buf = eol;
1245 *sublen = buf - *sub;
1246 /* drop trailing newline, if present */
1240 - if (*sublen && (*sub)[*sublen - 1] == '\n')
1247 + while (*sublen && ((*sub)[*sublen - 1] == '\n' ||
1248 + (*sub)[*sublen - 1] == '\r'))
1249 *sublen -= 1;
1250
1251 /* skip any empty lines */
1244 - while (*buf == '\n')
1252 + while (*buf == '\n' || *buf == '\r')
1253 buf++;
1254 *body = buf;
1255 *bodylen = strlen(buf);
t/t3920-crlf-messages.sh new
+108
@@ -0,0 +1,108 @@
1 +#!/bin/sh
2 +
3 +test_description='Test ref-filter and pretty APIs for commit and tag messages using CRLF'
4 +. ./test-lib.sh
5 +
6 +LIB_CRLF_BRANCHES=""
7 +
8 +create_crlf_ref () {
9 + branch="$1" &&
10 + cat >.crlf-orig-$branch.txt &&
11 + cat .crlf-orig-$branch.txt | append_cr >.crlf-message-$branch.txt &&
12 + grep 'Subject' .crlf-orig-$branch.txt | tr '\n' ' ' | sed 's/[ ]*$//' | tr -d '\n' >.crlf-subject-$branch.txt &&
13 + grep 'Body' .crlf-message-$branch.txt >.crlf-body-$branch.txt || true &&
14 + LIB_CRLF_BRANCHES="${LIB_CRLF_BRANCHES} ${branch}" &&
15 + test_tick &&
16 + hash=$(git commit-tree HEAD^{tree} -p HEAD -F .crlf-message-${branch}.txt) &&
17 + git branch ${branch} ${hash} &&
18 + git tag tag-${branch} ${branch} -F .crlf-message-${branch}.txt --cleanup=verbatim
19 +}
20 +
21 +create_crlf_refs () {
22 + create_crlf_ref crlf <<-\EOF &&
23 + Subject first line
24 +
25 + Body first line
26 + Body second line
27 + EOF
28 + create_crlf_ref crlf-empty-lines-after-subject <<-\EOF &&
29 + Subject first line
30 +
31 +
32 + Body first line
33 + Body second line
34 + EOF
35 + create_crlf_ref crlf-two-line-subject <<-\EOF &&
36 + Subject first line
37 + Subject second line
38 +
39 + Body first line
40 + Body second line
41 + EOF
42 + create_crlf_ref crlf-two-line-subject-no-body <<-\EOF &&
43 + Subject first line
44 + Subject second line
45 + EOF
46 + create_crlf_ref crlf-two-line-subject-no-body-trailing-newline <<-\EOF
47 + Subject first line
48 + Subject second line
49 +
50 + EOF
51 +}
52 +
53 +test_crlf_subject_body_and_contents() {
54 + command_and_args="$@" &&
55 + command=$1 &&
56 + if test ${command} = "branch" || test ${command} = "for-each-ref" || test ${command} = "tag"
57 + then
58 + atoms="(contents:subject) (contents:body) (contents)"
59 + elif test ${command} = "log" || test ${command} = "show"
60 + then
61 + atoms="s b B"
62 + fi &&
63 + files="subject body message" &&
64 + while test -n "${atoms}"
65 + do
66 + set ${atoms} && atom=$1 && shift && atoms="$*" &&
67 + set ${files} && file=$1 && shift && files="$*" &&
68 + test_expect_success "${command}: --format='%${atom}' works with messages using CRLF" "
69 + rm -f expect &&
70 + for ref in ${LIB_CRLF_BRANCHES}
71 + do
72 + cat .crlf-${file}-\"\${ref}\".txt >>expect &&
73 + printf \"\n\" >>expect
74 + done &&
75 + git $command_and_args --format=\"%${atom}\" >actual &&
76 + test_cmp expect actual
77 + "
78 + done
79 +}
80 +
81 +
82 +test_expect_success 'Setup refs with commit and tag messages using CRLF' '
83 + test_commit inital &&
84 + create_crlf_refs
85 +'
86 +
87 +test_expect_success 'branch: --verbose works with messages using CRLF' '
88 + rm -f expect &&
89 + for branch in $LIB_CRLF_BRANCHES
90 + do
91 + printf " " >>expect &&
92 + cat .crlf-subject-${branch}.txt >>expect &&
93 + printf "\n" >>expect
94 + done &&
95 + git branch -v >tmp &&
96 + # Remove first two columns, and the line for the currently checked out branch
97 + current=$(git branch --show-current) &&
98 + grep -v $current <tmp | awk "{\$1=\$2=\"\"}1" >actual &&
99 + test_cmp expect actual
100 +'
101 +
102 +test_crlf_subject_body_and_contents branch --list crlf*
103 +
104 +test_crlf_subject_body_and_contents tag --list tag-crlf*
105 +
106 +test_crlf_subject_body_and_contents for-each-ref refs/heads/crlf*
107 +
108 +test_done