{"thread":{"id":"31753","subject":"[PATCH 4/5] format-patch: fix rfc2047 address encoding with respect to rfc822 specials","startedAt":"2012-10-08T17:33:24Z","lastAt":"2012-10-10T17:02:24Z","messageCount":10,"participants":["Jan H. Schönherr","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"200764","messageId":"1349717609-4770-1-git-send-email-schnhrr@cs.tu-berlin.de","threadId":"31753","inReplyTo":null,"subject":"[PATCH 0/5] Cure some format-patch wrapping and encoding issues","fromName":"Jan H. Schönherr","fromEmail":"schnhrr@cs.tu-berlin.de","sentAt":"2012-10-08T17:33:24Z","receivedAt":"2012-10-08T17:33:24Z","isPatch":true,"sender":{"key":"schnhrr@cs.tu-berlin.de","avatar":null},"body":"Hi all.\n\nThe main point of this series is to teach git to encode my name\ncorrectly, see patch 4, so that the decoded version is actually\nmy name, so that send-email does not insist on adding a wrong\nsuperfluous From: line to the mail body.\n\nBut as always, you notice some other things going wrong. Here,\npatches 1 and 2 make the wrapping of header lines more correct,\ni. e., neither too early nor too late.\n\nPatch 3 does some refactoring, which is too unrelated to be included\nin patch 4 itself.\n\nPatch 5 points out further problems, but leaves the actual fixing\nto someone else.\n\nThe series is currently based on the maint branch, but it applies\nto the others as well.\n\n\n\nDuring the creation of this series, I came across the strbuf \nwrapping functions, and I wonder if there is an off-by-one issue.\n\nConsider the following excerpt from t4202:\n\ncat > expect << EOF\n This is\n the sixth\n commit.\n This is\n the fifth\n commit.\nEOF\n\ntest_expect_success 'format %w(12,1,2)' '\n\n\tgit log -2 --format=\"%w(12,1,2)This is the %s commit.\" > actual &&\n\ttest_cmp expect actual\n'\n\nSo this sets a maximum width of 12 characters. Is that 12 character limit\nsupposed to include the final newline, or not? Because the test above and\nmy series are only correct if the final newline is included, i. e., at most\neleven visible characters.\n\nIf this should mean at most 12 visible characters instead, then the output\nshould look like this:\n\n This is the\n sixth\n commit.\n This is the\n fifth\n commit.\n\n(In that case, I would repost an updated version of this series.)\n\nRegards\nJan\n\n\nJan H. Schönherr (5):\n format-patch: do not wrap non-rfc2047 headers too early\n format-patch: do not wrap rfc2047 encoded headers too late\n format-patch: introduce helper function last_line_length()\n format-patch: fix rfc2047 address encoding with respect to rfc822\n specials\n format-patch: tests: check rfc822+rfc2047 in to+cc headers\n\n pretty.c | 121 ++++++++++++++++++--------\n t/t4014-format-patch.sh | 227 ++++++++++++++++++++++++++++++------------------\n 2 Dateien geändert, 229 Zeilen hinzugefügt(+), 119 Zeilen entfernt(-)\n\n-- \n1.7.12\n"},{"id":"200766","messageId":"1349717609-4770-2-git-send-email-schnhrr@cs.tu-berlin.de","threadId":"31753","inReplyTo":"1349717609-4770-1-git-send-email-schnhrr@cs.tu-berlin.de","subject":"[PATCH 1/5] format-patch: do not wrap non-rfc2047 headers too early","fromName":"Jan H. Schönherr","fromEmail":"schnhrr@cs.tu-berlin.de","sentAt":"2012-10-08T17:33:25Z","receivedAt":"2012-10-08T17:33:25Z","isPatch":true,"sender":{"key":"schnhrr@cs.tu-berlin.de","avatar":null},"body":"From: Jan H. Schönherr \n\nDo not wrap the second and later lines of an ASCII header substantially\nbefore the 78 character limit.\n\nSigned-off-by: Jan H. Schönherr \n---\n pretty.c | 2 +-\n t/t4014-format-patch.sh | 60 ++++++++++++++++++++++++++++---------------------\n 2 Dateien geändert, 35 Zeilen hinzugefügt(+), 27 Zeilen entfernt(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 8b1ea9f..f5caecb 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -286,7 +286,7 @@ static void add_rfc2047(struct strbuf *sb, const char *line, int len,\n \t\tif ((i + 1 < len) && (ch == '=' && line[i+1] == '?'))\n \t\t\tgoto needquote;\n \t}\n-\tstrbuf_add_wrapped_bytes(sb, line, len, 0, 1, max_length - line_len);\n+\tstrbuf_add_wrapped_bytes(sb, line, len, -line_len, 1, max_length+1);\n \treturn;\n \n needquote:\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 959aa26..d66e358 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -752,16 +752,14 @@ M64=$M8$M8$M8$M8$M8$M8$M8$M8\n M512=$M64$M64$M64$M64$M64$M64$M64$M64\n cat >expect <<'EOF'\n Subject: [PATCH] foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo\n- bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar\n- foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo\n- bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar\n- foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo\n- bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar\n- foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo\n- bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar\n- foo bar foo bar foo bar foo bar\n+ bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar\n+ foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo\n+ bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar\n+ foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo\n+ bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar\n+ foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar\n EOF\n-test_expect_success 'format-patch wraps extremely long headers (ascii)' '\n+test_expect_success 'format-patch wraps extremely long subject (ascii)' '\n \techo content >>file &&\n \tgit add file &&\n \tgit commit -m \"$M512\" &&\n@@ -807,28 +805,12 @@ test_expect_success 'format-patch wraps extremely long headers (rfc2047)' '\n \ttest_cmp expect subject\n '\n \n-M8=\"foo_bar_\"\n-M64=$M8$M8$M8$M8$M8$M8$M8$M8\n-cat >expect <\n-EOF\n-test_expect_success 'format-patch wraps non-quotable headers' '\n-\trm -rf patches/ &&\n-\techo content >>file &&\n-\tgit add file &&\n-\tgit commit -mfoo --author \"$M64 \" &&\n-\tgit format-patch --stdout -1 >patch &&\n-\tsed -n \"/^From: /p; /^ /p; /^$/q\" from &&\n-\ttest_cmp expect from\n-'\n-\n check_author() {\n \techo content >>file &&\n \tgit add file &&\n \tGIT_AUTHOR_NAME=$1 git commit -m author-check &&\n \tgit format-patch --stdout -1 >patch &&\n-\tgrep ^From: patch >actual &&\n+\tsed -n \"/^From: /p; /^ /p; /^$/q\" actual &&\n \ttest_cmp expect actual\n }\n \n@@ -853,6 +835,32 @@ test_expect_success 'rfc2047-encoded headers also double-quote 822 specials' '\n \tcheck_author \"Föo B. Bar\"\n '\n \n+cat >expect <\n+EOF\n+test_expect_success 'format-patch wraps moderately long from-header (ascii)' '\n+\tcheck_author \"foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_\"\n+'\n+\n+cat >expect <<'EOF'\n+From: Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar\n+ Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo\n+ Bar Foo Bar Foo Bar Foo Bar \n+EOF\n+test_expect_success 'format-patch wraps extremely long from-header (ascii)' '\n+\tcheck_author \"Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar\"\n+'\n+\n+cat >expect <<'EOF'\n+From: \"Foo.Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar\n+ Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo\n+ Bar Foo Bar Foo Bar Foo Bar\" \n+EOF\n+test_expect_success 'format-patch wraps extremely long from-header (rfc822)' '\n+\tcheck_author \"Foo.Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar\"\n+'\n+\n cat >expect <<'EOF'\n Subject: header with . in it\n EOF\n-- \n1.7.12\n"},{"id":"200768","messageId":"1349717609-4770-3-git-send-email-schnhrr@cs.tu-berlin.de","threadId":"31753","inReplyTo":"1349717609-4770-1-git-send-email-schnhrr@cs.tu-berlin.de","subject":"[PATCH 2/5] format-patch: do not wrap rfc2047 encoded headers too late","fromName":"Jan H. Schönherr","fromEmail":"schnhrr@cs.tu-berlin.de","sentAt":"2012-10-08T17:33:26Z","receivedAt":"2012-10-08T17:33:26Z","isPatch":true,"sender":{"key":"schnhrr@cs.tu-berlin.de","avatar":null},"body":"From: Jan H. Schönherr \n\nEncoded characters add more than one character at once to an encoded\nheader. Include all characters that are about to be added in the length\ncalculation for wrapping.\n\nAdditionally, RFC 2047 imposes a maximum line length of 76 characters\nif that line contains an rfc2047 encoded word.\n\nSigned-off-by: Jan H. Schönherr \n---\n pretty.c | 24 +++++++++++---------\n t/t4014-format-patch.sh | 58 +++++++++++++++++++++++++++++--------------------\n 2 Dateien geändert, 49 Zeilen hinzugefügt(+), 33 Zeilen entfernt(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex f5caecb..daf8581 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -263,13 +263,22 @@ static void add_rfc822_quoted(struct strbuf *out, const char *s, int len)\n \n static int is_rfc2047_special(char ch)\n {\n+\t/*\n+\t * We encode ' ' using '=20' even though rfc2047\n+\t * allows using '_' for readability. Unfortunately,\n+\t * many programs do not understand this and just\n+\t * leave the underscore in place.\n+\t */\n+\tif (ch == ' ' || ch == '\\n')\n+\t\treturn 1;\n+\n \treturn (non_ascii(ch) || (ch == '=') || (ch == '?') || (ch == '_'));\n }\n \n static void add_rfc2047(struct strbuf *sb, const char *line, int len,\n \t\t const char *encoding)\n {\n-\tstatic const int max_length = 78; /* per rfc2822 */\n+\tstatic const int max_length = 76; /* per rfc2047 */\n \tint i;\n \tint line_len;\n \n@@ -286,7 +295,7 @@ static void add_rfc2047(struct strbuf *sb, const char *line, int len,\n \t\tif ((i + 1 < len) && (ch == '=' && line[i+1] == '?'))\n \t\t\tgoto needquote;\n \t}\n-\tstrbuf_add_wrapped_bytes(sb, line, len, -line_len, 1, max_length+1);\n+\tstrbuf_add_wrapped_bytes(sb, line, len, -line_len, 1, 78+1);\n \treturn;\n \n needquote:\n@@ -295,19 +304,14 @@ needquote:\n \tline_len += strlen(encoding) + 5; /* 5 for =??q? */\n \tfor (i = 0; i < len; i++) {\n \t\tunsigned ch = line[i] & 0xFF;\n+\t\tint is_special = is_rfc2047_special(ch);\n \n-\t\tif (line_len >= max_length - 2) {\n+\t\tif (line_len + 2 + (is_special ? 3 : 1) > max_length) {\n \t\t\tstrbuf_addf(sb, \"?=\\n =?%s?q?\", encoding);\n \t\t\tline_len = strlen(encoding) + 5 + 1; /* =??q? plus SP */\n \t\t}\n \n-\t\t/*\n-\t\t * We encode ' ' using '=20' even though rfc2047\n-\t\t * allows using '_' for readability. Unfortunately,\n-\t\t * many programs do not understand this and just\n-\t\t * leave the underscore in place.\n-\t\t */\n-\t\tif (is_rfc2047_special(ch) || ch == ' ' || ch == '\\n') {\n+\t\tif (is_special) {\n \t\t\tstrbuf_addf(sb, \"=%02X\", ch);\n \t\t\tline_len += 3;\n \t\t}\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex d66e358..1d5636d 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -772,30 +772,31 @@ M8=\"föö bar \"\n M64=$M8$M8$M8$M8$M8$M8$M8$M8\n M512=$M64$M64$M64$M64$M64$M64$M64$M64\n cat >expect <<'EOF'\n-Subject: [PATCH] =?UTF-8?q?f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=\n+Subject: [PATCH] =?UTF-8?q?f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=\n+ =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=\n+ =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=\n+ =?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=\n+ =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3?=\n+ =?UTF-8?q?=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n+ =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3?=\n+ =?UTF-8?q?=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=\n+ =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=\n+ =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=\n+ =?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=\n+ =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3?=\n+ =?UTF-8?q?=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n+ =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3?=\n+ =?UTF-8?q?=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=\n+ =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=\n+ =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=\n+ =?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=\n+ =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3?=\n+ =?UTF-8?q?=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=\n+ =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3?=\n+ =?UTF-8?q?=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=\n+ =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=\n EOF\n-test_expect_success 'format-patch wraps extremely long headers (rfc2047)' '\n+test_expect_success 'format-patch wraps extremely long subject (rfc2047)' '\n \trm -rf patches/ &&\n \techo content >>file &&\n \tgit add file &&\n@@ -862,6 +863,17 @@ test_expect_success 'format-patch wraps extremely long from-header (rfc822)' '\n '\n \n cat >expect <<'EOF'\n+From: =?UTF-8?q?Fo=C3=B6=20Bar=20Foo=20Bar=20Foo=20Bar=20Foo=20Bar=20Foo?=\n+ =?UTF-8?q?=20Bar=20Foo=20Bar=20Foo=20Bar=20Foo=20Bar=20Foo=20Bar=20Foo=20?=\n+ =?UTF-8?q?Bar=20Foo=20Bar=20Foo=20Bar=20Foo=20Bar=20Foo=20Bar=20Foo=20Bar?=\n+ =?UTF-8?q?=20Foo=20Bar=20Foo=20Bar=20Foo=20Bar=20Foo=20Bar=20Foo=20Bar=20?=\n+ =?UTF-8?q?Foo=20Bar=20Foo=20Bar?= \n+EOF\n+test_expect_success 'format-patch wraps extremely long from-header (rfc2047)' '\n+\tcheck_author \"Foö Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar\"\n+'\n+\n+cat >expect <<'EOF'\n Subject: header with . in it\n EOF\n test_expect_success 'subject lines do not have 822 atom-quoting' '\n-- \n1.7.12\n"},{"id":"200767","messageId":"1349717609-4770-4-git-send-email-schnhrr@cs.tu-berlin.de","threadId":"31753","inReplyTo":"1349717609-4770-1-git-send-email-schnhrr@cs.tu-berlin.de","subject":"[PATCH 3/5] format-patch: introduce helper function last_line_length()","fromName":"Jan H. Schönherr","fromEmail":"schnhrr@cs.tu-berlin.de","sentAt":"2012-10-08T17:33:27Z","receivedAt":"2012-10-08T17:33:27Z","isPatch":true,"sender":{"key":"schnhrr@cs.tu-berlin.de","avatar":null},"body":"From: Jan H. Schönherr \n\nCurrently, an open-coded loop to calculate the length of the last\nline of a string buffer is used in multiple places.\n\nMove that code into a function of its own.\n\nSigned-off-by: Jan H. Schönherr \n---\n pretty.c | 25 +++++++++++++------------\n 1 Datei geändert, 13 Zeilen hinzugefügt(+), 12 Zeilen entfernt(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex daf8581..ee76219 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -240,6 +240,17 @@ static int has_rfc822_specials(const char *s, int len)\n \treturn 0;\n }\n \n+static int last_line_length(struct strbuf *sb)\n+{\n+\tint i;\n+\n+\t/* How many bytes are already used on the last line? */\n+\tfor (i = sb->len - 1; i >= 0; i--)\n+\t\tif (sb->buf[i] == '\\n')\n+\t\t\tbreak;\n+\treturn sb->len - (i + 1);\n+}\n+\n static void add_rfc822_quoted(struct strbuf *out, const char *s, int len)\n {\n \tint i;\n@@ -280,13 +291,7 @@ static void add_rfc2047(struct strbuf *sb, const char *line, int len,\n {\n \tstatic const int max_length = 76; /* per rfc2047 */\n \tint i;\n-\tint line_len;\n-\n-\t/* How many bytes are already used on the current line? */\n-\tfor (i = sb->len - 1; i >= 0; i--)\n-\t\tif (sb->buf[i] == '\\n')\n-\t\t\tbreak;\n-\tline_len = sb->len - (i+1);\n+\tint line_len = last_line_length(sb);\n \n \tfor (i = 0; i < len; i++) {\n \t\tint ch = line[i];\n@@ -344,7 +349,6 @@ void pp_user_info(const struct pretty_print_context *pp,\n \tif (pp->fmt == CMIT_FMT_EMAIL) {\n \t\tchar *name_tail = strchr(line, '<');\n \t\tint display_name_length;\n-\t\tint final_line;\n \t\tif (!name_tail)\n \t\t\treturn;\n \t\twhile (line < name_tail && isspace(name_tail[-1]))\n@@ -359,10 +363,7 @@ void pp_user_info(const struct pretty_print_context *pp,\n \t\t\tadd_rfc2047(sb, quoted.buf, quoted.len, encoding);\n \t\t\tstrbuf_release("ed);\n \t\t}\n-\t\tfor (final_line = 0; final_line < sb->len; final_line++)\n-\t\t\tif (sb->buf[sb->len - final_line - 1] == '\\n')\n-\t\t\t\tbreak;\n-\t\tif (namelen - display_name_length + final_line > 78) {\n+\t\tif (namelen - display_name_length + last_line_length(sb) > 78) {\n \t\t\tstrbuf_addch(sb, '\\n');\n \t\t\tif (!isspace(name_tail[0]))\n \t\t\t\tstrbuf_addch(sb, ' ');\n-- \n1.7.12\n"},{"id":"200762","messageId":"1349717609-4770-5-git-send-email-schnhrr@cs.tu-berlin.de","threadId":"31753","inReplyTo":"1349717609-4770-1-git-send-email-schnhrr@cs.tu-berlin.de","subject":"[PATCH 4/5] format-patch: fix rfc2047 address encoding with respect to rfc822 specials","fromName":"Jan H. Schönherr","fromEmail":"schnhrr@cs.tu-berlin.de","sentAt":"2012-10-08T17:33:28Z","receivedAt":"2012-10-08T17:33:28Z","isPatch":true,"sender":{"key":"schnhrr@cs.tu-berlin.de","avatar":null},"body":"From: Jan H. Schönherr \n\nAccording to RFC 2047 and RFC 822, rfc2047 encoded words and and rfc822\nquoted strings do not mix.\n\nBe more strict about rfc2047 encoded words in addresses, so that it is a\nbit more conform to RFC 2047.\n\n(Especially, my own name gets correctly decoded as Jan H. Schönherr\n(without quotes) and not as \"Jan H. Schönherr\" (with quotes).)\n\nSigned-off-by: Jan H. Schönherr \n---\n pretty.c | 80 ++++++++++++++++++++++++++++++++++++++-----------\n t/t4014-format-patch.sh | 11 +++++--\n 2 Dateien geändert, 71 Zeilen hinzugefügt(+), 20 Zeilen entfernt(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex ee76219..f3a7383 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -231,7 +231,7 @@ static int is_rfc822_special(char ch)\n \t}\n }\n \n-static int has_rfc822_specials(const char *s, int len)\n+static int needs_rfc822_quoting(const char *s, int len)\n {\n \tint i;\n \tfor (i = 0; i < len; i++)\n@@ -272,7 +272,12 @@ static void add_rfc822_quoted(struct strbuf *out, const char *s, int len)\n \tstrbuf_addch(out, '\"');\n }\n \n-static int is_rfc2047_special(char ch)\n+enum rfc2047_type {\n+\tRFC2047_SUBJECT,\n+\tRFC2047_ADDRESS,\n+};\n+\n+static int is_rfc2047_special(char ch, enum rfc2047_type type)\n {\n \t/*\n \t * We encode ' ' using '=20' even though rfc2047\n@@ -283,33 +288,62 @@ static int is_rfc2047_special(char ch)\n \tif (ch == ' ' || ch == '\\n')\n \t\treturn 1;\n \n-\treturn (non_ascii(ch) || (ch == '=') || (ch == '?') || (ch == '_'));\n+\tif (non_ascii(ch) || (ch == '=') || (ch == '?') || (ch == '_'))\n+\t\treturn 1;\n+\n+\tif (type != RFC2047_ADDRESS)\n+\t\treturn 0;\n+\n+\t/*\n+\t * rfc2047, section 5.3:\n+\t *\n+\t * As a replacement for a 'word' entity within a 'phrase', for example,\n+\t * one that precedes an address in a From, To, or Cc header. The ABNF\n+\t * definition for 'phrase' from RFC 822 thus becomes:\n+\t *\n+\t * phrase = 1*( encoded-word / word )\n+\t *\n+\t * In this case the set of characters that may be used in a \"Q\"-encoded\n+\t * 'encoded-word' is restricted to: . An 'encoded-word' that appears within a\n+\t * 'phrase' MUST be separated from any adjacent 'word', 'text' or\n+\t * 'special' by 'linear-white-space'.\n+\t */\n+\n+\t/* '=' and '_' are special cases and have been checked above */\n+\treturn !(isalnum(ch) || ch == '!' || ch == '*' || ch == '+' || ch == '-' || ch == '/');\n }\n \n-static void add_rfc2047(struct strbuf *sb, const char *line, int len,\n-\t\t const char *encoding)\n+static int needs_rfc2047_encoding(const char *line, int len,\n+\t\t\t\t enum rfc2047_type type)\n {\n-\tstatic const int max_length = 76; /* per rfc2047 */\n \tint i;\n-\tint line_len = last_line_length(sb);\n \n \tfor (i = 0; i < len; i++) {\n \t\tint ch = line[i];\n \t\tif (non_ascii(ch) || ch == '\\n')\n-\t\t\tgoto needquote;\n+\t\t\treturn 1;\n \t\tif ((i + 1 < len) && (ch == '=' && line[i+1] == '?'))\n-\t\t\tgoto needquote;\n+\t\t\treturn 1;\n \t}\n-\tstrbuf_add_wrapped_bytes(sb, line, len, -line_len, 1, 78+1);\n-\treturn;\n \n-needquote:\n+\treturn 0;\n+}\n+\n+static void add_rfc2047(struct strbuf *sb, const char *line, int len,\n+\t\t const char *encoding, enum rfc2047_type type)\n+{\n+\tstatic const int max_length = 76; /* per rfc2047 */\n+\tint i;\n+\tint line_len = last_line_length(sb);\n+\n \tstrbuf_grow(sb, len * 3 + strlen(encoding) + 100);\n \tstrbuf_addf(sb, \"=?%s?q?\", encoding);\n \tline_len += strlen(encoding) + 5; /* 5 for =??q? */\n \tfor (i = 0; i < len; i++) {\n \t\tunsigned ch = line[i] & 0xFF;\n-\t\tint is_special = is_rfc2047_special(ch);\n+\t\tint is_special = is_rfc2047_special(ch, type);\n \n \t\tif (line_len + 2 + (is_special ? 3 : 1) > max_length) {\n \t\t\tstrbuf_addf(sb, \"?=\\n =?%s?q?\", encoding);\n@@ -355,13 +389,18 @@ void pp_user_info(const struct pretty_print_context *pp,\n \t\t\tname_tail--;\n \t\tdisplay_name_length = name_tail - line;\n \t\tstrbuf_addstr(sb, \"From: \");\n-\t\tif (!has_rfc822_specials(line, display_name_length)) {\n-\t\t\tadd_rfc2047(sb, line, display_name_length, encoding);\n-\t\t} else {\n+\t\tif (needs_rfc2047_encoding(line, display_name_length, RFC2047_ADDRESS)) {\n+\t\t\tadd_rfc2047(sb, line, display_name_length,\n+\t\t\t\t\t\tencoding, RFC2047_ADDRESS);\n+\t\t} else if (needs_rfc822_quoting(line, display_name_length)) {\n \t\t\tstruct strbuf quoted = STRBUF_INIT;\n \t\t\tadd_rfc822_quoted("ed, line, display_name_length);\n-\t\t\tadd_rfc2047(sb, quoted.buf, quoted.len, encoding);\n+\t\t\tstrbuf_add_wrapped_bytes(sb, quoted.buf, quoted.len,\n+\t\t\t\t\t\t\t\t-6, 1, 78+1);\n \t\t\tstrbuf_release("ed);\n+\t\t} else {\n+\t\t\tstrbuf_add_wrapped_bytes(sb, line, display_name_length,\n+\t\t\t\t\t\t\t\t-6, 1, 78+1);\n \t\t}\n \t\tif (namelen - display_name_length + last_line_length(sb) > 78) {\n \t\t\tstrbuf_addch(sb, '\\n');\n@@ -1292,7 +1331,12 @@ void pp_title_line(const struct pretty_print_context *pp,\n \tstrbuf_grow(sb, title.len + 1024);\n \tif (pp->subject) {\n \t\tstrbuf_addstr(sb, pp->subject);\n-\t\tadd_rfc2047(sb, title.buf, title.len, encoding);\n+\t\tif (needs_rfc2047_encoding(title.buf, title.len, RFC2047_SUBJECT))\n+\t\t\tadd_rfc2047(sb, title.buf, title.len, encoding,\n+\t\t\t\t RFC2047_SUBJECT);\n+\t\telse\n+\t\t\tstrbuf_add_wrapped_bytes(sb, title.buf, title.len,\n+\t\t\t\t\t\t -last_line_length(sb), 1, 78+1);\n \t} else {\n \t\tstrbuf_addbuf(sb, &title);\n \t}\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 1d5636d..1a3b6e8 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -830,9 +830,16 @@ test_expect_success 'format-patch quotes double-quote in headers' '\n '\n \n cat >expect <<'EOF'\n-From: =?UTF-8?q?\"F=C3=B6o=20B.=20Bar\"?= \n+From: =?UTF-8?q?F=C3=B6o=20Bar?= \n EOF\n-test_expect_success 'rfc2047-encoded headers also double-quote 822 specials' '\n+test_expect_success 'format-patch uses rfc2047-encoded headers when necessary' '\n+\tcheck_author \"Föo Bar\"\n+'\n+\n+cat >expect <<'EOF'\n+From: =?UTF-8?q?F=C3=B6o=20B=2E=20Bar?= \n+EOF\n+test_expect_success 'rfc2047-encoded headers leave no rfc822 specials' '\n \tcheck_author \"Föo B. Bar\"\n '\n \n-- \n1.7.12\n"},{"id":"200763","messageId":"1349717609-4770-6-git-send-email-schnhrr@cs.tu-berlin.de","threadId":"31753","inReplyTo":"1349717609-4770-1-git-send-email-schnhrr@cs.tu-berlin.de","subject":"[PATCH 5/5] format-patch: tests: check rfc822+rfc2047 in to+cc headers","fromName":"Jan H. Schönherr","fromEmail":"schnhrr@cs.tu-berlin.de","sentAt":"2012-10-08T17:33:29Z","receivedAt":"2012-10-08T17:33:29Z","isPatch":true,"sender":{"key":"schnhrr@cs.tu-berlin.de","avatar":null},"body":"From: Jan H. Schönherr \n\nDo some checks for RFC 822 and RFC 2047 support in To:\nand Cc: headers and fix ambiguous old checks.\n\nSigned-off-by: Jan H. Schönherr \n---\n t/t4014-format-patch.sh | 98 +++++++++++++++++++++++++++++++++----------------\n 1 Datei geändert, 66 Zeilen hinzugefügt(+), 32 Zeilen entfernt(-)\n\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 1a3b6e8..65ab4c9 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -110,73 +110,107 @@ test_expect_success 'replay did not screw up the log message' '\n \n test_expect_success 'extra headers' '\n \n-\tgit config format.headers \"To: R. E. Cipient \n+\tgit config format.headers \"To: R E Cipient \n \" &&\n-\tgit config --add format.headers \"Cc: S. E. Cipient \n+\tgit config --add format.headers \"Cc: S E Cipient \n \" &&\n \tgit format-patch --stdout master..side > patch2 &&\n \tsed -e \"/^\\$/q\" patch2 > hdrs2 &&\n-\tgrep \"^To: R. E. Cipient \\$\" hdrs2 &&\n-\tgrep \"^Cc: S. E. Cipient \\$\" hdrs2\n+\tgrep \"^To: R E Cipient \\$\" hdrs2 &&\n+\tgrep \"^Cc: S E Cipient \\$\" hdrs2\n \n '\n \n test_expect_success 'extra headers without newlines' '\n \n-\tgit config --replace-all format.headers \"To: R. E. Cipient \" &&\n-\tgit config --add format.headers \"Cc: S. E. Cipient \" &&\n+\tgit config --replace-all format.headers \"To: R E Cipient \" &&\n+\tgit config --add format.headers \"Cc: S E Cipient \" &&\n \tgit format-patch --stdout master..side >patch3 &&\n \tsed -e \"/^\\$/q\" patch3 > hdrs3 &&\n-\tgrep \"^To: R. E. Cipient \\$\" hdrs3 &&\n-\tgrep \"^Cc: S. E. Cipient \\$\" hdrs3\n+\tgrep \"^To: R E Cipient \\$\" hdrs3 &&\n+\tgrep \"^Cc: S E Cipient \\$\" hdrs3\n \n '\n \n test_expect_success 'extra headers with multiple To:s' '\n \n-\tgit config --replace-all format.headers \"To: R. E. Cipient \" &&\n-\tgit config --add format.headers \"To: S. E. Cipient \" &&\n+\tgit config --replace-all format.headers \"To: R E Cipient \" &&\n+\tgit config --add format.headers \"To: S E Cipient \" &&\n \tgit format-patch --stdout master..side > patch4 &&\n \tsed -e \"/^\\$/q\" patch4 > hdrs4 &&\n-\tgrep \"^To: R. E. Cipient ,\\$\" hdrs4 &&\n-\tgrep \"^ *S. E. Cipient \\$\" hdrs4\n+\tgrep \"^To: R E Cipient ,\\$\" hdrs4 &&\n+\tgrep \"^ *S E Cipient \\$\" hdrs4\n '\n \n-test_expect_success 'additional command line cc' '\n+test_expect_success 'additional command line cc (ascii)' '\n \n-\tgit config --replace-all format.headers \"Cc: R. E. Cipient \" &&\n+\tgit config --replace-all format.headers \"Cc: R E Cipient \" &&\n+\tgit format-patch --cc=\"S E Cipient \" --stdout master..side | sed -e \"/^\\$/q\" >patch5 &&\n+\tgrep \"^Cc: R E Cipient ,\\$\" patch5 &&\n+\tgrep \"^ *S E Cipient \\$\" patch5\n+'\n+\n+test_expect_failure 'additional command line cc (rfc822)' '\n+\n+\tgit config --replace-all format.headers \"Cc: R E Cipient \" &&\n \tgit format-patch --cc=\"S. E. Cipient \" --stdout master..side | sed -e \"/^\\$/q\" >patch5 &&\n-\tgrep \"^Cc: R. E. Cipient ,\\$\" patch5 &&\n-\tgrep \"^ *S. E. Cipient \\$\" patch5\n+\tgrep \"^Cc: R E Cipient ,\\$\" patch5 &&\n+\tgrep \"^ *\"S. E. Cipient\" \\$\" patch5\n '\n \n test_expect_success 'command line headers' '\n \n \tgit config --unset-all format.headers &&\n-\tgit format-patch --add-header=\"Cc: R. E. Cipient \" --stdout master..side | sed -e \"/^\\$/q\" >patch6 &&\n-\tgrep \"^Cc: R. E. Cipient \\$\" patch6\n+\tgit format-patch --add-header=\"Cc: R E Cipient \" --stdout master..side | sed -e \"/^\\$/q\" >patch6 &&\n+\tgrep \"^Cc: R E Cipient \\$\" patch6\n '\n \n test_expect_success 'configuration headers and command line headers' '\n \n-\tgit config --replace-all format.headers \"Cc: R. E. Cipient \" &&\n-\tgit format-patch --add-header=\"Cc: S. E. Cipient \" --stdout master..side | sed -e \"/^\\$/q\" >patch7 &&\n-\tgrep \"^Cc: R. E. Cipient ,\\$\" patch7 &&\n-\tgrep \"^ *S. E. Cipient \\$\" patch7\n+\tgit config --replace-all format.headers \"Cc: R E Cipient \" &&\n+\tgit format-patch --add-header=\"Cc: S E Cipient \" --stdout master..side | sed -e \"/^\\$/q\" >patch7 &&\n+\tgrep \"^Cc: R E Cipient ,\\$\" patch7 &&\n+\tgrep \"^ *S E Cipient \\$\" patch7\n '\n \n-test_expect_success 'command line To: header' '\n+test_expect_success 'command line To: header (ascii)' '\n \n \tgit config --unset-all format.headers &&\n+\tgit format-patch --to=\"R E Cipient \" --stdout master..side | sed -e \"/^\\$/q\" >patch8 &&\n+\tgrep \"^To: R E Cipient \\$\" patch8\n+'\n+\n+test_expect_failure 'command line To: header (rfc822)' '\n+\n \tgit format-patch --to=\"R. E. Cipient \" --stdout master..side | sed -e \"/^\\$/q\" >patch8 &&\n-\tgrep \"^To: R. E. Cipient \\$\" patch8\n+\tgrep \"^To: \"R. E. Cipient\" \\$\" patch8\n+'\n+\n+test_expect_failure 'command line To: header (rfc2047)' '\n+\n+\tgit format-patch --to=\"R Ä Cipient \" --stdout master..side | sed -e \"/^\\$/q\" >patch8 &&\n+\tgrep \"^To: =?UTF-8?q?R=20=C3=84=20Cipient?= \\$\" patch8\n '\n \n-test_expect_success 'configuration To: header' '\n+test_expect_success 'configuration To: header (ascii)' '\n+\n+\tgit config format.to \"R E Cipient \" &&\n+\tgit format-patch --stdout master..side | sed -e \"/^\\$/q\" >patch9 &&\n+\tgrep \"^To: R E Cipient \\$\" patch9\n+'\n+\n+test_expect_failure 'configuration To: header (rfc822)' '\n \n \tgit config format.to \"R. E. Cipient \" &&\n \tgit format-patch --stdout master..side | sed -e \"/^\\$/q\" >patch9 &&\n-\tgrep \"^To: R. E. Cipient \\$\" patch9\n+\tgrep \"^To: \"R. E. Cipient\" \\$\" patch9\n+'\n+\n+test_expect_failure 'configuration To: header (rfc2047)' '\n+\n+\tgit config format.to \"R Ä Cipient \" &&\n+\tgit format-patch --stdout master..side | sed -e \"/^\\$/q\" >patch9 &&\n+\tgrep \"^To: =?UTF-8?q?R=20=C3=84=20Cipient?= \\$\" patch9\n '\n \n # check_patch : Verify that looks like a half-sane\n@@ -190,11 +224,11 @@ check_patch () {\n test_expect_success '--no-to overrides config.to' '\n \n \tgit config --replace-all format.to \\\n-\t\t\"R. E. Cipient \" &&\n+\t\t\"R E Cipient \" &&\n \tgit format-patch --no-to --stdout master..side |\n \tsed -e \"/^\\$/q\" >patch10 &&\n \tcheck_patch patch10 &&\n-\t! grep \"^To: R. E. Cipient \\$\" patch10\n+\t! grep \"^To: R E Cipient \\$\" patch10\n '\n \n test_expect_success '--no-to and --to replaces config.to' '\n@@ -212,21 +246,21 @@ test_expect_success '--no-to and --to replaces config.to' '\n test_expect_success '--no-cc overrides config.cc' '\n \n \tgit config --replace-all format.cc \\\n-\t\t\"C. E. Cipient \" &&\n+\t\t\"C E Cipient \" &&\n \tgit format-patch --no-cc --stdout master..side |\n \tsed -e \"/^\\$/q\" >patch12 &&\n \tcheck_patch patch12 &&\n-\t! grep \"^Cc: C. E. Cipient \\$\" patch12\n+\t! grep \"^Cc: C E Cipient \\$\" patch12\n '\n \n test_expect_success '--no-add-header overrides config.headers' '\n \n \tgit config --replace-all format.headers \\\n-\t\t\"Header1: B. E. Cipient \" &&\n+\t\t\"Header1: B E Cipient \" &&\n \tgit format-patch --no-add-header --stdout master..side |\n \tsed -e \"/^\\$/q\" >patch13 &&\n \tcheck_patch patch13 &&\n-\t! grep \"^Header1: B. E. Cipient \\$\" patch13\n+\t! grep \"^Header1: B E Cipient \\$\" patch13\n '\n \n test_expect_success 'multiple files' '\n-- \n1.7.12\n"},{"id":"200894","messageId":"5075408F.8050502@cs.tu-berlin.de","threadId":"31753","inReplyTo":"7v7gqzfnpj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/5] format-patch: do not wrap rfc2047 encoded headers too late","fromName":"Jan H. Schönherr","fromEmail":"schnhrr@cs.tu-berlin.de","sentAt":"2012-10-10T09:31:59Z","receivedAt":"2012-10-10T09:31:59Z","isPatch":true,"sender":{"key":"schnhrr@cs.tu-berlin.de","avatar":null},"body":"Am 09.10.2012 21:30, schrieb Junio C Hamano:\n> Jan H. Schönherr writes:\n...\n>> static int is_rfc2047_special(char ch)\n>> {\n>> +\t/*\n>> +\t * We encode ' ' using '=20' even though rfc2047\n>> +\t * allows using '_' for readability. Unfortunately,\n>> +\t * many programs do not understand this and just\n>> +\t * leave the underscore in place.\n>> +\t */\n> \n> The sentence break made me read the above three times to understand\n> what it is trying to say. \"Unfortunately\" refers to what happens if\n> we were to use '_', but it initially appeared to be describing some\n> bug due to our encoding ' ' as '=20'. Perhaps like this?\n> \n> \t/*\n> \t * rfc2047 allows '_' to encode ' ' for readability, but\n> \t * many programs do not understand ...; encode ' ' using\n> \t * '=20' instead to avoid the problem.\n> \t */\n\nI was just moving that comment (and the following check) around,\nbut I'll update the comment in the next version.\n\n>> +\tif (ch == ' ' || ch == '\\n')\n>> +\t\treturn 1;\n> \n> The comment justifies why this \"if (ch == ' ')\", which could be part\n> of the \"return\" below, separately is done, but nothing explains why\n> you add '\\n' (and not other controls, e.g. '\\t') to the mix.\n\nThe check for '\\n' was introduced in commit c22e7de3\n(\"format-patch: rfc2047-encode newlines in headers\").\n\nThe commit log was:\n\n These should generally never happen, as we already\n concatenate multiples in subjects into a single line. But\n let's be defensive, since not encoding them means we will\n output malformed headers.\n\nHaving again a look at RFC 2047, I see that we should be\neven more strict and not allow any non-printable character to\nbe passed through unencoded. I guess that adds another patch to\nthe series. Hmm... Maybe I can split patch 4 into two patches,\none that mostly fixes is_rfc2047_special() and one that\navoids 822 quoting when doing 2047 encoding.\n\n> \n>> \treturn (non_ascii(ch) || (ch == '=') || (ch == '?') || (ch == '_'));\n>> }\n>> \n>> static void add_rfc2047(struct strbuf *sb, const char *line, int len,\n>> \t\t const char *encoding)\n>> {\n>> -\tstatic const int max_length = 78; /* per rfc2822 */\n>> +\tstatic const int max_length = 76; /* per rfc2047 */\n>> \tint i;\n>> \tint line_len;\n>> \n>> @@ -286,7 +295,7 @@ static void add_rfc2047(struct strbuf *sb, const char *line, int len,\n>> \t\tif ((i + 1 < len) && (ch == '=' && line[i+1] == '?'))\n>> \t\t\tgoto needquote;\n>> \t}\n>> -\tstrbuf_add_wrapped_bytes(sb, line, len, -line_len, 1, max_length+1);\n>> +\tstrbuf_add_wrapped_bytes(sb, line, len, -line_len, 1, 78+1);\n>> \treturn;\n> \n> Yuck. If you do want to retain 78 for non-quoted output for\n> backward compatibility, that is OK, but if that is the case, please\n> introduce a new constant \"max_quoted_length\" or something to stand\n> for 76 and use it in the \"needquote:\" part below.\n\nWill do.\n\nRegards\nJan\n"},{"id":"200910","messageId":"50755170.1080205@cs.tu-berlin.de","threadId":"31753","inReplyTo":"7v391nfmzn.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 5/5] format-patch: tests: check rfc822+rfc2047 in to+cc headers","fromName":"Jan H. Schönherr","fromEmail":"schnhrr@cs.tu-berlin.de","sentAt":"2012-10-10T10:44:00Z","receivedAt":"2012-10-10T10:44:00Z","isPatch":true,"sender":{"key":"schnhrr@cs.tu-berlin.de","avatar":null},"body":"Am 09.10.2012 21:45, schrieb Junio C Hamano:\n> Jan H. Schönherr writes:\n> \n>> +test_expect_failure 'additional command line cc (rfc822)' '\n>> +\n>> +\tgit config --replace-all format.headers \"Cc: R E Cipient \" &&\n>> \tgit format-patch --cc=\"S. E. Cipient \" --stdout master..side | sed -e \"/^\\$/q\" >patch5 &&\n>> -\tgrep \"^Cc: R. E. Cipient ,\\$\" patch5 &&\n>> -\tgrep \"^ *S. E. Cipient \\$\" patch5\n>> +\tgrep \"^Cc: R E Cipient ,\\$\" patch5 &&\n>> +\tgrep \"^ *\"S. E. Cipient\" \\$\" patch5\n> \n> Hrm.\n> \n> As we are not in the business of parsing out whatever junk given\n> with --cc or --recipient from the command line or configuration, but\n> are merely parroting them to the output stream, isn't this a\n> user-error in the test that gives --cc='S. E. Cipient '\n> instead of giving --cc='\"S. E. Cipient\" '? Same comment\n> on the new 'expect-failure' tests.\n\nOriginally, I just wanted to emphasize, that --to and --cc are\ncurrently handled differently than in git-send-email, where\nall this quoting/encoding is done.\n\nAnd it is much more convenient to add\n\t--cc 'Jan H. Schönherr <...>'\nthan\n\t--cc '=?UTF-8?q?Jan=20H=2E=20Sch=C3=B6nherr?= <...>'\n\nEven more, since I would expect git to correctly handle\naddresses given in a format that is also used elsewhere\nwithin git.\n\n\nHowever, I agree that we are not responsible to check/quote/encode\nanything when the user supplies whole headers (though we probably\ncould).\n\n\nBut if I cannot convince you, I'll just drop this patch. :)\n\nRegards\nJan\n"},{"id":"200911","messageId":"507552C8.2020402@cs.tu-berlin.de","threadId":"31753","inReplyTo":"7vfw5nfoq9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/5] Cure some format-patch wrapping and encoding issues","fromName":"Jan H. Schönherr","fromEmail":"schnhrr@cs.tu-berlin.de","sentAt":"2012-10-10T10:49:44Z","receivedAt":"2012-10-10T10:49:44Z","isPatch":true,"sender":{"key":"schnhrr@cs.tu-berlin.de","avatar":null},"body":"Am 09.10.2012 21:07, schrieb Junio C Hamano:\n> Jan H. Schönherr writes:\n> \n>> During the creation of this series, I came across the strbuf \n>> wrapping functions, and I wonder if there is an off-by-one issue.\n>>\n>> Consider the following excerpt from t4202:\n...\n> \n> Yeah, that does sound like an off-by-one bug. When we as end users\n> say %w(72), we do expect some lines fill to the 72nd column, not\n> stopping at the 71st. I suspect that dates back to the very first\n> implementation of %w() but I think we should fix it (perhaps as a\n> separate patch either the earliest or the last in the series).\n\nI will include a fix for that, then.\n\n(But I won't be able the send out the next round of this series\nbefore next week.)\n\nRegards\nJan\n"},{"id":"200930","messageId":"7vhaq2clb3.fsf@alter.siamese.dyndns.org","threadId":"31753","inReplyTo":"50755170.1080205@cs.tu-berlin.de","subject":"Re: [PATCH 5/5] format-patch: tests: check rfc822+rfc2047 in to+cc headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-10T17:02:24Z","receivedAt":"2012-10-10T17:02:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jan H. Schönherr\" writes:\n\n> Am 09.10.2012 21:45, schrieb Junio C Hamano:\n>> Jan H. Schönherr writes:\n>> \n>>> +test_expect_failure 'additional command line cc (rfc822)' '\n>>> +\n>>> +\tgit config --replace-all format.headers \"Cc: R E Cipient \" &&\n>>> \tgit format-patch --cc=\"S. E. Cipient \" --stdout master..side | sed -e \"/^\\$/q\" >patch5 &&\n>>> -\tgrep \"^Cc: R. E. Cipient ,\\$\" patch5 &&\n>>> -\tgrep \"^ *S. E. Cipient \\$\" patch5\n>>> +\tgrep \"^Cc: R E Cipient ,\\$\" patch5 &&\n>>> +\tgrep \"^ *\"S. E. Cipient\" \\$\" patch5\n>> \n>> Hrm.\n>> \n>> As we are not in the business of parsing out whatever junk given\n>> with --cc or --recipient from the command line or configuration, but\n>> are merely parroting them to the output stream, isn't this a\n>> user-error in the test that gives --cc='S. E. Cipient '\n>> instead of giving --cc='\"S. E. Cipient\" '? Same comment\n>> on the new 'expect-failure' tests.\n>\n> Originally, I just wanted to emphasize, that --to and --cc are\n> currently handled differently than in git-send-email, where\n> all this quoting/encoding is done.\n>\n> And it is much more convenient to add\n> \t--cc 'Jan H. Schönherr <...>'\n> than\n> \t--cc '=?UTF-8?q?Jan=20H=2E=20Sch=C3=B6nherr?= <...>'\n>\n> Even more, since I would expect git to correctly handle\n> addresses given in a format that is also used elsewhere\n> within git.\n>\n>\n> However, I agree that we are not responsible to check/quote/encode\n> anything when the user supplies whole headers (though we probably\n> could).\n>\n> But if I cannot convince you, I'll just drop this patch. :)\n\nIt wasn't about convincing or not convincing me.\n\nI couldn't read, just from \"expect_failure\" and \"Do some checks\nfor...\", what the intention of the tests and the proposed future\nplans were.\n\nIf the proposed commit log message (or comments before these\n\"expect_failure\" tests) said something like this:\n\n \"git send-email\" historically did not parse the user supplied\n extra header values (e.g. --cc, --recipient) and just replayed\n them, but that forces users to add them in encoded form, e.g.\n\n \t--cc '=?UTF-8?q?Jan=20H=2E=20Sch=C3=B6nherr?= <...>'\n\n which is inconvenient. We would want to update send-email to\n accept human-readable\n\n \t--cc 'Jan H. Schönherr <...>'\n\n and encode in the future. Add test_expect_failure tests as a\n reminder.\n\nthat would have avoided such confusion, and even more importantly,\nmade it easier for us to start discussion on the proposed future\ndirection. I am personally on the fence.\n"}]}