The ref-filter API 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. This impacts the output git branch, git tag and git for-each-refwhen used with a --format argument containing the atoms%(contents:subject) or %(contents:body), as well as the output ofgit branch --verbose, which uses %(contents:subject) internally. The function find_subpos in ref-filter.c looks for two consecutive '\n' 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, '\n' is replaced by space, but '\r' is untouched, resulting in the escape sequence '^M' being output verbatim in most terminal emulators: $ git branch --verbose * crlf 2113b0e Subject first line^M ^M Body first line^M Body second line This bug is a regression for git branch --verbose, which bisects down to 949af06 (branch: use ref-filter printing APIs, 2017-01-10). The first patch in this series adds a new test library, t/lib-crlf-messages.sh, that contains functions to test this behaviour for commands using either the ref-filter or the pretty API to display messages. The second patch fixes the bug in ref-filter.c and adds corresponding tests. Finally, the third patch adds a test that checks the behaviour of git log in the presence of CRLF in messages, to prevent futur regressions. changes since v1: * improved the cover letter and the commit message of the 2nd patch to actually describe the bug this series is fixing ---------------------------------------------------------------------------- v1: The ref-filter API 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. This impacts the output of git branch -v, and git branch, git tag and git for-each-ref when used with a --format argument containing the atoms %(contents:subject) or %(contents:body). The first patch in this series adds a new test library, t/lib-crlf-messages.sh, that contains functions to test this behaviour for commands using either the ref-filter or the pretty API to display messages. The second patch fixes the bug in ref-filter.c and adds corresponding tests. Finally, the third patch adds a test that checks the behaviour of git log in the presence of CRLF in messages, to prevent futur regressions. Philippe Blain (3): t: add lib-crlf-messages.sh for messages containing CRLF ref-filter: fix the API to correctly handle CRLF log: add tests for messages containing CRLF to t4202 ref-filter.c | 19 +++++++++-- t/lib-crlf-messages.sh | 73 ++++++++++++++++++++++++++++++++++++++++ t/t3203-branch-output.sh | 26 +++++++++++--- t/t4202-log.sh | 24 +++++++++++++ t/t6300-for-each-ref.sh | 5 +++ t/t7004-tag.sh | 7 ++++ 6 files changed, 147 insertions(+), 7 deletions(-) create mode 100644 t/lib-crlf-messages.sh base-commit: 076cbdcd739aeb33c1be87b73aebae5e43d7bcc5 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-576%2Fphil-blain%2Ffix-branch-verbose-crlf-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-576/phil-blain/fix-branch-verbose-crlf-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/576 Range-diff vs v1: 1: b2521138035 = 1: b2521138035 t: add lib-crlf-messages.sh for messages containing CRLF 2: c68bc2b3788 ! 2: aab1f45ba97 ref-filter: teach the API to correctly handle CRLF @@ -1,26 +1,49 @@ Author: Philippe Blain <levraiphilippeblain@xxxxxxxxx> - ref-filter: teach the API to correctly handle CRLF + ref-filter: fix the API to correctly handle CRLF The ref-filter API 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. - This impacts the output of `git branch -v`, and `git branch`, `git - tag` and `git for-each-ref` when used with a `--format` argument - containing the atoms `%(contents:subject)` or `%(contents:body)`. + This impacts the output `git branch`, `git tag` and `git for-each-ref` + when used with a `--format` argument containing the atoms + `%(contents:subject)` or `%(contents:body)`, as well as the output of + `git branch --verbose`, which uses `%(contents:subject)` internally. - Fix this bug in ref-filter by improving the logic in `copy_subject` and - `find_subpos`. + The function find_subpos in ref-filter.c looks for two consecutive '\n' + 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, '\n' is replaced by space, but '\r' is + untouched, resulting in the escape sequence '^M' being output verbatim + in most terminal emulators: + + $ git branch --verbose + * crlf 2113b0e Subject first line^M ^M Body first line^M Body second line + + 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 CRFL. 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 needs to be modified since it did not use - `test_tick` and thus the commit hashes were not reproducible. For - simplicity, use `test_commit` as the content and name of the files - created in this setup test are irrelevant to the rest of the test - script. + lib-crlf-messages.sh. + + The 'make commits' test at the beginning of t3203-branch-output.sh needs + to be modified since it did not use `test_tick` and thus the commit + hashes were not reproducible. For simplicity, use `test_commit` as the + content and name of the files created in this setup test are irrelevant + to the rest of the test script. + + `test_cleanup_crlf_refs` is used in t3203-branch-output.sh and + t7004-tag.sh to avoid having to modify the expected output in later + tests. Signed-off-by: Philippe Blain <levraiphilippeblain@xxxxxxxxx> 3: 3e5b8ace7b8 = 3: c84092e457c log: add tests for messages containing CRLF to t4202 -- gitgitgadget
