* Bug: format-patch MIME boundary not added to cover letter when attach enabled @ 2018-04-30 1:40 Patrick Hemmer 2018-04-30 2:09 ` brian m. carlson 0 siblings, 1 reply; 8+ messages in thread From: Patrick Hemmer @ 2018-04-30 1:40 UTC (permalink / raw) To: git When you use `git format-patch --cover-letter --attach`, the cover letter does not have the trailing MIME boundary. RFC2046 states that the last part must be followed by a closing boundary. This causes some email clients (Thunderbird in my case) to discard the message body. This is experienced with git 2.16.3. For example: $ git format-patch --cover-letter --attach --root -o /tmp/out /tmp/out/0000-cover-letter.patch /tmp/out/0001-hello-world.patch $ cat /tmp/out/0000-cover-letter.patch From a25ac88e6216131e8b000335d32bb99d4e5185ac Mon Sep 17 00:00:00 2001 From: Patrick Hemmer <git@stormcloud9.net> Date: Sun, 29 Apr 2018 21:26:45 -0400 Subject: [PATCH 0/1] *** SUBJECT HERE *** MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="------------2.16.3" This is a multi-part message in MIME format. --------------2.16.3 Content-Type: text/plain; charset=UTF-8; format=fixed Content-Transfer-Encoding: 8bit *** BLURB HERE *** Patrick Hemmer (1): hello world -- 2.16.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Bug: format-patch MIME boundary not added to cover letter when attach enabled 2018-04-30 1:40 Bug: format-patch MIME boundary not added to cover letter when attach enabled Patrick Hemmer @ 2018-04-30 2:09 ` brian m. carlson 2018-04-30 3:30 ` Junio C Hamano 2018-05-01 0:02 ` [PATCH] format-patch: make cover letters always text/plain brian m. carlson 0 siblings, 2 replies; 8+ messages in thread From: brian m. carlson @ 2018-04-30 2:09 UTC (permalink / raw) To: Patrick Hemmer; +Cc: git [-- Attachment #1: Type: text/plain, Size: 596 bytes --] On Sun, Apr 29, 2018 at 09:40:13PM -0400, Patrick Hemmer wrote: > When you use `git format-patch --cover-letter --attach`, the cover > letter does not have the trailing MIME boundary. RFC2046 states that the > last part must be followed by a closing boundary. This causes some email > clients (Thunderbird in my case) to discard the message body. > This is experienced with git 2.16.3. Thanks for reporting this. I can confirm this with a reasonably recent next. Let me see if I can come up with a patch. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 867 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Bug: format-patch MIME boundary not added to cover letter when attach enabled 2018-04-30 2:09 ` brian m. carlson @ 2018-04-30 3:30 ` Junio C Hamano 2018-04-30 11:59 ` brian m. carlson 2018-05-01 0:02 ` [PATCH] format-patch: make cover letters always text/plain brian m. carlson 1 sibling, 1 reply; 8+ messages in thread From: Junio C Hamano @ 2018-04-30 3:30 UTC (permalink / raw) To: brian m. carlson; +Cc: Patrick Hemmer, git "brian m. carlson" <sandals@crustytoothpaste.net> writes: > On Sun, Apr 29, 2018 at 09:40:13PM -0400, Patrick Hemmer wrote: >> When you use `git format-patch --cover-letter --attach`, the cover >> letter does not have the trailing MIME boundary. RFC2046 states that the >> last part must be followed by a closing boundary. This causes some email >> clients (Thunderbird in my case) to discard the message body. >> This is experienced with git 2.16.3. > > Thanks for reporting this. I can confirm this with a reasonably recent > next. Let me see if I can come up with a patch. Thanks. It is true that the current output from the tool is corrupt mime multi-part, and we need to do something about it. I however have to wonder if it even makes sense for --cover to pay attention to --attach and produce the cover template that has "BLURB HERE" etc. in a multi-part format. Shouldn't we be making a simple plain text file instead? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Bug: format-patch MIME boundary not added to cover letter when attach enabled 2018-04-30 3:30 ` Junio C Hamano @ 2018-04-30 11:59 ` brian m. carlson 0 siblings, 0 replies; 8+ messages in thread From: brian m. carlson @ 2018-04-30 11:59 UTC (permalink / raw) To: Junio C Hamano; +Cc: Patrick Hemmer, git [-- Attachment #1: Type: text/plain, Size: 837 bytes --] On Mon, Apr 30, 2018 at 12:30:57PM +0900, Junio C Hamano wrote: > Thanks. It is true that the current output from the tool is corrupt > mime multi-part, and we need to do something about it. > > I however have to wonder if it even makes sense for --cover to pay > attention to --attach and produce the cover template that has "BLURB > HERE" etc. in a multi-part format. Shouldn't we be making a simple > plain text file instead? I agree that multipart/mixed is not a useful content-type for only one plain text part. I have a patch to add the trailing boundary, but I think you make a good argument that perhaps omitting the entire multipart portion would be better. I'll have to work on this after work, so expect a patch later today. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 867 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] format-patch: make cover letters always text/plain 2018-04-30 2:09 ` brian m. carlson 2018-04-30 3:30 ` Junio C Hamano @ 2018-05-01 0:02 ` brian m. carlson 2018-05-01 1:53 ` Eric Sunshine 2018-05-02 2:20 ` [PATCH v2] " brian m. carlson 1 sibling, 2 replies; 8+ messages in thread From: brian m. carlson @ 2018-05-01 0:02 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Patrick Hemmer When formatting a series of patches using --attach and --cover-letter, the cover letter lacks the closing MIME boundary, violating RFC 2046. Certain clients, such as Thunderbird, discard the message body in such a case. Since the cover letter is just one part and sending it as multipart/mixed is not very useful, always emit it as text/plain, avoiding the boundary problem altogether. Reported-by: Patrick Hemmer <git@stormcloud9.net> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- builtin/log.c | 2 +- log-tree.c | 7 ++++--- log-tree.h | 3 ++- t/t4014-format-patch.sh | 9 +++++++++ 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 71f68a3e4f..24868ed070 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1019,7 +1019,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, open_next_file(NULL, rev->numbered_files ? NULL : "cover-letter", rev, quiet)) return; - log_write_email_headers(rev, head, &pp.after_subject, &need_8bit_cte); + log_write_email_headers(rev, head, &pp.after_subject, &need_8bit_cte, 0); for (i = 0; !need_8bit_cte && i < nr; i++) { const char *buf = get_commit_buffer(list[i], NULL); diff --git a/log-tree.c b/log-tree.c index d1c0bedf24..9f5eb346a4 100644 --- a/log-tree.c +++ b/log-tree.c @@ -362,7 +362,8 @@ void fmt_output_email_subject(struct strbuf *sb, struct rev_info *opt) void log_write_email_headers(struct rev_info *opt, struct commit *commit, const char **extra_headers_p, - int *need_8bit_cte_p) + int *need_8bit_cte_p, + int maybe_multipart) { const char *extra_headers = opt->extra_headers; const char *name = oid_to_hex(opt->zero_commit ? @@ -385,7 +386,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit, opt->ref_message_ids->items[i].string); graph_show_oneline(opt->graph); } - if (opt->mime_boundary) { + if (opt->mime_boundary && maybe_multipart) { static char subject_buffer[1024]; static char buffer[1024]; struct strbuf filename = STRBUF_INIT; @@ -610,7 +611,7 @@ void show_log(struct rev_info *opt) if (cmit_fmt_is_mail(opt->commit_format)) { log_write_email_headers(opt, commit, &extra_headers, - &ctx.need_8bit_cte); + &ctx.need_8bit_cte, 1); ctx.rev = opt; ctx.print_email_subject = 1; } else if (opt->commit_format != CMIT_FMT_USERFORMAT) { diff --git a/log-tree.h b/log-tree.h index deba035187..e668628074 100644 --- a/log-tree.h +++ b/log-tree.h @@ -27,7 +27,8 @@ void format_decorations_extended(struct strbuf *sb, const struct commit *commit, void show_decorations(struct rev_info *opt, struct commit *commit); void log_write_email_headers(struct rev_info *opt, struct commit *commit, const char **extra_headers_p, - int *need_8bit_cte_p); + int *need_8bit_cte_p, + int maybe_multipart); void load_ref_decorations(struct decoration_filter *filter, int flags); #define FORMAT_PATCH_NAME_MAX 64 diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 482112ca33..83c596a842 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1661,6 +1661,15 @@ test_expect_success 'format-patch --base with --attach' ' test_write_lines 1 2 >expect && test_cmp expect actual ' +test_expect_success 'format-patch --attach cover-letter only is non-multipart' ' + test_when_finished "rm -r patches" && + git format-patch -o patches --cover-letter --attach=mimemime --base=HEAD~ -1 && + ! egrep "^--+mimemime" patches/0000*.patch && + egrep "^--+mimemime$" patches/0001*.patch >output && + test_line_count = 2 output && + egrep "^--+mimemime--$" patches/0001*.patch >output && + test_line_count = 1 output +' test_expect_success 'format-patch --pretty=mboxrd' ' sp=" " && ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] format-patch: make cover letters always text/plain 2018-05-01 0:02 ` [PATCH] format-patch: make cover letters always text/plain brian m. carlson @ 2018-05-01 1:53 ` Eric Sunshine 2018-05-02 0:16 ` brian m. carlson 2018-05-02 2:20 ` [PATCH v2] " brian m. carlson 1 sibling, 1 reply; 8+ messages in thread From: Eric Sunshine @ 2018-05-01 1:53 UTC (permalink / raw) To: brian m. carlson; +Cc: Git List, Junio C Hamano, Patrick Hemmer On Mon, Apr 30, 2018 at 8:02 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote: > When formatting a series of patches using --attach and --cover-letter, > the cover letter lacks the closing MIME boundary, violating RFC 2046. > Certain clients, such as Thunderbird, discard the message body in such a > case. > > Since the cover letter is just one part and sending it as > multipart/mixed is not very useful, always emit it as text/plain, > avoiding the boundary problem altogether. > > Reported-by: Patrick Hemmer <git@stormcloud9.net> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> > --- > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > @@ -1661,6 +1661,15 @@ test_expect_success 'format-patch --base with --attach' ' > +test_expect_success 'format-patch --attach cover-letter only is non-multipart' ' > + test_when_finished "rm -r patches" && > + git format-patch -o patches --cover-letter --attach=mimemime --base=HEAD~ -1 && Nit: "rm -rf" would be a bit more robust against git-format-patch somehow crashing before creating the "patches" directory. > + ! egrep "^--+mimemime" patches/0000*.patch && > + egrep "^--+mimemime$" patches/0001*.patch >output && > + test_line_count = 2 output && > + egrep "^--+mimemime--$" patches/0001*.patch >output && > + test_line_count = 1 output > +' ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] format-patch: make cover letters always text/plain 2018-05-01 1:53 ` Eric Sunshine @ 2018-05-02 0:16 ` brian m. carlson 0 siblings, 0 replies; 8+ messages in thread From: brian m. carlson @ 2018-05-02 0:16 UTC (permalink / raw) To: Eric Sunshine; +Cc: Git List, Junio C Hamano, Patrick Hemmer [-- Attachment #1: Type: text/plain, Size: 1490 bytes --] On Mon, Apr 30, 2018 at 09:53:33PM -0400, Eric Sunshine wrote: > On Mon, Apr 30, 2018 at 8:02 PM, brian m. carlson > <sandals@crustytoothpaste.net> wrote: > > When formatting a series of patches using --attach and --cover-letter, > > the cover letter lacks the closing MIME boundary, violating RFC 2046. > > Certain clients, such as Thunderbird, discard the message body in such a > > case. > > > > Since the cover letter is just one part and sending it as > > multipart/mixed is not very useful, always emit it as text/plain, > > avoiding the boundary problem altogether. > > > > Reported-by: Patrick Hemmer <git@stormcloud9.net> > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> > > --- > > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > > @@ -1661,6 +1661,15 @@ test_expect_success 'format-patch --base with --attach' ' > > +test_expect_success 'format-patch --attach cover-letter only is non-multipart' ' > > + test_when_finished "rm -r patches" && > > + git format-patch -o patches --cover-letter --attach=mimemime --base=HEAD~ -1 && > > Nit: "rm -rf" would be a bit more robust against git-format-patch > somehow crashing before creating the "patches" directory. Sure, I can reroll with that change. I had considered doing that, but decided against it. I hadn't thought of resilience against a failed git format-patch, though. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 867 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] format-patch: make cover letters always text/plain 2018-05-01 0:02 ` [PATCH] format-patch: make cover letters always text/plain brian m. carlson 2018-05-01 1:53 ` Eric Sunshine @ 2018-05-02 2:20 ` brian m. carlson 1 sibling, 0 replies; 8+ messages in thread From: brian m. carlson @ 2018-05-02 2:20 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Patrick Hemmer, Eric Sunshine When formatting a series of patches using --attach and --cover-letter, the cover letter lacks the closing MIME boundary, violating RFC 2046. Certain clients, such as Thunderbird, discard the message body in such a case. Since the cover letter is just one part and sending it as multipart/mixed is not very useful, always emit it as text/plain, avoiding the boundary problem altogether. Reported-by: Patrick Hemmer <git@stormcloud9.net> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- Changes from v1: * Switch from rm -r to rm -fr. builtin/log.c | 2 +- log-tree.c | 7 ++++--- log-tree.h | 3 ++- t/t4014-format-patch.sh | 9 +++++++++ 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 71f68a3e4f..24868ed070 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1019,7 +1019,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, open_next_file(NULL, rev->numbered_files ? NULL : "cover-letter", rev, quiet)) return; - log_write_email_headers(rev, head, &pp.after_subject, &need_8bit_cte); + log_write_email_headers(rev, head, &pp.after_subject, &need_8bit_cte, 0); for (i = 0; !need_8bit_cte && i < nr; i++) { const char *buf = get_commit_buffer(list[i], NULL); diff --git a/log-tree.c b/log-tree.c index d1c0bedf24..9f5eb346a4 100644 --- a/log-tree.c +++ b/log-tree.c @@ -362,7 +362,8 @@ void fmt_output_email_subject(struct strbuf *sb, struct rev_info *opt) void log_write_email_headers(struct rev_info *opt, struct commit *commit, const char **extra_headers_p, - int *need_8bit_cte_p) + int *need_8bit_cte_p, + int maybe_multipart) { const char *extra_headers = opt->extra_headers; const char *name = oid_to_hex(opt->zero_commit ? @@ -385,7 +386,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit, opt->ref_message_ids->items[i].string); graph_show_oneline(opt->graph); } - if (opt->mime_boundary) { + if (opt->mime_boundary && maybe_multipart) { static char subject_buffer[1024]; static char buffer[1024]; struct strbuf filename = STRBUF_INIT; @@ -610,7 +611,7 @@ void show_log(struct rev_info *opt) if (cmit_fmt_is_mail(opt->commit_format)) { log_write_email_headers(opt, commit, &extra_headers, - &ctx.need_8bit_cte); + &ctx.need_8bit_cte, 1); ctx.rev = opt; ctx.print_email_subject = 1; } else if (opt->commit_format != CMIT_FMT_USERFORMAT) { diff --git a/log-tree.h b/log-tree.h index deba035187..e668628074 100644 --- a/log-tree.h +++ b/log-tree.h @@ -27,7 +27,8 @@ void format_decorations_extended(struct strbuf *sb, const struct commit *commit, void show_decorations(struct rev_info *opt, struct commit *commit); void log_write_email_headers(struct rev_info *opt, struct commit *commit, const char **extra_headers_p, - int *need_8bit_cte_p); + int *need_8bit_cte_p, + int maybe_multipart); void load_ref_decorations(struct decoration_filter *filter, int flags); #define FORMAT_PATCH_NAME_MAX 64 diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 482112ca33..6ea08fd5e9 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1661,6 +1661,15 @@ test_expect_success 'format-patch --base with --attach' ' test_write_lines 1 2 >expect && test_cmp expect actual ' +test_expect_success 'format-patch --attach cover-letter only is non-multipart' ' + test_when_finished "rm -fr patches" && + git format-patch -o patches --cover-letter --attach=mimemime --base=HEAD~ -1 && + ! egrep "^--+mimemime" patches/0000*.patch && + egrep "^--+mimemime$" patches/0001*.patch >output && + test_line_count = 2 output && + egrep "^--+mimemime--$" patches/0001*.patch >output && + test_line_count = 1 output +' test_expect_success 'format-patch --pretty=mboxrd' ' sp=" " && ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-05-02 2:21 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-30 1:40 Bug: format-patch MIME boundary not added to cover letter when attach enabled Patrick Hemmer 2018-04-30 2:09 ` brian m. carlson 2018-04-30 3:30 ` Junio C Hamano 2018-04-30 11:59 ` brian m. carlson 2018-05-01 0:02 ` [PATCH] format-patch: make cover letters always text/plain brian m. carlson 2018-05-01 1:53 ` Eric Sunshine 2018-05-02 0:16 ` brian m. carlson 2018-05-02 2:20 ` [PATCH v2] " brian m. carlson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).