Re: [bug] notmuch requires Content-Disposition mime header value to be lower case

[Jani Nikula]

On Fri, 18 Sep 2015, David Bremner  wrote:
> Johannes Schauer  writes:
>
>> Hi,
>>
>> Quoting David Bremner (2015-09-18 14:03:20)
>>> I'd recommend making your own new test, rather than modifying existing
>>> ones to test multiple things.  I'd also recommend using json / sexp
>>> output for your tests, since the ad-hoc text format is kindof
>>> semi-deprecated.
>>
>> can you take care of that? My goal was actually just to report this bug, not 
>> to
>> spend more time to develop a proper patch for it :)
>
> Consider the bug reported. At the moment I'm not completely convinced
> the patch is correct, never mind the tests. I agree it looks obvious
> enough, but the test results don't make sense to me yet.
>
>>
>> Also, a related problem occurs when the Content-Disposition header contains
>> UTF8 characters, in which case the header value gets encoded. Apparently
>> notmuch does not attempt to decode it. Example mime header:
>>
>> --===7161366892858136962==
>> Content-Type: application/pdf
>> MIME-Version: 1.0
>> Content-Transfer-Encoding: base64
>> Content-Disposition: =?utf-8?b?YXR0YWNobWVudDsgZmlsZW5hbWU9ImJlZ3LDvMOfdW5n?=
>>  =?utf-8?b?LnBkZiI=?=
>
> yes, that sounds like a distinct bug.

...in the sending end. Correct me if you think I'm wrong, but I don't
think that kind of encoding is allowed in Content-Disposition.

BR,
Jani.


> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [bug] notmuch requires Content-Disposition mime header value to be lower case

[David Bremner]

Johannes Schauer  writes:


> Funnily though there seem to be some weird newline differences that I cannot
> explain, so I left them for somebody else to fix.
>

Looking at the files (.EXPECTED and .OUTPUT) in test/tmp.T190-multipart,
there's more than whitespace changes. Some things that used to marked
"attachment" in the output are now marked "part".

> diff --git a/test/T190-multipart.sh b/test/T190-multipart.sh
> index 7c4c9f7..f16cc90 100755
> --- a/test/T190-multipart.sh
> +++ b/test/T190-multipart.sh
> @@ -48,7 +48,7 @@ cat embedded_message >> ${MAIL_DIR}/multipart
>  cat <> ${MAIL_DIR}/multipart
>  
>  --=-=-=
> -Content-Disposition: attachment; filename=attachment
> +Content-Disposition: ATTACHMENT; FILENAME=attachment
>  
>  This is a text attachment.
>  
> @@ -487,7 +487,7 @@ This is an embedded message, with a multipart/alternative 
> part.
>  --==-=-==--
>  
>  --=-=-=
> -Content-Disposition: attachment; filename=attachment
> +Content-Disposition: ATTACHMENT; FILENAME=attachment
>  
>  This is a text attachment.
>  

I'd recommend making your own new test, rather than modifying existing
ones to test multiple things.  I'd also recommend using json / sexp
output for your tests, since the ad-hoc text format is kindof
semi-deprecated.

d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [bug] notmuch requires Content-Disposition mime header value to be lower case

[David Bremner]

Johannes Schauer  writes:

> Hi,
>
> Quoting David Bremner (2015-09-18 14:03:20)
>> I'd recommend making your own new test, rather than modifying existing
>> ones to test multiple things.  I'd also recommend using json / sexp
>> output for your tests, since the ad-hoc text format is kindof
>> semi-deprecated.
>
> can you take care of that? My goal was actually just to report this bug, not 
> to
> spend more time to develop a proper patch for it :)

Consider the bug reported. At the moment I'm not completely convinced
the patch is correct, never mind the tests. I agree it looks obvious
enough, but the test results don't make sense to me yet.

>
> Also, a related problem occurs when the Content-Disposition header contains
> UTF8 characters, in which case the header value gets encoded. Apparently
> notmuch does not attempt to decode it. Example mime header:
>
> --===7161366892858136962==
> Content-Type: application/pdf
> MIME-Version: 1.0
> Content-Transfer-Encoding: base64
> Content-Disposition: =?utf-8?b?YXR0YWNobWVudDsgZmlsZW5hbWU9ImJlZ3LDvMOfdW5n?=
>  =?utf-8?b?LnBkZiI=?=

yes, that sounds like a distinct bug.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [bug] notmuch requires Content-Disposition mime header value to be lower case

[Johannes Schauer]

Hi,

Quoting David Bremner (2015-09-18 14:03:20)
> I'd recommend making your own new test, rather than modifying existing
> ones to test multiple things.  I'd also recommend using json / sexp
> output for your tests, since the ad-hoc text format is kindof
> semi-deprecated.

can you take care of that? My goal was actually just to report this bug, not to
spend more time to develop a proper patch for it :)

Also, a related problem occurs when the Content-Disposition header contains
UTF8 characters, in which case the header value gets encoded. Apparently
notmuch does not attempt to decode it. Example mime header:

--===7161366892858136962==
Content-Type: application/pdf
MIME-Version: 1.0
Content-Transfer-Encoding: base64
Content-Disposition: =?utf-8?b?YXR0YWNobWVudDsgZmlsZW5hbWU9ImJlZ3LDvMOfdW5n?=
 =?utf-8?b?LnBkZiI=?=



Thanks!

cheers, josch


signature.asc
Description: signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [bug] notmuch requires Content-Disposition mime header value to be lower case

[Johannes Schauer]

Hi,

Quoting Johannes Schauer (2015-09-17 10:36:12)
> Quoting Johannes Schauer (2015-09-17 09:00:56)
> > it seems that notmuch does not put the attachment tag if:
> > 
> > Content-Disposition: ATTACHMENT; FILENAME=flyer-vk-web.pdf
> > 
> > but it works for:
> > 
> > Content-Disposition: attachment; filename=flyer-vk-web.pdf
> > 
> > But rfc1341 says that the value should be treated as case insensitive 
> > (section 2).
> > 
> > I got an email with upper case Content-Disposition value in an email with
> > "User-Agent: Alpine 2.11 (LSU 23 2013-08-11)".
> > 
> > Please CC me as I'm not subscribed - thanks!
> 
> the fix seems to be to:
> 
> --- a/lib/index.cc
> +++ b/lib/index.cc
> @@ -377,7 +377,7 @@ _index_mime_part (notmuch_message_t *message,
>  
>  disposition = g_mime_object_get_content_disposition (part);
>  if (disposition &&
> -   strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> +   strcasecmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) 
> == 0)
>  {
> const char *filename = g_mime_part_get_filename (GMIME_PART (part));
>  
> 
> but then I saw that this was already done in your git.

whoops, I confused my local git repositories.

So the attached git format patch fixes the issue and adds a test case for this.

Funnily though there seem to be some weird newline differences that I cannot
explain, so I left them for somebody else to fix.

Thanks!

cheers, josch
From 8187076ab1ee9ce640cd15e9a214d49039b0f197 Mon Sep 17 00:00:00 2001
From: Johannes 'josch' Schauer 
Date: Thu, 17 Sep 2015 10:39:29 +0200
Subject: [PATCH] allow case-insensitive content-disposition values

---
 lib/index.cc   | 2 +-
 test/T190-multipart.sh | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index e81aa81..34bab4e 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -377,7 +377,7 @@ _index_mime_part (notmuch_message_t *message,
 
 disposition = g_mime_object_get_content_disposition (part);
 if (disposition &&
-	strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
+	strcasecmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
 {
 	const char *filename = g_mime_part_get_filename (GMIME_PART (part));
 
diff --git a/test/T190-multipart.sh b/test/T190-multipart.sh
index 7c4c9f7..f16cc90 100755
--- a/test/T190-multipart.sh
+++ b/test/T190-multipart.sh
@@ -48,7 +48,7 @@ cat embedded_message >> ${MAIL_DIR}/multipart
 cat <> ${MAIL_DIR}/multipart
 
 --=-=-=
-Content-Disposition: attachment; filename=attachment
+Content-Disposition: ATTACHMENT; FILENAME=attachment
 
 This is a text attachment.
 
@@ -487,7 +487,7 @@ This is an embedded message, with a multipart/alternative part.
 --==-=-==--
 
 --=-=-=
-Content-Disposition: attachment; filename=attachment
+Content-Disposition: ATTACHMENT; FILENAME=attachment
 
 This is a text attachment.
 
-- 
2.5.1



signature.asc
Description: signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch