{"thread":{"id":"48379","subject":"Bug: format-patch MIME boundary not added to cover letter when attach enabled","startedAt":"2018-04-30T01:48:16Z","lastAt":"2018-05-02T02:21:05Z","messageCount":8,"participants":["Patrick Hemmer","brian m. carlson","Junio C Hamano","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"346119","messageId":"5e409069-835f-3c85-f55e-c27f534dc9e2@stormcloud9.net","threadId":"48379","inReplyTo":null,"subject":"Bug: format-patch MIME boundary not added to cover letter when attach enabled","fromName":"Patrick Hemmer","fromEmail":"git@stormcloud9.net","sentAt":"2018-04-30T01:40:13Z","receivedAt":"2018-04-30T01:48:16Z","isPatch":false,"sender":{"key":"git@stormcloud9.net","avatar":null},"body":"When you use `git format-patch --cover-letter --attach`, the cover\nletter does not have the trailing MIME boundary. RFC2046 states that the\nlast part must be followed by a closing boundary. This causes some email\nclients (Thunderbird in my case) to discard the message body.\nThis is experienced with git 2.16.3.\n\nFor example:\n\n$ git format-patch --cover-letter --attach --root -o /tmp/out\n/tmp/out/0000-cover-letter.patch\n/tmp/out/0001-hello-world.patch\n\n$ cat /tmp/out/0000-cover-letter.patch\nFrom a25ac88e6216131e8b000335d32bb99d4e5185ac Mon Sep 17 00:00:00 2001\nFrom: Patrick Hemmer \nDate: Sun, 29 Apr 2018 21:26:45 -0400\nSubject: [PATCH 0/1] *** SUBJECT HERE ***\nMIME-Version: 1.0\nContent-Type: multipart/mixed; boundary=\"------------2.16.3\"\n\nThis is a multi-part message in MIME format.\n--------------2.16.3\nContent-Type: text/plain; charset=UTF-8; format=fixed\nContent-Transfer-Encoding: 8bit\n\n\n*** BLURB HERE ***\n\nPatrick Hemmer (1):\n hello world\n\n-- \n2.16.3\n"},{"id":"346120","messageId":"20180430020930.GA13217@genre.crustytoothpaste.net","threadId":"48379","inReplyTo":"5e409069-835f-3c85-f55e-c27f534dc9e2@stormcloud9.net","subject":"Re: Bug: format-patch MIME boundary not added to cover letter when attach enabled","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-04-30T02:09:31Z","receivedAt":"2018-04-30T02:09:41Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sun, Apr 29, 2018 at 09:40:13PM -0400, Patrick Hemmer wrote:\n> When you use `git format-patch --cover-letter --attach`, the cover\n> letter does not have the trailing MIME boundary. RFC2046 states that the\n> last part must be followed by a closing boundary. This causes some email\n> clients (Thunderbird in my case) to discard the message body.\n> This is experienced with git 2.16.3.\n\nThanks for reporting this. I can confirm this with a reasonably recent\nnext. Let me see if I can come up with a patch.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"346123","messageId":"xmqqzi1lxte6.fsf@gitster-ct.c.googlers.com","threadId":"48379","inReplyTo":"20180430020930.GA13217@genre.crustytoothpaste.net","subject":"Re: Bug: format-patch MIME boundary not added to cover letter when attach enabled","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-30T03:30:57Z","receivedAt":"2018-04-30T03:31:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> On Sun, Apr 29, 2018 at 09:40:13PM -0400, Patrick Hemmer wrote:\n>> When you use `git format-patch --cover-letter --attach`, the cover\n>> letter does not have the trailing MIME boundary. RFC2046 states that the\n>> last part must be followed by a closing boundary. This causes some email\n>> clients (Thunderbird in my case) to discard the message body.\n>> This is experienced with git 2.16.3.\n>\n> Thanks for reporting this. I can confirm this with a reasonably recent\n> next. Let me see if I can come up with a patch.\n\nThanks. It is true that the current output from the tool is corrupt\nmime multi-part, and we need to do something about it.\n\nI however have to wonder if it even makes sense for --cover to pay\nattention to --attach and produce the cover template that has \"BLURB\nHERE\" etc. in a multi-part format. Shouldn't we be making a simple\nplain text file instead?\n"},{"id":"346141","messageId":"20180430115909.GB13217@genre.crustytoothpaste.net","threadId":"48379","inReplyTo":"xmqqzi1lxte6.fsf@gitster-ct.c.googlers.com","subject":"Re: Bug: format-patch MIME boundary not added to cover letter when attach enabled","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-04-30T11:59:09Z","receivedAt":"2018-04-30T11:59:19Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Apr 30, 2018 at 12:30:57PM +0900, Junio C Hamano wrote:\n> Thanks. It is true that the current output from the tool is corrupt\n> mime multi-part, and we need to do something about it.\n> \n> I however have to wonder if it even makes sense for --cover to pay\n> attention to --attach and produce the cover template that has \"BLURB\n> HERE\" etc. in a multi-part format. Shouldn't we be making a simple\n> plain text file instead?\n\nI agree that multipart/mixed is not a useful content-type for only one\nplain text part. I have a patch to add the trailing boundary, but I\nthink you make a good argument that perhaps omitting the entire\nmultipart portion would be better.\n\nI'll have to work on this after work, so expect a patch later today.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"346216","messageId":"20180501000214.833766-1-sandals@crustytoothpaste.net","threadId":"48379","inReplyTo":"20180430020930.GA13217@genre.crustytoothpaste.net","subject":"[PATCH] format-patch: make cover letters always text/plain","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-05-01T00:02:14Z","receivedAt":"2018-05-01T00:02:28Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When formatting a series of patches using --attach and --cover-letter,\nthe cover letter lacks the closing MIME boundary, violating RFC 2046.\nCertain clients, such as Thunderbird, discard the message body in such a\ncase.\n\nSince the cover letter is just one part and sending it as\nmultipart/mixed is not very useful, always emit it as text/plain,\navoiding the boundary problem altogether.\n\nReported-by: Patrick Hemmer \nSigned-off-by: brian m. carlson \n---\n builtin/log.c | 2 +-\n log-tree.c | 7 ++++---\n log-tree.h | 3 ++-\n t/t4014-format-patch.sh | 9 +++++++++\n 4 files changed, 16 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 71f68a3e4f..24868ed070 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1019,7 +1019,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t open_next_file(NULL, rev->numbered_files ? NULL : \"cover-letter\", rev, quiet))\n \t\treturn;\n \n-\tlog_write_email_headers(rev, head, &pp.after_subject, &need_8bit_cte);\n+\tlog_write_email_headers(rev, head, &pp.after_subject, &need_8bit_cte, 0);\n \n \tfor (i = 0; !need_8bit_cte && i < nr; i++) {\n \t\tconst char *buf = get_commit_buffer(list[i], NULL);\ndiff --git a/log-tree.c b/log-tree.c\nindex d1c0bedf24..9f5eb346a4 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -362,7 +362,8 @@ void fmt_output_email_subject(struct strbuf *sb, struct rev_info *opt)\n \n void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\t\t const char **extra_headers_p,\n-\t\t\t int *need_8bit_cte_p)\n+\t\t\t int *need_8bit_cte_p,\n+\t\t\t int maybe_multipart)\n {\n \tconst char *extra_headers = opt->extra_headers;\n \tconst char *name = oid_to_hex(opt->zero_commit ?\n@@ -385,7 +386,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\t\t opt->ref_message_ids->items[i].string);\n \t\tgraph_show_oneline(opt->graph);\n \t}\n-\tif (opt->mime_boundary) {\n+\tif (opt->mime_boundary && maybe_multipart) {\n \t\tstatic char subject_buffer[1024];\n \t\tstatic char buffer[1024];\n \t\tstruct strbuf filename = STRBUF_INIT;\n@@ -610,7 +611,7 @@ void show_log(struct rev_info *opt)\n \n \tif (cmit_fmt_is_mail(opt->commit_format)) {\n \t\tlog_write_email_headers(opt, commit, &extra_headers,\n-\t\t\t\t\t&ctx.need_8bit_cte);\n+\t\t\t\t\t&ctx.need_8bit_cte, 1);\n \t\tctx.rev = opt;\n \t\tctx.print_email_subject = 1;\n \t} else if (opt->commit_format != CMIT_FMT_USERFORMAT) {\ndiff --git a/log-tree.h b/log-tree.h\nindex deba035187..e668628074 100644\n--- a/log-tree.h\n+++ b/log-tree.h\n@@ -27,7 +27,8 @@ void format_decorations_extended(struct strbuf *sb, const struct commit *commit,\n void show_decorations(struct rev_info *opt, struct commit *commit);\n void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\t\t const char **extra_headers_p,\n-\t\t\t int *need_8bit_cte_p);\n+\t\t\t int *need_8bit_cte_p,\n+\t\t\t int maybe_multipart);\n void load_ref_decorations(struct decoration_filter *filter, int flags);\n \n #define FORMAT_PATCH_NAME_MAX 64\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 482112ca33..83c596a842 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1661,6 +1661,15 @@ test_expect_success 'format-patch --base with --attach' '\n \ttest_write_lines 1 2 >expect &&\n \ttest_cmp expect actual\n '\n+test_expect_success 'format-patch --attach cover-letter only is non-multipart' '\n+\ttest_when_finished \"rm -r patches\" &&\n+\tgit format-patch -o patches --cover-letter --attach=mimemime --base=HEAD~ -1 &&\n+\t! egrep \"^--+mimemime\" patches/0000*.patch &&\n+\tegrep \"^--+mimemime$\" patches/0001*.patch >output &&\n+\ttest_line_count = 2 output &&\n+\tegrep \"^--+mimemime--$\" patches/0001*.patch >output &&\n+\ttest_line_count = 1 output\n+'\n \n test_expect_success 'format-patch --pretty=mboxrd' '\n \tsp=\" \" &&\n"},{"id":"346219","messageId":"CAPig+cRucJfzm8Z0XhPa0CMw+LiBdY_b5=bMz0LkezJxvC9qJA@mail.gmail.com","threadId":"48379","inReplyTo":"20180501000214.833766-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH] format-patch: make cover letters always text/plain","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-05-01T01:53:33Z","receivedAt":"2018-05-01T01:53:37Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Apr 30, 2018 at 8:02 PM, brian m. carlson\n wrote:\n> When formatting a series of patches using --attach and --cover-letter,\n> the cover letter lacks the closing MIME boundary, violating RFC 2046.\n> Certain clients, such as Thunderbird, discard the message body in such a\n> case.\n>\n> Since the cover letter is just one part and sending it as\n> multipart/mixed is not very useful, always emit it as text/plain,\n> avoiding the boundary problem altogether.\n>\n> Reported-by: Patrick Hemmer \n> Signed-off-by: brian m. carlson \n> ---\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> @@ -1661,6 +1661,15 @@ test_expect_success 'format-patch --base with --attach' '\n> +test_expect_success 'format-patch --attach cover-letter only is non-multipart' '\n> + test_when_finished \"rm -r patches\" &&\n> + git format-patch -o patches --cover-letter --attach=mimemime --base=HEAD~ -1 &&\n\nNit: \"rm -rf\" would be a bit more robust against git-format-patch\nsomehow crashing before creating the \"patches\" directory.\n\n> + ! egrep \"^--+mimemime\" patches/0000*.patch &&\n> + egrep \"^--+mimemime$\" patches/0001*.patch >output &&\n> + test_line_count = 2 output &&\n> + egrep \"^--+mimemime--$\" patches/0001*.patch >output &&\n> + test_line_count = 1 output\n> +'\n"},{"id":"346357","messageId":"20180502001606.GI13217@genre.crustytoothpaste.net","threadId":"48379","inReplyTo":"CAPig+cRucJfzm8Z0XhPa0CMw+LiBdY_b5=bMz0LkezJxvC9qJA@mail.gmail.com","subject":"Re: [PATCH] format-patch: make cover letters always text/plain","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-05-02T00:16:07Z","receivedAt":"2018-05-02T00:16:16Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Apr 30, 2018 at 09:53:33PM -0400, Eric Sunshine wrote:\n> On Mon, Apr 30, 2018 at 8:02 PM, brian m. carlson\n> wrote:\n> > When formatting a series of patches using --attach and --cover-letter,\n> > the cover letter lacks the closing MIME boundary, violating RFC 2046.\n> > Certain clients, such as Thunderbird, discard the message body in such a\n> > case.\n> >\n> > Since the cover letter is just one part and sending it as\n> > multipart/mixed is not very useful, always emit it as text/plain,\n> > avoiding the boundary problem altogether.\n> >\n> > Reported-by: Patrick Hemmer \n> > Signed-off-by: brian m. carlson \n> > ---\n> > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> > @@ -1661,6 +1661,15 @@ test_expect_success 'format-patch --base with --attach' '\n> > +test_expect_success 'format-patch --attach cover-letter only is non-multipart' '\n> > + test_when_finished \"rm -r patches\" &&\n> > + git format-patch -o patches --cover-letter --attach=mimemime --base=HEAD~ -1 &&\n> \n> Nit: \"rm -rf\" would be a bit more robust against git-format-patch\n> somehow crashing before creating the \"patches\" directory.\n\nSure, I can reroll with that change. I had considered doing that, but\ndecided against it. I hadn't thought of resilience against a failed git\nformat-patch, though.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"346416","messageId":"20180502022052.939511-1-sandals@crustytoothpaste.net","threadId":"48379","inReplyTo":"20180501000214.833766-1-sandals@crustytoothpaste.net","subject":"[PATCH v2] format-patch: make cover letters always text/plain","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-05-02T02:20:52Z","receivedAt":"2018-05-02T02:21:05Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When formatting a series of patches using --attach and --cover-letter,\nthe cover letter lacks the closing MIME boundary, violating RFC 2046.\nCertain clients, such as Thunderbird, discard the message body in such a\ncase.\n\nSince the cover letter is just one part and sending it as\nmultipart/mixed is not very useful, always emit it as text/plain,\navoiding the boundary problem altogether.\n\nReported-by: Patrick Hemmer \nSigned-off-by: brian m. carlson \n---\nChanges from v1:\n* Switch from rm -r to rm -fr.\n\n builtin/log.c | 2 +-\n log-tree.c | 7 ++++---\n log-tree.h | 3 ++-\n t/t4014-format-patch.sh | 9 +++++++++\n 4 files changed, 16 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 71f68a3e4f..24868ed070 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1019,7 +1019,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t open_next_file(NULL, rev->numbered_files ? NULL : \"cover-letter\", rev, quiet))\n \t\treturn;\n \n-\tlog_write_email_headers(rev, head, &pp.after_subject, &need_8bit_cte);\n+\tlog_write_email_headers(rev, head, &pp.after_subject, &need_8bit_cte, 0);\n \n \tfor (i = 0; !need_8bit_cte && i < nr; i++) {\n \t\tconst char *buf = get_commit_buffer(list[i], NULL);\ndiff --git a/log-tree.c b/log-tree.c\nindex d1c0bedf24..9f5eb346a4 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -362,7 +362,8 @@ void fmt_output_email_subject(struct strbuf *sb, struct rev_info *opt)\n \n void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\t\t const char **extra_headers_p,\n-\t\t\t int *need_8bit_cte_p)\n+\t\t\t int *need_8bit_cte_p,\n+\t\t\t int maybe_multipart)\n {\n \tconst char *extra_headers = opt->extra_headers;\n \tconst char *name = oid_to_hex(opt->zero_commit ?\n@@ -385,7 +386,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\t\t opt->ref_message_ids->items[i].string);\n \t\tgraph_show_oneline(opt->graph);\n \t}\n-\tif (opt->mime_boundary) {\n+\tif (opt->mime_boundary && maybe_multipart) {\n \t\tstatic char subject_buffer[1024];\n \t\tstatic char buffer[1024];\n \t\tstruct strbuf filename = STRBUF_INIT;\n@@ -610,7 +611,7 @@ void show_log(struct rev_info *opt)\n \n \tif (cmit_fmt_is_mail(opt->commit_format)) {\n \t\tlog_write_email_headers(opt, commit, &extra_headers,\n-\t\t\t\t\t&ctx.need_8bit_cte);\n+\t\t\t\t\t&ctx.need_8bit_cte, 1);\n \t\tctx.rev = opt;\n \t\tctx.print_email_subject = 1;\n \t} else if (opt->commit_format != CMIT_FMT_USERFORMAT) {\ndiff --git a/log-tree.h b/log-tree.h\nindex deba035187..e668628074 100644\n--- a/log-tree.h\n+++ b/log-tree.h\n@@ -27,7 +27,8 @@ void format_decorations_extended(struct strbuf *sb, const struct commit *commit,\n void show_decorations(struct rev_info *opt, struct commit *commit);\n void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\t\t const char **extra_headers_p,\n-\t\t\t int *need_8bit_cte_p);\n+\t\t\t int *need_8bit_cte_p,\n+\t\t\t int maybe_multipart);\n void load_ref_decorations(struct decoration_filter *filter, int flags);\n \n #define FORMAT_PATCH_NAME_MAX 64\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 482112ca33..6ea08fd5e9 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1661,6 +1661,15 @@ test_expect_success 'format-patch --base with --attach' '\n \ttest_write_lines 1 2 >expect &&\n \ttest_cmp expect actual\n '\n+test_expect_success 'format-patch --attach cover-letter only is non-multipart' '\n+\ttest_when_finished \"rm -fr patches\" &&\n+\tgit format-patch -o patches --cover-letter --attach=mimemime --base=HEAD~ -1 &&\n+\t! egrep \"^--+mimemime\" patches/0000*.patch &&\n+\tegrep \"^--+mimemime$\" patches/0001*.patch >output &&\n+\ttest_line_count = 2 output &&\n+\tegrep \"^--+mimemime--$\" patches/0001*.patch >output &&\n+\ttest_line_count = 1 output\n+'\n \n test_expect_success 'format-patch --pretty=mboxrd' '\n \tsp=\" \" &&\n"}]}