Re: mailinfo: don't require text mime type for attachments

2012-10-01 Thread Don Zickus
On Sun, Sep 30, 2012 at 03:10:48PM -0700, Linus Torvalds wrote:
 This code goes all the way back to March of 2007, in commit 87ab79923463 
 (builtin-mailinfo.c infrastrcture changes), and apparently Don used to 
 pass random mbox contents to git. However, the pre-decode vs post-decode 
 logic really shouldn't matter even for that case, and more importantly, I 
 fed git am crap is not a valid reason to break *real* patch attachments.
 
 If somebody really cares, and determines that some attachment is binary 
 data (by looking at the data, not the MIME-type), the whole attachment 
 should be dismissed, rather than fed in random-sized chunks to 
 handle_filter().

Heh.  Years ago when I tried using git as a patch-control-management
system instead of a traditional SCM,  I fed my custom git-am script an
internal kernel-mail-archives list to help process the meta data for
patches (acks, nacks, needinfo, bugzillas, etc).  It served its purpose
until we switched to a fork'd copy of patch-work.

So I haven't done 'insane' stuff in years.  :-)  I'm sure this patch is
right, but it doesn't affect me anymore.

Sorry for any problems that arose..

Cheers,
Don
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


mailinfo: don't require text mime type for attachments

2012-09-30 Thread Linus Torvalds

Currently git am does insane things if the mbox it is given contains 
attachments with a MIME type that aren't text/*.

In particular, it will still decode them, and pass them one line at a 
time to the mail body filter, but because it has determined that they 
aren't text (without actually looking at the contents, just at the mime 
type) the line will be the encoding line (eg 'base64') rather than a 
line of *content*.

Which then will cause the text filtering to fail, because we won't 
correctly notice when the attachment text switches from the commit message 
to the actual patch. Resulting in a patch failure, even if patch may be a 
perfectly well-formed attachment, it's just that the message type may be 
(for example) application/octet-stream instead of text/plain.

Just remove all the bogus games with the message_type. The only difference 
that code creates is how the data is passed to the filter function 
(chunked per-pred-code line or per post-decode line), and that difference 
is *wrong*, since chunking things per pre-decode line can never be a 
sensible operation, and cannot possibly matter for binary data anyway.

This code goes all the way back to March of 2007, in commit 87ab79923463 
(builtin-mailinfo.c infrastrcture changes), and apparently Don used to 
pass random mbox contents to git. However, the pre-decode vs post-decode 
logic really shouldn't matter even for that case, and more importantly, I 
fed git am crap is not a valid reason to break *real* patch attachments.

If somebody really cares, and determines that some attachment is binary 
data (by looking at the data, not the MIME-type), the whole attachment 
should be dismissed, rather than fed in random-sized chunks to 
handle_filter().

Signed-off-by: Linus Torvalds torva...@linux-foundation.org
Cc: Don Zickus dzic...@redhat.com
---
 builtin/mailinfo.c | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 2b3f4d955eaa..da231400b327 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -19,9 +19,6 @@ static struct strbuf email = STRBUF_INIT;
 static enum  {
TE_DONTCARE, TE_QP, TE_BASE64
 } transfer_encoding;
-static enum  {
-   TYPE_TEXT, TYPE_OTHER
-} message_type;
 
 static struct strbuf charset = STRBUF_INIT;
 static int patch_lines;
@@ -184,8 +181,6 @@ static void handle_content_type(struct strbuf *line)
struct strbuf *boundary = xmalloc(sizeof(struct strbuf));
strbuf_init(boundary, line-len);
 
-   if (!strcasestr(line-buf, text/))
-message_type = TYPE_OTHER;
if (slurp_attr(line-buf, boundary=, boundary)) {
strbuf_insert(boundary, 0, --, 2);
if (++content_top  content[MAX_BOUNDARIES]) {
@@ -657,7 +652,6 @@ again:
/* set some defaults */
transfer_encoding = TE_DONTCARE;
strbuf_reset(charset);
-   message_type = TYPE_TEXT;
 
/* slurp in this section's info */
while (read_one_header_line(line, fin))
@@ -871,11 +865,6 @@ static void handle_body(void)
strbuf_insert(line, 0, prev.buf, prev.len);
strbuf_reset(prev);
 
-   /* binary data most likely doesn't have newlines */
-   if (message_type != TYPE_TEXT) {
-   handle_filter(line);
-   break;
-   }
/*
 * This is a decoded line that may contain
 * multiple new lines.  Pass only one chunk
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html