Skip to content

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

Philippe Blain via GitGitGadget git 5 versions – New

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v5 Latest2 patches 0 repliesNew
  2. v42 patches 5 repliesSuperseded
  3. v33 patches 10 repliesSuperseded
Earlier versions · 2
VersionPostedPatchesRepliesStatus
v23 patches3 repliesSuperseded
v13 patches2 repliesSuperseded

Changes in v5 from the cover letter

* tweaked the wording of both commit messages to not call the behaviour a bug
* added || return 1 in one test - that had already been squashed in by Junio

Patches in v5 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/2ref-filter: handle CRLF at end-of-line more gracefullyref-filter.c t/t3920-crlf-messages.sh+130 -140 replies
2/2log, show: add tests for messages containing CRLFt/t3920-crlf-messages.sh+18 -00 replies

Reviews trailers per patch

None of the 2 patches has a Reviewed-by or Acked-by.

PatchJunio C HamanoEric Sunshine
1/2Helped-by Helped-by
2/2

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v5 thread

Cover letter [PATCH v5 0/2] ref-filter: handle CRLF at end-of-line more gracefully

Changes since v4 :

* tweaked the wording of both commit messages to not call the behaviour a bug
* added || return 1 in one test - that had already been squashed in by Junio

----------------------------------------------------------------------------

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 '\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.

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

The first patch hardens the code in ref-filter.c to handle these messages more gracefully and adds a test script to check that the ref-filter and pretty APIs deal correctly with CRLF messages.

The second patch adds tests that check the behaviour of git log andgit show in the presence of CRLF in messages, to prevent futur regressions.

Cc: Michael J Gruber [email protected] [[email protected]], Matthieu Moy [email protected] [[email protected]], John Keeping [email protected] [[email protected]], Karthik Nayak [email protected] [[email protected]], Jeff King [email protected] [[email protected]], Alex Henrie [email protected] [[email protected]]cc: Eric Sunshine [email protected] [[email protected]]cc: Junio C Hamano [email protected] [[email protected]]

Philippe Blain (2):
ref-filter: handle CRLF at end-of-line more gracefully log, show: add tests for messages containing CRLF

 ref-filter.c             |  36 ++++++-----
 t/t3920-crlf-messages.sh | 126 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 148 insertions(+), 14 deletions(-)
 create mode 100755 t/t3920-crlf-messages.sh

base-commit: a5fa49ff0a8f3252c6bff49f92b85e7683868f8a
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-576%2Fphil-blain%2Ffix-branch-verbose-crlf-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-576/phil-blain/fix-branch-verbose-crlf-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/576

Range-diff vs v4:

1: 03b2d7d78a ! 1: 06231c315f ref-filter: handle CRLF at end-of-line more gracefully
@@ Commit message
This impacts the output of git branch, git tag and git for-each-ref.

- This bug is a regression for git branch --verbose, which
+ This behaviour 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.
+ 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
2: 75a87887be ! 2: f536fee695 log, show: add tests for messages containing CRLF
@@ Metadata
## Commit message ##
log, show: add tests for messages containing CRLF

- A previous commit fixed a bug in ref-filter.c causing messages
- containing CRLF to be incorrectly parsed and displayed.
+ A previous commit adjusted the code in ref-filter.c so that messages
+ containing CRLF are now correctly parsed and displayed.

Add tests to also check that git log and git show correctly handle such messages, to prevent futur regressions if these commands are
@@ t/t3920-crlf-messages.sh: test_crlf_subject_body_and_contents tag --list tag-crl
+ cut -d" " -f2- <tmp-branch >actual-branch &&
+ cut -d" " -f2- <tmp-tag >actual-tag &&
+ test_cmp expect actual-branch &&
-+ test_cmp expect actual-tag
++ test_cmp expect actual-tag || return 1
+ done
+'
+

Signature · 1 line
gitgitgadget