[PATCH v2 0/3] Teach ref-filter API to correctly handle CRLF in messages

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@gmail.com>
      
     -    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@gmail.com>
      
 3:  3e5b8ace7b8 = 3:  c84092e457c log: add tests for messages containing CRLF to t4202