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

[PATCH v2 7/7] format-patch tests: check quoting/encoding in To: and Cc: headers

From
JSJan H. Schönherr <schnhrr@cs.tu-berlin.de>
Date
Oct 18, 2012, 14:43 UTC
Message-ID
<1350571414-17907-8-git-send-email-schnhrr@cs.tu-berlin.de>
In-Reply-To
<1350571414-17907-1-git-send-email-schnhrr@cs.tu-berlin.de>
From: Jan H. Schönherr <schnhrr@cs.tu-berlin.de>

git-format-patch does currently not parse user supplied extra header values (e. g., --cc, --add-header) and just replays them. That forces users to add them RFC 2822/2047 conform in encoded form, e. g.

--cc '=?UTF-8?q?Jan=20H=2E=20Sch=C3=B6nherr?= <...>'

which is inconvenient. We would want to update git-format-patch to accept human-readable input

--cc 'Jan H. Schönherr <...>'

and handle the encoding, wrapping and quoting internally in the future, similar to what is already done in git-send-email. The necessary code should mostly exist in the code paths that handle the From: and Subject: headers.

Whether we want to do this only for the git-format-patch options --to and --cc (and the corresponding config options) or also for user supplied headers via --add-header, is open for discussion.

For now, add test_expect_failure tests for To: and Cc: headers as a reminder and fix tests that would otherwise fail should this get implemented.

Signed-off-by: Jan H. Schönherr <schnhrr@cs.tu-berlin.de>
---
This patch is RFC material. There are a few reasons, why this
is a good idea and also a few, why it is bad:
Pro:
- current git-format-patch behavior differs from git-send-email
- we should be able to use the address format that git uses
  elsewhere (e. g., author and committer info)
- necessary code mostly exists
Con:
- changes current behavior
- make code more complex
(Feel free to add more.)

The first drawback can be mitigated by checking whether the input is already properly encoded, so that we do not accidentally double-encode things. git-send-email does that, but that's written in Perl, so we would need even more code.

For now, this is only about _addresses_ supplied to git-format-patch, not _headers_. We could also validate/encode/wrap user supplied headers. RFC 2822/2047 is specific enough to allow that. But there is no point thinking about that without the intention of encoding addresses.

v2:
- updated commit message as suggested by Junio C Hamano
---
 t/t4014-format-patch.sh | 98 +++++++++++++++++++++++++++++++++----------------
 1 Datei geändert, 66 Zeilen hinzugefügt(+), 32 Zeilen entfernt(-)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index e024eb8..ad9f69e 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -110,73 +110,107 @@ test_expect_success 'replay did not screw up the log message' '
 
 test_expect_success 'extra headers' '
 
-	git config format.headers "To: R. E. Cipient <rcipient@example.com>
+	git config format.headers "To: R E Cipient <rcipient@example.com>
 " &&
-	git config --add format.headers "Cc: S. E. Cipient <scipient@example.com>
+	git config --add format.headers "Cc: S E Cipient <scipient@example.com>
 " &&
 	git format-patch --stdout master..side > patch2 &&
 	sed -e "/^\$/q" patch2 > hdrs2 &&
-	grep "^To: R. E. Cipient <rcipient@example.com>\$" hdrs2 &&
-	grep "^Cc: S. E. Cipient <scipient@example.com>\$" hdrs2
+	grep "^To: R E Cipient <rcipient@example.com>\$" hdrs2 &&
+	grep "^Cc: S E Cipient <scipient@example.com>\$" hdrs2
 
 '
 
 test_expect_success 'extra headers without newlines' '
 
-	git config --replace-all format.headers "To: R. E. Cipient <rcipient@example.com>" &&
-	git config --add format.headers "Cc: S. E. Cipient <scipient@example.com>" &&
+	git config --replace-all format.headers "To: R E Cipient <rcipient@example.com>" &&
+	git config --add format.headers "Cc: S E Cipient <scipient@example.com>" &&
 	git format-patch --stdout master..side >patch3 &&
 	sed -e "/^\$/q" patch3 > hdrs3 &&
-	grep "^To: R. E. Cipient <rcipient@example.com>\$" hdrs3 &&
-	grep "^Cc: S. E. Cipient <scipient@example.com>\$" hdrs3
+	grep "^To: R E Cipient <rcipient@example.com>\$" hdrs3 &&
+	grep "^Cc: S E Cipient <scipient@example.com>\$" hdrs3
 
 '
 
 test_expect_success 'extra headers with multiple To:s' '
 
-	git config --replace-all format.headers "To: R. E. Cipient <rcipient@example.com>" &&
-	git config --add format.headers "To: S. E. Cipient <scipient@example.com>" &&
+	git config --replace-all format.headers "To: R E Cipient <rcipient@example.com>" &&
+	git config --add format.headers "To: S E Cipient <scipient@example.com>" &&
 	git format-patch --stdout master..side > patch4 &&
 	sed -e "/^\$/q" patch4 > hdrs4 &&
-	grep "^To: R. E. Cipient <rcipient@example.com>,\$" hdrs4 &&
-	grep "^ *S. E. Cipient <scipient@example.com>\$" hdrs4
+	grep "^To: R E Cipient <rcipient@example.com>,\$" hdrs4 &&
+	grep "^ *S E Cipient <scipient@example.com>\$" hdrs4
 '
 
-test_expect_success 'additional command line cc' '
+test_expect_success 'additional command line cc (ascii)' '
 
-	git config --replace-all format.headers "Cc: R. E. Cipient <rcipient@example.com>" &&
+	git config --replace-all format.headers "Cc: R E Cipient <rcipient@example.com>" &&
+	git format-patch --cc="S E Cipient <scipient@example.com>" --stdout master..side | sed -e "/^\$/q" >patch5 &&
+	grep "^Cc: R E Cipient <rcipient@example.com>,\$" patch5 &&
+	grep "^ *S E Cipient <scipient@example.com>\$" patch5
+'
+
+test_expect_failure 'additional command line cc (rfc822)' '
+
+	git config --replace-all format.headers "Cc: R E Cipient <rcipient@example.com>" &&
 	git format-patch --cc="S. E. Cipient <scipient@example.com>" --stdout master..side | sed -e "/^\$/q" >patch5 &&
-	grep "^Cc: R. E. Cipient <rcipient@example.com>,\$" patch5 &&
-	grep "^ *S. E. Cipient <scipient@example.com>\$" patch5
+	grep "^Cc: R E Cipient <rcipient@example.com>,\$" patch5 &&
+	grep "^ *"S. E. Cipient" <scipient@example.com>\$" patch5
 '
 
 test_expect_success 'command line headers' '
 
 	git config --unset-all format.headers &&
-	git format-patch --add-header="Cc: R. E. Cipient <rcipient@example.com>" --stdout master..side | sed -e "/^\$/q" >patch6 &&
-	grep "^Cc: R. E. Cipient <rcipient@example.com>\$" patch6
+	git format-patch --add-header="Cc: R E Cipient <rcipient@example.com>" --stdout master..side | sed -e "/^\$/q" >patch6 &&
+	grep "^Cc: R E Cipient <rcipient@example.com>\$" patch6
 '
 
 test_expect_success 'configuration headers and command line headers' '
 
-	git config --replace-all format.headers "Cc: R. E. Cipient <rcipient@example.com>" &&
-	git format-patch --add-header="Cc: S. E. Cipient <scipient@example.com>" --stdout master..side | sed -e "/^\$/q" >patch7 &&
-	grep "^Cc: R. E. Cipient <rcipient@example.com>,\$" patch7 &&
-	grep "^ *S. E. Cipient <scipient@example.com>\$" patch7
+	git config --replace-all format.headers "Cc: R E Cipient <rcipient@example.com>" &&
+	git format-patch --add-header="Cc: S E Cipient <scipient@example.com>" --stdout master..side | sed -e "/^\$/q" >patch7 &&
+	grep "^Cc: R E Cipient <rcipient@example.com>,\$" patch7 &&
+	grep "^ *S E Cipient <scipient@example.com>\$" patch7
 '
 
-test_expect_success 'command line To: header' '
+test_expect_success 'command line To: header (ascii)' '
 
 	git config --unset-all format.headers &&
+	git format-patch --to="R E Cipient <rcipient@example.com>" --stdout master..side | sed -e "/^\$/q" >patch8 &&
+	grep "^To: R E Cipient <rcipient@example.com>\$" patch8
+'
+
+test_expect_failure 'command line To: header (rfc822)' '
+
 	git format-patch --to="R. E. Cipient <rcipient@example.com>" --stdout master..side | sed -e "/^\$/q" >patch8 &&
-	grep "^To: R. E. Cipient <rcipient@example.com>\$" patch8
+	grep "^To: "R. E. Cipient" <rcipient@example.com>\$" patch8
+'
+
+test_expect_failure 'command line To: header (rfc2047)' '
+
+	git format-patch --to="R Ä Cipient <rcipient@example.com>" --stdout master..side | sed -e "/^\$/q" >patch8 &&
+	grep "^To: =?UTF-8?q?R=20=C3=84=20Cipient?= <rcipient@example.com>\$" patch8
 '
 
-test_expect_success 'configuration To: header' '
+test_expect_success 'configuration To: header (ascii)' '
+
+	git config format.to "R E Cipient <rcipient@example.com>" &&
+	git format-patch --stdout master..side | sed -e "/^\$/q" >patch9 &&
+	grep "^To: R E Cipient <rcipient@example.com>\$" patch9
+'
+
+test_expect_failure 'configuration To: header (rfc822)' '
 
 	git config format.to "R. E. Cipient <rcipient@example.com>" &&
 	git format-patch --stdout master..side | sed -e "/^\$/q" >patch9 &&
-	grep "^To: R. E. Cipient <rcipient@example.com>\$" patch9
+	grep "^To: "R. E. Cipient" <rcipient@example.com>\$" patch9
+'
+
+test_expect_failure 'configuration To: header (rfc2047)' '
+
+	git config format.to "R Ä Cipient <rcipient@example.com>" &&
+	git format-patch --stdout master..side | sed -e "/^\$/q" >patch9 &&
+	grep "^To: =?UTF-8?q?R=20=C3=84=20Cipient?= <rcipient@example.com>\$" patch9
 '
 
 # check_patch <patch>: Verify that <patch> looks like a half-sane
@@ -190,11 +224,11 @@ check_patch () {
 test_expect_success '--no-to overrides config.to' '
 
 	git config --replace-all format.to \
-		"R. E. Cipient <rcipient@example.com>" &&
+		"R E Cipient <rcipient@example.com>" &&
 	git format-patch --no-to --stdout master..side |
 	sed -e "/^\$/q" >patch10 &&
 	check_patch patch10 &&
-	! grep "^To: R. E. Cipient <rcipient@example.com>\$" patch10
+	! grep "^To: R E Cipient <rcipient@example.com>\$" patch10
 '
 
 test_expect_success '--no-to and --to replaces config.to' '
@@ -212,21 +246,21 @@ test_expect_success '--no-to and --to replaces config.to' '
 test_expect_success '--no-cc overrides config.cc' '
 
 	git config --replace-all format.cc \
-		"C. E. Cipient <rcipient@example.com>" &&
+		"C E Cipient <rcipient@example.com>" &&
 	git format-patch --no-cc --stdout master..side |
 	sed -e "/^\$/q" >patch12 &&
 	check_patch patch12 &&
-	! grep "^Cc: C. E. Cipient <rcipient@example.com>\$" patch12
+	! grep "^Cc: C E Cipient <rcipient@example.com>\$" patch12
 '
 
 test_expect_success '--no-add-header overrides config.headers' '
 
 	git config --replace-all format.headers \
-		"Header1: B. E. Cipient <rcipient@example.com>" &&
+		"Header1: B E Cipient <rcipient@example.com>" &&
 	git format-patch --no-add-header --stdout master..side |
 	sed -e "/^\$/q" >patch13 &&
 	check_patch patch13 &&
-	! grep "^Header1: B. E. Cipient <rcipient@example.com>\$" patch13
+	! grep "^Header1: B E Cipient <rcipient@example.com>\$" patch13
 '
 
 test_expect_success 'multiple files' '
-- 
1.7.12
Previous: Jan H. Schönherr
Message 8 of 8 in “Cure some format-patch wrapping and encoding issues”
  1. 0/7 Cure some format-patch wrapping and encoding issuesJan H. Schönherr, Oct 18, 2012
  2. 1/7 utf8: fix off-by-one wrapping of textJan H. Schönherr, Oct 18, 2012
  3. 2/7 format-patch: do not wrap non-rfc2047 headers too earlyJan H. Schönherr, Oct 18, 2012
  4. 3/7 format-patch: do not wrap rfc2047 encoded headers too lateJan H. Schönherr, Oct 18, 2012
  5. 4/7 format-patch: introduce helper function last_line_length()Jan H. Schönherr, Oct 18, 2012
  6. 5/7 format-patch: make rfc2047 encoding more strictJan H. Schönherr, Oct 18, 2012
  7. 6/7 format-patch: fix rfc2047 address encoding with respect to rfc822 specialsJan H. Schönherr, Oct 18, 2012
  8. 7/7 format-patch tests: check quoting/encoding in To: and Cc: headersJan H. Schönherr, Oct 18, 2012

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.