Skip to content

Commit

Permalink
mailinfo: fix MIME multi-part message boundary handling
Browse files Browse the repository at this point in the history
After finding a MIME multi-part message boundary line, the handle_body()
function is supposed to first flush any accumulated contents from the
previous part to the output stream.  However, the code mistakenly output
the boundary line it found.

The old code that used one global, fixed-length buffer line[] used an
alternate static buffer newline[] for keeping track of this accumulated
contents and flushed newline[] upon seeing the boundary; when 3b6121f
(git-mailinfo: use strbuf's instead of fixed buffers, 2008-07-13)
converted a fixed-length buffer in this program to use strbuf,these two
buffers were converted to "line" and "prev" (the latter of which now has a
much more sensible name) strbufs, but the code mistakenly flushed "line"
(which contains the boundary we have just found), instead of "prev".

This resulted in the first boundary to be output in front of the first
line of the message.

The rewritten implementation of handle_boundary() lost the terminating
newline; this would then result in the second line of the message to be
stuck with the first line.

The is_multipart_boundary() was designed to catch both the internal
boundary and the terminating one (the one with trailing "--"); this also
was broken with the rewrite, and the code in the handle_boundary() to
handle the terminating boundary was never triggered.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Junio C Hamano committed Aug 9, 2008
1 parent 01144f2 commit a9fd138
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 7 deletions.
17 changes: 11 additions & 6 deletions builtin-mailinfo.c
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ static void handle_content_transfer_encoding(const struct strbuf *line)

static int is_multipart_boundary(const struct strbuf *line)
{
return !strbuf_cmp(line, *content_top);
return (((*content_top)->len <= line->len) &&
!memcmp(line->buf, (*content_top)->buf, (*content_top)->len));
}

static void cleanup_subject(struct strbuf *subject)
Expand Down Expand Up @@ -649,8 +650,11 @@ static int handle_boundary(void)
check_header(&line, p_hdr_data, 0);

strbuf_release(&newline);
/* eat the blank line after section info */
return (strbuf_getline(&line, fin, '\n') == 0);
/* replenish line */
if (strbuf_getline(&line, fin, '\n'))
return 0;
strbuf_addch(&line, '\n');
return 1;
}

static inline int patchbreak(const struct strbuf *line)
Expand Down Expand Up @@ -757,9 +761,10 @@ static void handle_body(void)
/* process any boundary lines */
if (*content_top && is_multipart_boundary(&line)) {
/* flush any leftover */
if (line.len)
handle_filter(&line);

if (prev.len) {
handle_filter(&prev);
strbuf_reset(&prev);
}
if (!handle_boundary())
goto handle_body_out;
}
Expand Down
2 changes: 1 addition & 1 deletion t/t5100-mailinfo.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ test_expect_success 'split sample box' \
'git mailsplit -o. ../t5100/sample.mbox >last &&
last=`cat last` &&
echo total is $last &&
test `cat last` = 10'
test `cat last` = 11'

for mail in `echo 00*`
do
Expand Down
5 changes: 5 additions & 0 deletions t/t5100/info0011
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Author: A U Thor
Email: a.u.thor@example.com
Subject: Xyzzy
Date: Fri, 8 Aug 2008 13:08:37 +0200 (CEST)

2 changes: 2 additions & 0 deletions t/t5100/msg0011
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Here comes a commit log message, and
its second line is here.
22 changes: 22 additions & 0 deletions t/t5100/patch0011
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
builtin-mailinfo.c | 4 ++--

diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index 3e5fe51..aabfe5c 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -758,8 +758,8 @@ static void handle_body(void)
/* process any boundary lines */
if (*content_top && is_multipart_boundary(&line)) {
/* flush any leftover */
- if (line.len)
- handle_filter(&line);
+ if (prev.len)
+ handle_filter(&prev);

if (!handle_boundary())
goto handle_body_out;
--
1.6.0.rc2


35 changes: 35 additions & 0 deletions t/t5100/sample.mbox
Original file line number Diff line number Diff line change
Expand Up @@ -465,3 +465,38 @@ index 962aa34..2d1520f 100644
--
1.5.6.2.455.g1efb2

From nobody Fri Aug 8 22:24:03 2008
Date: Fri, 8 Aug 2008 13:08:37 +0200 (CEST)
From: A U Thor <a.u.thor@example.com>
Subject: [PATCH 3/3 v2] Xyzzy
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
--=-=-=
Content-Type: text/plain; charset=iso-8859-15
Content-Transfer-Encoding: quoted-printable
Here comes a commit log message, and
its second line is here.
---
builtin-mailinfo.c | 4 ++--

diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index 3e5fe51..aabfe5c 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -758,8 +758,8 @@ static void handle_body(void)
/* process any boundary lines */
if (*content_top && is_multipart_boundary(&line)) {
/* flush any leftover */
- if (line.len)
- handle_filter(&line);
+ if (prev.len)
+ handle_filter(&prev);
=20
if (!handle_boundary())
goto handle_body_out;
--=20
1.6.0.rc2

--=-=-=--

0 comments on commit a9fd138

Please sign in to comment.