{"thread":{"id":"14904","subject":"[PATCH] mailinfo: fix MIME multi-part message boundary handling","startedAt":"2008-08-09T08:33:59Z","lastAt":"2008-08-09T08:33:59Z","messageCount":1,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"86587","messageId":"7vy7367fk8.fsf@gitster.siamese.dyndns.org","threadId":"14904","inReplyTo":null,"subject":"[PATCH] mailinfo: fix MIME multi-part message boundary handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-09T08:33:59Z","receivedAt":"2008-08-09T08:33:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"After finding a MIME multi-part message boundary line, the handle_body()\nfunction is supposed to first flush any accumulated contents from the\nprevious part to the output stream.  However, the code mistakenly output\nthe boundary line it found.\n\nThe old code that used one global, fixed-length buffer line[] used an\nalternate static buffer newline[] for keeping track of this accumulated\ncontents and flushed newline[] upon seeing the boundary; when 3b6121f\n(git-mailinfo: use strbuf's instead of fixed buffers, 2008-07-13)\nconverted a fixed-length buffer in this program to use strbuf,these two\nbuffers were converted to \"line\" and \"prev\" (the latter of which now has a\nmuch more sensible name) strbufs, but the code mistakenly flushed \"line\"\n(which contains the boundary we have just found), instead of \"prev\".\n\nThis resulted in the first boundary to be output in front of the first\nline of the message.\n\nThe rewritten implementation of handle_boundary() lost the terminating\nnewline; this would then result in the second line of the message to be\nstuck with the first line.\n\nThe is_multipart_boundary() was designed to catch both the internal\nboundary and the terminating one (the one with trailing \"--\"); this also\nwas broken with the rewrite, and the code in the handle_boundary() to\nhandle the terminating boundary was never triggered.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-mailinfo.c  |   17 +++++++++++------\n t/t5100-mailinfo.sh |    2 +-\n t/t5100/info0011    |    5 +++++\n t/t5100/msg0011     |    2 ++\n t/t5100/patch0011   |   22 ++++++++++++++++++++++\n t/t5100/sample.mbox |   35 +++++++++++++++++++++++++++++++++++\n 6 files changed, 76 insertions(+), 7 deletions(-)\n create mode 100644 t/t5100/info0011\n create mode 100644 t/t5100/msg0011\n create mode 100644 t/t5100/patch0011\n\ndiff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\nindex f974b9d..3577382 100644\n--- a/builtin-mailinfo.c\n+++ b/builtin-mailinfo.c\n@@ -203,7 +203,8 @@ static void handle_content_transfer_encoding(const struct strbuf *line)\n \n static int is_multipart_boundary(const struct strbuf *line)\n {\n-\treturn !strbuf_cmp(line, *content_top);\n+\treturn (((*content_top)->len <= line->len) &&\n+\t\t!memcmp(line->buf, (*content_top)->buf, (*content_top)->len));\n }\n \n static void cleanup_subject(struct strbuf *subject)\n@@ -649,8 +650,11 @@ again:\n \t\tcheck_header(&line, p_hdr_data, 0);\n \n \tstrbuf_release(&newline);\n-\t/* eat the blank line after section info */\n-\treturn (strbuf_getline(&line, fin, '\\n') == 0);\n+\t/* replenish line */\n+\tif (strbuf_getline(&line, fin, '\\n'))\n+\t\treturn 0;\n+\tstrbuf_addch(&line, '\\n');\n+\treturn 1;\n }\n \n static inline int patchbreak(const struct strbuf *line)\n@@ -757,9 +761,10 @@ static void handle_body(void)\n \t\t/* process any boundary lines */\n \t\tif (*content_top && is_multipart_boundary(&line)) {\n \t\t\t/* flush any leftover */\n-\t\t\tif (line.len)\n-\t\t\t\thandle_filter(&line);\n-\n+\t\t\tif (prev.len) {\n+\t\t\t\thandle_filter(&prev);\n+\t\t\t\tstrbuf_reset(&prev);\n+\t\t\t}\n \t\t\tif (!handle_boundary())\n \t\t\t\tgoto handle_body_out;\n \t\t}\ndiff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh\nindex e9f3e72..8dfaddd 100755\n--- a/t/t5100-mailinfo.sh\n+++ b/t/t5100-mailinfo.sh\n@@ -11,7 +11,7 @@ test_expect_success 'split sample box' \\\n \t'git mailsplit -o. ../t5100/sample.mbox >last &&\n \tlast=`cat last` &&\n \techo total is $last &&\n-\ttest `cat last` = 10'\n+\ttest `cat last` = 11'\n \n for mail in `echo 00*`\n do\ndiff --git a/t/t5100/info0011 b/t/t5100/info0011\nnew file mode 100644\nindex 0000000..da5a605\n--- /dev/null\n+++ b/t/t5100/info0011\n@@ -0,0 +1,5 @@\n+Author: A U Thor\n+Email: a.u.thor@example.com\n+Subject: Xyzzy\n+Date: Fri, 8 Aug 2008 13:08:37 +0200 (CEST)\n+\ndiff --git a/t/t5100/msg0011 b/t/t5100/msg0011\nnew file mode 100644\nindex 0000000..4667f21\n--- /dev/null\n+++ b/t/t5100/msg0011\n@@ -0,0 +1,2 @@\n+Here comes a commit log message, and\n+its second line is here.\ndiff --git a/t/t5100/patch0011 b/t/t5100/patch0011\nnew file mode 100644\nindex 0000000..8841d3c\n--- /dev/null\n+++ b/t/t5100/patch0011\n@@ -0,0 +1,22 @@\n+---\n+ builtin-mailinfo.c  |    4 ++--\n+\n+diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\n+index 3e5fe51..aabfe5c 100644\n+--- a/builtin-mailinfo.c\n++++ b/builtin-mailinfo.c\n+@@ -758,8 +758,8 @@ static void handle_body(void)\n+ \t\t/* process any boundary lines */\n+ \t\tif (*content_top && is_multipart_boundary(&line)) {\n+ \t\t\t/* flush any leftover */\n+-\t\t\tif (line.len)\n+-\t\t\t\thandle_filter(&line);\n++\t\t\tif (prev.len)\n++\t\t\t\thandle_filter(&prev);\n+ \n+ \t\t\tif (!handle_boundary())\n+ \t\t\t\tgoto handle_body_out;\n+-- \n+1.6.0.rc2\n+\n+\ndiff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox\nindex aba57f9..d7ca79b 100644\n--- a/t/t5100/sample.mbox\n+++ b/t/t5100/sample.mbox\n@@ -465,3 +465,38 @@ index 962aa34..2d1520f 100644\n -- \n 1.5.6.2.455.g1efb2\n \n+From nobody Fri Aug  8 22:24:03 2008\n+Date: Fri, 8 Aug 2008 13:08:37 +0200 (CEST)\n+From: A U Thor <a.u.thor@example.com>\n+Subject: [PATCH 3/3 v2] Xyzzy\n+MIME-Version: 1.0\n+Content-Type: multipart/mixed; boundary=\"=-=-=\"\n+\n+--=-=-=\n+Content-Type: text/plain; charset=iso-8859-15\n+Content-Transfer-Encoding: quoted-printable\n+\n+Here comes a commit log message, and\n+its second line is here.\n+---\n+ builtin-mailinfo.c  |    4 ++--\n+\n+diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\n+index 3e5fe51..aabfe5c 100644\n+--- a/builtin-mailinfo.c\n++++ b/builtin-mailinfo.c\n+@@ -758,8 +758,8 @@ static void handle_body(void)\n+ \t\t/* process any boundary lines */\n+ \t\tif (*content_top && is_multipart_boundary(&line)) {\n+ \t\t\t/* flush any leftover */\n+-\t\t\tif (line.len)\n+-\t\t\t\thandle_filter(&line);\n++\t\t\tif (prev.len)\n++\t\t\t\thandle_filter(&prev);\n+=20\n+ \t\t\tif (!handle_boundary())\n+ \t\t\t\tgoto handle_body_out;\n+--=20\n+1.6.0.rc2\n+\n+--=-=-=--\n-- \n1.6.0.rc2\n"}]}