{"thread":{"id":"26492","subject":"[PATCH] generate a valid rfc2047 mail header for multi-line subject.","startedAt":"2011-02-14T08:09:28Z","lastAt":"2011-02-24T07:34:16Z","messageCount":15,"participants":["xzer","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"161033","messageId":"1297670968-28130-1-git-send-email-xiaozhu@gmail.com","threadId":"26492","inReplyTo":null,"subject":"[PATCH] generate a valid rfc2047 mail header for multi-line subject.","fromName":"xzer","fromEmail":"xiaozhu@gmail.com","sentAt":"2011-02-14T08:09:28Z","receivedAt":"2011-02-14T08:09:28Z","isPatch":true,"sender":{"key":"xiaozhu@gmail.com","avatar":null},"body":"There is still a problem that git-am will lost the line break.\nIt's not easy to retain it, but as the first step, we can generate\na valid rfc2047 header now.\n---\n pretty.c |   29 ++++++++++++++++++++++++++++-\n 1 files changed, 28 insertions(+), 1 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 8549934..f18a38d 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -249,6 +249,33 @@ needquote:\n \tstrbuf_addstr(sb, \"?=\");\n }\n \n+static void add_rfc2047_multiline(struct strbuf *sb, const char *line, int len,\n+                       const char *encoding)\n+{\n+\tint first = 1;\n+\tchar *mline = xmemdupz(line, len);\n+\tconst char *cline = mline;\n+\tint offset = 0, linelen = 0;\n+        for (;;) {\n+                linelen = get_one_line(cline);\n+\n+                cline += linelen;\n+\n+                if (!linelen)\n+                        break;\n+\t\t\n+\t\tif (!first)\n+\t\t\tstrbuf_addf(sb, \"\\n \");\n+\n+\t\toffset = *(cline -1) == '\\n'; \n+\n+\t\tadd_rfc2047(sb, cline-linelen, linelen-offset, encoding);\n+\t\tfirst = 0;\n+\n+        }\n+\tfree(mline);\n+}\n+\n void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,\n \t\t  const char *line, enum date_mode dmode,\n \t\t  const char *encoding)\n@@ -1115,7 +1142,7 @@ void pp_title_line(enum cmit_fmt fmt,\n \tstrbuf_grow(sb, title.len + 1024);\n \tif (subject) {\n \t\tstrbuf_addstr(sb, subject);\n-\t\tadd_rfc2047(sb, title.buf, title.len, encoding);\n+\t\tadd_rfc2047_multiline(sb, title.buf, title.len, encoding);\n \t} else {\n \t\tstrbuf_addbuf(sb, &title);\n \t}\n-- \n1.7.4.52.g00e6e.dirty\n"},{"id":"161913","messageId":"7vsjvfby0z.fsf@alter.siamese.dyndns.org","threadId":"26492","inReplyTo":"1297670968-28130-1-git-send-email-xiaozhu@gmail.com","subject":"Re: [PATCH] generate a valid rfc2047 mail header for multi-line subject.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-22T20:43:40Z","receivedAt":"2011-02-22T20:43:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"xzer <xiaozhu@gmail.com> writes:\n\n> Subject: Re: [PATCH] generate a valid rfc2047 mail header for multi-line subject.\n\nWe prefer to have \"[PATCH] subsystem: description without final full-stop\" here.\n\n> There is still a problem that git-am will lost the line break.\n\nWhat does \"still\" refer to?  It is unclear under what condition the\ncommand lose \"the line break\" (nor which line break you are refering to; I\nam guessing that you have a commit that begins with a multi-line paragraph\nand you are talking about line breaks between the lines in the first\nparagraph).\n\n> It's not easy to retain it, but as the first step, we can generate\n> a valid rfc2047 header now.\n\nPlease describe what is broken (iow, \"Given this sample input, we\ncurrently generate this output, which is not a valid rfc2047\") and what\nthe new output looks like (\"Update pp_title_line() to generate this output\ninstead.\")\n\n> ---\n\nMissing sign-off with a real name.\n\n> diff --git a/pretty.c b/pretty.c\n> index 8549934..f18a38d 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -249,6 +249,33 @@ needquote:\n>  \tstrbuf_addstr(sb, \"?=\");\n>  }\n>  \n> +static void add_rfc2047_multiline(struct strbuf *sb, const char *line, int len,\n> +                       const char *encoding)\n> +{\n> +\tint first = 1;\n> +\tchar *mline = xmemdupz(line, len);\n> +\tconst char *cline = mline;\n> +\tint offset = 0, linelen = 0;\n> +        for (;;) {\n\nYou seem to have indent that uses SPs instead of HT around here...\n\n> +                linelen = get_one_line(cline);\n\nI can see you are trying to be careful not to let get_one_line() overstep\npast \"len\" the caller gave you by making a copy first, but is this\noverhead really necessary?  After all we know in this static function that\nthe caller is feeding the contents from a strbuf, which always have a\nterminating NUL (and that is why it is Ok that get_one_line() is not a\ncounted string interface).\n\n> +\n> +                cline += linelen;\n> +\n> +                if (!linelen)\n> +                        break;\n> +\t\t\n> +\t\tif (!first)\n> +\t\t\tstrbuf_addf(sb, \"\\n \");\n> +\n> +\t\toffset = *(cline -1) == '\\n'; \n> +\n> +\t\tadd_rfc2047(sb, cline-linelen, linelen-offset, encoding);\n> +\t\tfirst = 0;\n> +\n> +        }\n> +\tfree(mline);\n> +}\n\nSo the general idea of this change (I am thinking aloud what should be in\nthe updated commit log message as the problem description) is that:\n\n - We currently give an entire multi-line paragraph string to the\n   add_rfc2047() function to be formatted as the title of the commit;\n\n - The add_rfc2047() functionjust passes \"\\n\" through, without making it a\n   folding whitespace followed by a newline, to help callers that want to\n   use this function to produce a header line that is rfc 2822 conformant;\n\n - The patch introduces a new function add_rfc2047_multiline() that splits\n   its input and performs line folding for such a caller (namely, the\n   pp_title_line() function);\n\n - Another caller of add_rfc2047(), pp_user_info, is not changed, and it\n   won't fold the name of the user that appear on the From: line.\n\nIt is unclear if the last point is really the right thing to do, though.\nIt is not a new problem that an author name that has a \"\\n\" in it would\nbreak the output, but we probably would want to fix that case too here?\n"},{"id":"162007","messageId":"20110223080854.GB2724@sigill.intra.peff.net","threadId":"26492","inReplyTo":"7vsjvfby0z.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] generate a valid rfc2047 mail header for multi-line subject.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-23T08:08:54Z","receivedAt":"2011-02-23T08:08:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 22, 2011 at 12:43:40PM -0800, Junio C Hamano wrote:\n\n> So the general idea of this change (I am thinking aloud what should be in\n> the updated commit log message as the problem description) is that:\n> \n>  - We currently give an entire multi-line paragraph string to the\n>    add_rfc2047() function to be formatted as the title of the commit;\n> \n>  - The add_rfc2047() functionjust passes \"\\n\" through, without making it a\n>    folding whitespace followed by a newline, to help callers that want to\n>    use this function to produce a header line that is rfc 2822 conformant;\n> \n>  - The patch introduces a new function add_rfc2047_multiline() that splits\n>    its input and performs line folding for such a caller (namely, the\n>    pp_title_line() function);\n> \n>  - Another caller of add_rfc2047(), pp_user_info, is not changed, and it\n>    won't fold the name of the user that appear on the From: line.\n> \n> It is unclear if the last point is really the right thing to do, though.\n> It is not a new problem that an author name that has a \"\\n\" in it would\n> break the output, but we probably would want to fix that case too here?\n\nYeah, I think the best path forward is:\n\n  1. Stop feeding \"pre-folded\" subject lines to the email formatter.\n     Give it the regular subject line with no newlines.\n\n  2. rfc2047 encoding should encode a literal newline. Which should\n     generally never happen, but is probably the most sane thing to do\n     if it does.\n\n  3. rfc2047 should fold all lines at some sane length. As it is now, we\n     may sometimes generate long lines in headers (though in practice, I\n     doubt this is much of a problem).\n\nI started to work on this, but got stuck on (3). Our existing wrap\nfunctions want NUL-terminated strings, and we are operating on a\nsubstring. I tried converting the wrap functions to handle lengths, but\nit got way uglier than I had hoped. I think just strdup'ing the subject\ntemporarily is probably fine, though. Let me see what I can come up\nwith.\n\n-Peff\n"},{"id":"162011","messageId":"20110223094844.GA9205@sigill.intra.peff.net","threadId":"26492","inReplyTo":"20110223080854.GB2724@sigill.intra.peff.net","subject":"Re: [PATCH] generate a valid rfc2047 mail header for multi-line subject.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-23T09:48:45Z","receivedAt":"2011-02-23T09:48:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 23, 2011 at 03:08:54AM -0500, Jeff King wrote:\n\n> Yeah, I think the best path forward is:\n> \n>   1. Stop feeding \"pre-folded\" subject lines to the email formatter.\n>      Give it the regular subject line with no newlines.\n> \n>   2. rfc2047 encoding should encode a literal newline. Which should\n>      generally never happen, but is probably the most sane thing to do\n>      if it does.\n> \n>   3. rfc2047 should fold all lines at some sane length. As it is now, we\n>      may sometimes generate long lines in headers (though in practice, I\n>      doubt this is much of a problem).\n\nSo here is a series that does this. It still doesn't preserve subject\nnewlines in \"format-patch | am\", but I don't think that was ever a goal\nof the code. If we want to add it as an optional feature on top (maybe\nas part of \"-k\"?), it should be easy to do (since the rfc2047 encoding\nwill now preserve embedded newlines).\n\n  [1/3]: strbuf: add fixed-length version of add_wrapped_text\n  [2/3]: format-patch: wrap long header lines\n  [3/3]: format-patch: rfc2047-encode newlines in headers\n\n-Peff\n"},{"id":"162012","messageId":"20110223095018.GA9222@sigill.intra.peff.net","threadId":"26492","inReplyTo":"20110223094844.GA9205@sigill.intra.peff.net","subject":"[PATCH 1/3] strbuf: add fixed-length version of add_wrapped_text","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-23T09:50:19Z","receivedAt":"2011-02-23T09:50:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The function strbuf_add_wrapped_text takes a NUL-terminated\nstring. This makes it annoying to wrap strings we have as a\npointer and a length.\n\nRefactoring strbuf_add_wrapped_text and all of its\nsub-functions to handle fixed-length strings turned out to\nbe really ugly. So this implementation is lame; it just\nstrdups the text and operates on the NUL-terminated version.\nThis should be fine as the strings we are wrapping are\ngenerally pretty short.  If it becomes a problem, we can\noptimize later.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n utf8.c |    9 +++++++++\n utf8.h |    2 ++\n 2 files changed, 11 insertions(+), 0 deletions(-)\n\ndiff --git a/utf8.c b/utf8.c\nindex 84cfc72..8acbc66 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -405,6 +405,15 @@ new_line:\n \t}\n }\n \n+int strbuf_add_wrapped_bytes(struct strbuf *buf, const char *data, int len,\n+\t\t\t     int indent, int indent2, int width)\n+{\n+\tchar *tmp = xstrndup(data, len);\n+\tint r = strbuf_add_wrapped_text(buf, tmp, indent, indent2, width);\n+\tfree(tmp);\n+\treturn r;\n+}\n+\n int is_encoding_utf8(const char *name)\n {\n \tif (!name)\ndiff --git a/utf8.h b/utf8.h\nindex ebc4d2f..81f2c82 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -10,6 +10,8 @@ int is_encoding_utf8(const char *name);\n \n int strbuf_add_wrapped_text(struct strbuf *buf,\n \t\tconst char *text, int indent, int indent2, int width);\n+int strbuf_add_wrapped_bytes(struct strbuf *buf, const char *data, int len,\n+\t\t\t     int indent, int indent2, int width);\n \n #ifndef NO_ICONV\n char *reencode_string(const char *in, const char *out_encoding, const char *in_encoding);\n-- \n1.7.2.5.15.gfdd1c\n"},{"id":"162014","messageId":"20110223095841.GB9222@sigill.intra.peff.net","threadId":"26492","inReplyTo":"20110223094844.GA9205@sigill.intra.peff.net","subject":"[PATCH 2/3] format-patch: wrap long header lines","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-23T09:58:41Z","receivedAt":"2011-02-23T09:58:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Subject and identity headers may be arbitrarily long. In the\npast, we just assumed that single-line headers would be\nreasonably short. For multi-line subjects that we squish\ninto a single line, we just \"pre-folded\" the data in\npp_title_line by adding a newline and indentation.\n\nThere were two problems. One is that, although rare,\nsingle-line messages can actually be longer than the\nrecommended line-length limits. The second is that the\npre-folding interacted badly with rfc2047 encoding, leading\nto malformed headers.\n\nInstead, let's stop pre-folding the subject lines, and just\nfold everything based on length in add_rfc2047, whether\nit is encoded or not.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThree things to note:\n\n  1. We call strbuf_add_wrapped_bytes for the non-encoded case. This\n     nicely wraps on word and multi-character boundaries. But it will\n     never do a \"hard\" wrap if there are no word boundaries, and it\n     probably should at the 998-character mark which rfc2822 specifies\n     as a hard limit.\n\n     I don't know how much we care. For something like that you'd have\n     to be maliciously trying to create a bogus patch. If you're mailing\n     it, you could just create the bogus mail by hand. If you're trying\n     to buffer overflow somebody's \"format-patch | am\" script, it won't\n     do anything, as mailinfo does not have a limit on line length.\n\n  2. For the non-quoted case, technically we want to indent less on the\n     first line than we do on subsequent lines (to account for \"Subject:\n     [PATCH]\"). strbuf_add_wrapped_bytes doesn't support that notion. We\n     could add it, but it probably doesn't matter. We just end up\n     wrapping the subsequent lines a little tighter than we need to.\n\n  3. I used RFC2822's SHOULD value of 78 characters as a line length.\n     That's probably unnecessarily conservative. In theory wrapping\n     shouldn't make a difference to the data, but maybe people who\n     hand-edit the result would prefer not to see wrapping? I dunno. In\n     that case, we could set it to something higher like 120, which\n     would still wrap the really ridiculous cases.\n\n pretty.c                |   32 +++++++++++++----\n t/t4014-format-patch.sh |   84 +++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 108 insertions(+), 8 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 8549934..0e167f4 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -216,7 +216,15 @@ static int is_rfc2047_special(char ch)\n static void add_rfc2047(struct strbuf *sb, const char *line, int len,\n \t\t       const char *encoding)\n {\n-\tint i, last;\n+\tstatic const int max_length = 78; /* per rfc2822 */\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 \n \tfor (i = 0; i < len; i++) {\n \t\tint ch = line[i];\n@@ -225,14 +233,21 @@ 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(sb, line, len);\n+\tstrbuf_add_wrapped_bytes(sb, line, len, 0, 1, max_length - line_len);\n \treturn;\n \n needquote:\n \tstrbuf_grow(sb, len * 3 + strlen(encoding) + 100);\n \tstrbuf_addf(sb, \"=?%s?q?\", encoding);\n-\tfor (i = last = 0; i < len; i++) {\n+\tline_len += strlen(encoding) + 5; /* 5 for =??q? */\n+\tfor (i = 0; i < len; i++) {\n \t\tunsigned ch = line[i] & 0xFF;\n+\n+\t\tif (line_len >= max_length - 2) {\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@@ -240,12 +255,14 @@ needquote:\n \t\t * leave the underscore in place.\n \t\t */\n \t\tif (is_rfc2047_special(ch) || ch == ' ') {\n-\t\t\tstrbuf_add(sb, line + last, i - last);\n \t\t\tstrbuf_addf(sb, \"=%02X\", ch);\n-\t\t\tlast = i + 1;\n+\t\t\tline_len += 3;\n+\t\t}\n+\t\telse {\n+\t\t\tstrbuf_addch(sb, ch);\n+\t\t\tline_len++;\n \t\t}\n \t}\n-\tstrbuf_add(sb, line + last, len - last);\n \tstrbuf_addstr(sb, \"?=\");\n }\n \n@@ -1106,11 +1123,10 @@ void pp_title_line(enum cmit_fmt fmt,\n \t\t   const char *encoding,\n \t\t   int need_8bit_cte)\n {\n-\tconst char *line_separator = (fmt == CMIT_FMT_EMAIL) ? \"\\n \" : \" \";\n \tstruct strbuf title;\n \n \tstrbuf_init(&title, 80);\n-\t*msg_p = format_subject(&title, *msg_p, line_separator);\n+\t*msg_p = format_subject(&title, *msg_p, \" \");\n \n \tstrbuf_grow(sb, title.len + 1024);\n \tif (subject) {\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 027c13d..9c66367 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -709,4 +709,88 @@ test_expect_success TTY 'format-patch --stdout paginates' '\n \ttest_path_is_missing .git/pager_used\n '\n \n+test_expect_success 'format-patch handles multi-line subjects' '\n+\trm -rf patches/ &&\n+\techo content >>file &&\n+\tfor i in one two three; do echo $i; done >msg &&\n+\tgit add file &&\n+\tgit commit -F msg &&\n+\tgit format-patch -o patches -1 &&\n+\tgrep ^Subject: patches/0001-one.patch >actual &&\n+\techo \"Subject: [PATCH] one two three\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'format-patch handles multi-line encoded subjects' '\n+\trm -rf patches/ &&\n+\techo content >>file &&\n+\tfor i in en två tre; do echo $i; done >msg &&\n+\tgit add file &&\n+\tgit commit -F msg &&\n+\tgit format-patch -o patches -1 &&\n+\tgrep ^Subject: patches/0001-en.patch >actual &&\n+\techo \"Subject: [PATCH] =?UTF-8?q?en=20tv=C3=A5=20tre?=\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+M8=\"foo 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] 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+EOF\n+test_expect_success 'format-patch wraps extremely long headers (ascii)' '\n+\techo content >>file &&\n+\tgit add file &&\n+\tgit commit -m \"$M512\" &&\n+\tgit format-patch --stdout -1 >patch &&\n+\tsed -n \"/^Subject/p; /^ /p; /^$/q\" <patch >subject &&\n+\ttest_cmp expect subject\n+'\n+\n+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+EOF\n+test_expect_success 'format-patch wraps extremely long headers (rfc2047)' '\n+\trm -rf patches/ &&\n+\techo content >>file &&\n+\tgit add file &&\n+\tgit commit -m \"$M512\" &&\n+\tgit format-patch --stdout -1 >patch &&\n+\tsed -n \"/^Subject/p; /^ /p; /^$/q\" <patch >subject &&\n+\ttest_cmp expect subject\n+'\n+\n test_done\n-- \n1.7.2.5.15.gfdd1c\n"},{"id":"162015","messageId":"20110223095917.GC9222@sigill.intra.peff.net","threadId":"26492","inReplyTo":"20110223094844.GA9205@sigill.intra.peff.net","subject":"[PATCH 3/3] format-patch: rfc2047-encode newlines in headers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-23T09:59:18Z","receivedAt":"2011-02-23T09:59:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"These should generally never happen, as we already\nconcatenate multiples in subjects into a single line. But\nlet's be defensive, since not encoding them means we will\noutput malformed headers.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n pretty.c |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 0e167f4..65d20a7 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -228,7 +228,7 @@ static void add_rfc2047(struct strbuf *sb, const char *line, int len,\n \n \tfor (i = 0; i < len; i++) {\n \t\tint ch = line[i];\n-\t\tif (non_ascii(ch))\n+\t\tif (non_ascii(ch) || ch == '\\n')\n \t\t\tgoto needquote;\n \t\tif ((i + 1 < len) && (ch == '=' && line[i+1] == '?'))\n \t\t\tgoto needquote;\n@@ -254,7 +254,7 @@ needquote:\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 == ' ') {\n+\t\tif (is_rfc2047_special(ch) || ch == ' ' || ch == '\\n') {\n \t\t\tstrbuf_addf(sb, \"=%02X\", ch);\n \t\t\tline_len += 3;\n \t\t}\n-- \n1.7.2.5.15.gfdd1c\n"},{"id":"162027","messageId":"AANLkTimUXqKdTDcSVDK44XPhxWbHtQuDWHMED3PKqWE4@mail.gmail.com","threadId":"26492","inReplyTo":"20110223094844.GA9205@sigill.intra.peff.net","subject":"Re: [PATCH] generate a valid rfc2047 mail header for multi-line subject.","fromName":"xzer","fromEmail":"xiaozhu@gmail.com","sentAt":"2011-02-23T15:16:04Z","receivedAt":"2011-02-23T15:16:04Z","isPatch":true,"sender":{"key":"xiaozhu@gmail.com","avatar":null},"body":"2011/2/23 Jeff King <peff@peff.net>:\n> On Wed, Feb 23, 2011 at 03:08:54AM -0500, Jeff King wrote:\n>\n>> Yeah, I think the best path forward is:\n>>\n>>   1. Stop feeding \"pre-folded\" subject lines to the email formatter.\n>>      Give it the regular subject line with no newlines.\n>>\n>>   2. rfc2047 encoding should encode a literal newline. Which should\n>>      generally never happen, but is probably the most sane thing to do\n>>      if it does.\n>>\n>>   3. rfc2047 should fold all lines at some sane length. As it is now, we\n>>      may sometimes generate long lines in headers (though in practice, I\n>>      doubt this is much of a problem).\n>\n> So here is a series that does this. It still doesn't preserve subject\n> newlines in \"format-patch | am\", but I don't think that was ever a goal\n> of the code. If we want to add it as an optional feature on top (maybe\n> as part of \"-k\"?), it should be easy to do (since the rfc2047 encoding\n> will now preserve embedded newlines).\n>\n>  [1/3]: strbuf: add fixed-length version of add_wrapped_text\n>  [2/3]: format-patch: wrap long header lines\n>  [3/3]: format-patch: rfc2047-encode newlines in headers\n>\n> -Peff\n>\n\nTo the first point, I really want to find a way that we can remain the\nline breaker\nafter import a formatted patch. That's why I add a new function to product multi\nline header, I want to do something which is special to subject. In my usage,\nI told my men every day that don't write too long in the first\nparagraph, but there\nare always somebody who forgets it, then I will get a patch with a\nvery long subject\njust like a nightmare(yes, I gave them my temporary fix which I submitted here,\nso they can write as long as they want).\n\nSo I want to know whether we can generate a 2047 compatible header so\nthat mailer\ncan catch it correctly and the git-am can import it with line breaker\ncorrectly too.\n"},{"id":"162028","messageId":"AANLkTinf6P-erY-9p5WPWbK+uAf1hozvAutV0zPSpHGQ@mail.gmail.com","threadId":"26492","inReplyTo":"7vsjvfby0z.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] generate a valid rfc2047 mail header for multi-line subject.","fromName":"xzer","fromEmail":"xiaozhu@gmail.com","sentAt":"2011-02-23T15:34:54Z","receivedAt":"2011-02-23T15:34:54Z","isPatch":true,"sender":{"key":"xiaozhu@gmail.com","avatar":null},"body":"2011/2/23 Junio C Hamano <gitster@pobox.com>:\n> xzer <xiaozhu@gmail.com> writes:\n>\n>> Subject: Re: [PATCH] generate a valid rfc2047 mail header for multi-line subject.\n>\n> We prefer to have \"[PATCH] subsystem: description without final full-stop\" here.\n>\n>> There is still a problem that git-am will lost the line break.\n>\n> What does \"still\" refer to?  It is unclear under what condition the\n> command lose \"the line break\" (nor which line break you are refering to; I\n> am guessing that you have a commit that begins with a multi-line paragraph\n> and you are talking about line breaks between the lines in the first\n> paragraph).\n>\n\nYes, that is what I am refering, the line breaks in the first paragraph.\n\n>> It's not easy to retain it, but as the first step, we can generate\n>> a valid rfc2047 header now.\n>\n> Please describe what is broken (iow, \"Given this sample input, we\n> currently generate this output, which is not a valid rfc2047\") and what\n> the new output looks like (\"Update pp_title_line() to generate this output\n> instead.\")\n>\n\nAt present we can only concatenate the lines in the first paragraph so\nthat we can generate a valid rfc2047 mail, but we will lost the line breaks\nafter import the patch by git-am.\n\n>> ---\n>\n> Missing sign-off with a real name.\n>\n\nI am sorry that I didn't find the document of submitting a patch until\nyesterday, Thanks for your comment.\n\n>> diff --git a/pretty.c b/pretty.c\n>> index 8549934..f18a38d 100644\n>> --- a/pretty.c\n>> +++ b/pretty.c\n>> @@ -249,6 +249,33 @@ needquote:\n>>       strbuf_addstr(sb, \"?=\");\n>>  }\n>>\n>> +static void add_rfc2047_multiline(struct strbuf *sb, const char *line, int len,\n>> +                       const char *encoding)\n>> +{\n>> +     int first = 1;\n>> +     char *mline = xmemdupz(line, len);\n>> +     const char *cline = mline;\n>> +     int offset = 0, linelen = 0;\n>> +        for (;;) {\n>\n> You seem to have indent that uses SPs instead of HT around here...\n>\n>> +                linelen = get_one_line(cline);\n>\n> I can see you are trying to be careful not to let get_one_line() overstep\n> past \"len\" the caller gave you by making a copy first, but is this\n> overhead really necessary?  After all we know in this static function that\n> the caller is feeding the contents from a strbuf, which always have a\n> terminating NUL (and that is why it is Ok that get_one_line() is not a\n> counted string interface).\n>\n\nI am not sure that who will call this function in future, I think since there is\na argument as len, so I'd better to obey the function declare.\n\n>> +\n>> +                cline += linelen;\n>> +\n>> +                if (!linelen)\n>> +                        break;\n>> +\n>> +             if (!first)\n>> +                     strbuf_addf(sb, \"\\n \");\n>> +\n>> +             offset = *(cline -1) == '\\n';\n>> +\n>> +             add_rfc2047(sb, cline-linelen, linelen-offset, encoding);\n>> +             first = 0;\n>> +\n>> +        }\n>> +     free(mline);\n>> +}\n>\n> So the general idea of this change (I am thinking aloud what should be in\n> the updated commit log message as the problem description) is that:\n>\n>  - We currently give an entire multi-line paragraph string to the\n>   add_rfc2047() function to be formatted as the title of the commit;\n>\n>  - The add_rfc2047() functionjust passes \"\\n\" through, without making it a\n>   folding whitespace followed by a newline, to help callers that want to\n>   use this function to produce a header line that is rfc 2822 conformant;\n>\n>  - The patch introduces a new function add_rfc2047_multiline() that splits\n>   its input and performs line folding for such a caller (namely, the\n>   pp_title_line() function);\n>\n>  - Another caller of add_rfc2047(), pp_user_info, is not changed, and it\n>   won't fold the name of the user that appear on the From: line.\n>\n> It is unclear if the last point is really the right thing to do, though.\n> It is not a new problem that an author name that has a \"\\n\" in it would\n> break the output, but we probably would want to fix that case too here?\n>\n\nYour comment is just right for what I tried to do, I explained why I add a new\nfunction for subject specially in the mail which replied to Jeff, I\nwant to remain\nthe line breaks after import the patch, so I think I need do something here\nin future, it will be compatible with rfc2047 and also can be imported with\nline breaks correctly. I don't know how yet, so I just want to left a\npossibility.\nSo I introduce a new function for subject only.\n\nxzer\n"},{"id":"162030","messageId":"20110223163558.GA10042@sigill.intra.peff.net","threadId":"26492","inReplyTo":"AANLkTimUXqKdTDcSVDK44XPhxWbHtQuDWHMED3PKqWE4@mail.gmail.com","subject":"Re: [PATCH] generate a valid rfc2047 mail header for multi-line subject.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-23T16:35:58Z","receivedAt":"2011-02-23T16:35:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 24, 2011 at 12:16:04AM +0900, xzer wrote:\n\n> To the first point, I really want to find a way that we can remain the\n> line breaker\n> after import a formatted patch. That's why I add a new function to product multi\n> line header, I want to do something which is special to subject. In my usage,\n> I told my men every day that don't write too long in the first\n> paragraph, but there\n> are always somebody who forgets it, then I will get a patch with a\n> very long subject\n> just like a nightmare(yes, I gave them my temporary fix which I submitted here,\n> so they can write as long as they want).\n> \n> So I want to know whether we can generate a 2047 compatible header so\n> that mailer\n> can catch it correctly and the git-am can import it with line breaker\n> correctly too.\n\nYes. With my patches, if you feed a subject with linebreaks to\nadd_rfc2047, they will be encoded. So you just need an extra patch on\ntop of mine that will use straight linebreaks (_not_ linebreaks with an\nextra space) in pp_title_line.  Below is a quick and dirty patch to do\nthat when \"-k\" is specified. You will also need to specify \"-k\" with\napplying it with \"git am\", but other than that it seems to work.\n\nHowever, I'm still not sure it's a good idea. Other parts of git will\ntry to treat your paragraph as a single line (e.g., git log --oneline).\nPlus this patch is ugly because of the number of layers of abstraction\nwe have to pass the keep-subject through. I'm not sure there's a good\nway around that.\n\n---\ndiff --git a/builtin/log.c b/builtin/log.c\nindex d8c6c28..3fdf488 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -768,7 +768,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \tpp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, DATE_RFC2822,\n \t\t     encoding);\n \tpp_title_line(CMIT_FMT_EMAIL, &msg, &sb, subject_start, extra_headers,\n-\t\t      encoding, need_8bit_cte);\n+\t\t      encoding, need_8bit_cte, 0);\n \tpp_remainder(CMIT_FMT_EMAIL, &msg, &sb, 0);\n \tprintf(\"%s\\n\", sb.buf);\n \n@@ -1130,6 +1130,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tdie (\"-n and -k are mutually exclusive.\");\n \tif (keep_subject && subject_prefix)\n \t\tdie (\"--subject-prefix and -k are mutually exclusive.\");\n+\trev.preserve_subject = keep_subject;\n \n \targc = setup_revisions(argc, argv, &rev, &s_r_opt);\n \tif (argc > 1)\ndiff --git a/commit.h b/commit.h\nindex 659c87c..6eace1c 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -73,6 +73,7 @@ struct pretty_print_context\n \tint abbrev;\n \tconst char *subject;\n \tconst char *after_subject;\n+\tint preserve_subject;\n \tenum date_mode date_mode;\n \tint need_8bit_cte;\n \tint show_notes;\n@@ -107,7 +108,8 @@ void pp_title_line(enum cmit_fmt fmt,\n \t\t   const char *subject,\n \t\t   const char *after_subject,\n \t\t   const char *encoding,\n-\t\t   int need_8bit_cte);\n+\t\t   int need_8bit_cte,\n+\t\t   int preserve_lines);\n void pp_remainder(enum cmit_fmt fmt,\n \t\t  const char **msg_p,\n \t\t  struct strbuf *sb,\ndiff --git a/log-tree.c b/log-tree.c\nindex b46ed3b..9b9aaf2 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -504,6 +504,7 @@ void show_log(struct rev_info *opt)\n \tctx.date_mode = opt->date_mode;\n \tctx.abbrev = opt->diffopt.abbrev;\n \tctx.after_subject = extra_headers;\n+\tctx.preserve_subject = opt->preserve_subject;\n \tctx.reflog_info = opt->reflog_info;\n \tpretty_print_commit(opt->commit_format, commit, &msgbuf, &ctx);\n \ndiff --git a/pretty.c b/pretty.c\nindex 65d20a7..315f1d2 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1121,12 +1121,13 @@ void pp_title_line(enum cmit_fmt fmt,\n \t\t   const char *subject,\n \t\t   const char *after_subject,\n \t\t   const char *encoding,\n-\t\t   int need_8bit_cte)\n+\t\t   int need_8bit_cte,\n+\t\t   int preserve_lines)\n {\n \tstruct strbuf title;\n \n \tstrbuf_init(&title, 80);\n-\t*msg_p = format_subject(&title, *msg_p, \" \");\n+\t*msg_p = format_subject(&title, *msg_p, preserve_lines ? \"\\n\" : \" \");\n \n \tstrbuf_grow(sb, title.len + 1024);\n \tif (subject) {\n@@ -1254,7 +1255,8 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,\n \t/* These formats treat the title line specially. */\n \tif (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)\n \t\tpp_title_line(fmt, &msg, sb, context->subject,\n-\t\t\t      context->after_subject, encoding, need_8bit_cte);\n+\t\t\t      context->after_subject, encoding, need_8bit_cte,\n+\t\t\t      context->preserve_subject);\n \n \tbeginning_of_body = sb->len;\n \tif (fmt != CMIT_FMT_ONELINE)\ndiff --git a/revision.h b/revision.h\nindex 05659c6..f8ddd83 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -90,7 +90,8 @@ struct rev_info {\n \t\t\tabbrev_commit:1,\n \t\t\tuse_terminator:1,\n \t\t\tmissing_newline:1,\n-\t\t\tdate_mode_explicit:1;\n+\t\t\tdate_mode_explicit:1,\n+\t\t\tpreserve_subject:1;\n \tunsigned int\tdisable_stdin:1;\n \n \tenum date_mode date_mode;\n"},{"id":"162037","messageId":"7vd3miac47.fsf@alter.siamese.dyndns.org","threadId":"26492","inReplyTo":"20110223080854.GB2724@sigill.intra.peff.net","subject":"Re: [PATCH] generate a valid rfc2047 mail header for multi-line subject.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-23T17:34:32Z","receivedAt":"2011-02-23T17:34:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Yeah, I think the best path forward is:\n>\n>   1. Stop feeding \"pre-folded\" subject lines to the email formatter.\n>      Give it the regular subject line with no newlines.\n\nA bit of history.  The original design of the pp_title_line() function\nsince 4234a76 (Extend --pretty=oneline to cover the first paragraph,\n2007-06-11) was to notice a multi-line paragraph and turn embedded\nnewlines into line folds (this seems to be a breakage specific to\nnon-ASCII titles).\n\nAs RFC 5322 (or 822/2822 for that matter) does not allow newlines in field\nbodies (2.2: A field body MUST NOT include CR and LF except when used in\n\"folding\" and \"unfolding\"...), it was the only way to allow the recipient\nto tell where the original line breaks were to fold at the line breaks in\nthe original commit message.  Then the recipient _can_ be git aware and\nturn the folding CRLF-SP into a LF, not just a SP, relying on the hope\nthat the transport between the sender and the recipient would not clobber\nline folding, to recover the original.\n\nThe rebase pipeline (i.e. \"format-patch | am\") would have satisfied such a\nflaky assumption and that was the only reason I wrote the line folding on\nthe output side that way.  These days, however, \"am\" invoked in the rebase\npipeline knows to slurp the message not from the patch text but from the\noriginal message, so we can safely depart form the original design rationale.\n\n>   2. rfc2047 encoding should encode a literal newline. Which should\n>      generally never happen, but is probably the most sane thing to do\n>      if it does.\n\nI was re-reading RFC 2047 and its 5. (3) [Page 8] seems to imply that this\nmight be allowed: \"Only printable and white space character data should be\nencoded using this scheme.\"; I think LF is counted as a white space\ncharacter in this context, but it is a bit unclear.\n\nIf this \"encode newline via 2047\" _were_ allowed, I would say that my\npreference is not to go with your 1. above.  Instead I would prefer to see\nus feed the entire first paragraph, whether it is a single-liner or\nmulti-line paragraph, to the step 2 ...\n\n>   3. rfc2047 should fold all lines at some sane length...\n\n... and the have step3 fold its result to limit the physical length of the\noutput line(s).  Note that a multi-line first paragraph always will be\nencoded using 2047 because we cannot have a newline in the field body per\nRFC5322.  But going the above route would allow us to recover the original\nfirst paragraph intact.\n\nWe might need to tweak the receiving end a bit, though.  IIRC, mailinfo\noutput assumed we will always be dealing with a single-liner subject.\n"},{"id":"162039","messageId":"7v8vx6abm7.fsf@alter.siamese.dyndns.org","threadId":"26492","inReplyTo":"AANLkTinf6P-erY-9p5WPWbK+uAf1hozvAutV0zPSpHGQ@mail.gmail.com","subject":"Re: [PATCH] generate a valid rfc2047 mail header for multi-line subject.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-23T17:45:20Z","receivedAt":"2011-02-23T17:45:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"xzer <xiaozhu@gmail.com> writes:\n\n> 2011/2/23 Junio C Hamano <gitster@pobox.com>:\n>> xzer <xiaozhu@gmail.com> writes:\n>>\n>>> Subject: Re: [PATCH] generate a valid rfc2047 mail header for multi-line subject.\n>>\n>> We prefer to have \"[PATCH] subsystem: description without final full-stop\" here.\n>>\n>>> There is still a problem that git-am will lost the line break.\n>>\n>> What does \"still\" refer to?  It is unclear under what condition the\n>> command lose \"the line break\" (nor which line break you are refering to; I\n>> am guessing that you have a commit that begins with a multi-line paragraph\n>> and you are talking about line breaks between the lines in the first\n>> paragraph).\n>\n> Yes, that is what I am refering, the line breaks in the first paragraph.\n\nHmm, I gave a suggestion and asked three questions, and only get one\nanswer back?\n\n>> ... After all we know in this static function that\n>> the caller is feeding the contents from a strbuf, which always have a\n>> terminating NUL (and that is why it is Ok that get_one_line() is not a\n>> counted string interface).\n>\n> I am not sure that who will call this function in future, I think since there is\n> a argument as len, so I'd better to obey the function declare.\n\nIf that is the case I would have preferred to see you give get_one_line()\nthat is a function static to this file an ability to read from a counted\nstring, instead of making an extra allcation.  But I think you will notice\nthat all the callchain that pass a pointer into the message around knows\nand relies on the fact that the buffer is NUL terminated if you look\naround in the file, and that was why I made that suggestion.\n"},{"id":"162060","messageId":"7vhbbu7792.fsf@alter.siamese.dyndns.org","threadId":"26492","inReplyTo":"20110223095917.GC9222@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] format-patch: rfc2047-encode newlines in headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-23T21:47:53Z","receivedAt":"2011-02-23T21:47:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\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\nIn this particular case, wouldn't it be more conservative and defensive to\nproduce malformed headers so that the patch won't leave the originator?  I\nhave a suspicion that mailinfo would choke on the output of this one, even\nthough I didn't try.\n"},{"id":"162107","messageId":"20110224071534.GC16550@sigill.intra.peff.net","threadId":"26492","inReplyTo":"7vhbbu7792.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] format-patch: rfc2047-encode newlines in headers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-24T07:15:35Z","receivedAt":"2011-02-24T07:15:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 23, 2011 at 01:47:53PM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\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> \n> In this particular case, wouldn't it be more conservative and defensive to\n> produce malformed headers so that the patch won't leave the\n> originator?\n\nNo. If you go back to xzer's original mail, the malformed headers didn't\ncause messages not to be sent. They just resulted in corrupted and\nmissing data on the receiver side. I don't think we can rely on any MUA\nor MTA having a particular behavior for malformed mail. Some of them may\ncomplain, but many won't.\n\n> I have a suspicion that mailinfo would choke on the output of this\n> one, even though I didn't try.\n\nActually, it does quite well. Without \"-k\", mailinfo turns it into a\nsingle line, which is what I would expect. With \"-k\", the info file\ncontains:\n\n  Author: Jeff King\n  Email: peff@peff.net\n  Subject: this is a long\n  Subject: subject line with\n  Subject: many lines in it\n  Date: Wed, 23 Feb 2011 11:30:43 -0500\n\nwhich \"git am\" turns back into the original multi-line subject.\n\nSo I think it's definitely the right thing to do. Not only does it avoid\nus generating malformed mail, but because existing mailinfo handles it\nsanely, it makes it easy to do a \"preserve-newlines\" patch on top (which\nI'm still not sure is a great idea, but I can see the use in certain\ncircumstances).\n\n-Peff\n"},{"id":"162108","messageId":"20110224073416.GD16550@sigill.intra.peff.net","threadId":"26492","inReplyTo":"7vd3miac47.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] generate a valid rfc2047 mail header for multi-line subject.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-24T07:34:16Z","receivedAt":"2011-02-24T07:34:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 23, 2011 at 09:34:32AM -0800, Junio C Hamano wrote:\n\n> A bit of history.  The original design of the pp_title_line() function\n> since 4234a76 (Extend --pretty=oneline to cover the first paragraph,\n> 2007-06-11) was to notice a multi-line paragraph and turn embedded\n> newlines into line folds (this seems to be a breakage specific to\n> non-ASCII titles).\n\nYes, the only actual breakage is the interaction of the folding with\nnon-ascii titles. The \"fold everything\" is a new feature. And one that\nis probably not all that necessary in the real world, as rfc2822 allows\nup to 998 characters. So it is more about complying with the SHOULD\nthere than the MUST.\n\n> As RFC 5322 (or 822/2822 for that matter) does not allow newlines in field\n\nWow, I'm out of date. I had no idea 2822 had been superseded. ;)\n\n> bodies (2.2: A field body MUST NOT include CR and LF except when used in\n> \"folding\" and \"unfolding\"...), it was the only way to allow the recipient\n> to tell where the original line breaks were to fold at the line breaks in\n> the original commit message.  Then the recipient _can_ be git aware and\n> turn the folding CRLF-SP into a LF, not just a SP, relying on the hope\n> that the transport between the sender and the recipient would not clobber\n> line folding, to recover the original.\n\nAh, that makes the current code make a lot more sense. Thanks for the\nhistory.\n\n> The rebase pipeline (i.e. \"format-patch | am\") would have satisfied such a\n> flaky assumption and that was the only reason I wrote the line folding on\n> the output side that way.  These days, however, \"am\" invoked in the rebase\n> pipeline knows to slurp the message not from the patch text but from the\n> original message, so we can safely depart form the original design rationale.\n\nAgreed.\n\n> I was re-reading RFC 2047 and its 5. (3) [Page 8] seems to imply that this\n> might be allowed: \"Only printable and white space character data should be\n> encoded using this scheme.\"; I think LF is counted as a white space\n> character in this context, but it is a bit unclear.\n\nYeah, the section is a little vague. I think the intent is that senders\nshould encode reasonable text things, not multimedia garbage, and that\nreceivers should be wary of getting arbitrary crap. So I think we are\nfollowing the spirit of the section in any case.\n\nReading over it, though, I do notice that we are specifically forbidden\nto break multi-byte characters between encoded-words. And my\nimplementation doesn't take care about that. I think in practice, any\nreasonable implementation would just concatenate the results and be\nhappy, but you never know.\n\n> If this \"encode newline via 2047\" _were_ allowed, I would say that my\n> preference is not to go with your 1. above.  Instead I would prefer to see\n> us feed the entire first paragraph, whether it is a single-liner or\n> multi-line paragraph, to the step 2 ...\n\nHmm. I disagree. I thought the decision was made long ago to convert\nsuch multi-paragraph subjects into a single line in most cases.  Because\nsupporting such subjects at all was never about encouraging people to\nflaunt the subject-line convention, but about letting the tools have a\nreasonable default behavior for commits imported from systems that did\nnot follow that convention.\n\nOn top of that, I think there is some question of how encoded newlines\nin a subject line will be handled by MUAs in the wild (given the\nambiguity of rfc2047 mentioned above). So perhaps it is better to be\nconservative and not generate them by default.\n\nAnd on top of that, it is not just a \"how should it be if starting from\nscratch\" decision. We have been flattening multi-line ascii subjects for\nyears, so this would be a behavior change.\n\nSo I would think this should be triggered by an option (\"-k\" makes the\nmost sense to me) if anything. I am somewhat lukewarm on even that; as\nyou mentioned above, the rebase pipeline has a better preservation\nmechanism these days, so it is really about people who want to email\npatches to each other while disregarding the subject-line convention.\n\n> >   3. rfc2047 should fold all lines at some sane length...\n> \n> ... and the have step3 fold its result to limit the physical length of the\n> output line(s).  Note that a multi-line first paragraph always will be\n> encoded using 2047 because we cannot have a newline in the field body per\n> RFC5322.  But going the above route would allow us to recover the original\n> first paragraph intact.\n\nYes, exactly.\n\n> We might need to tweak the receiving end a bit, though.  IIRC, mailinfo\n> output assumed we will always be dealing with a single-liner subject.\n\nI don't know if it's intentional or accidental, but mailinfo handles it\njust as I would expect. See my other message.\n\n-Peff\n"}]}