name parameter for content-type for attachments should be last

Bug #194642 reported by Michael B. Trausch
4
Affects Status Importance Assigned to Milestone
Evolution
Fix Released
Medium
evolution (Ubuntu)
Invalid
Low
Ubuntu Desktop Bugs
evolution-data-server (Ubuntu)
Fix Released
Low
Unassigned

Bug Description

To work around broken servers which assume that the name parameter should come last when there is a file name present in the Content-Type header (and the server does not use the Content-Disposition header), the name parameter should come last. For example:

Content-Disposition: attachment; filename=CheckpointTextPrintingProgramTrausch1.java
Content-Transfer-Encoding: base64
Content-Type: text/x-java; name=CheckpointTextPrintingProgramTrausch1.java; charset=UTF-8

Should really be:

Content-Disposition: attachment; filename=CheckpointTextPrintingProgramTrausch1.java
Content-Transfer-Encoding: base64
Content-Type: text/x-java; charset=UTF-8; name=CheckpointTextPrintingProgramTrausch1.java

In the case of this particular attachment, the message was processed improperly by the server and came back with:

Content-Type: text/x-java; name="CheckpointTextPrintingProgramTrausch1.java; charset=UTF-8"
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="CheckpointTextPrintingProgramTrausch1.java; charset=UTF-8"

This is, in particular, to work around a bug in the NNTP gateway in the Jive Forums software. There may be other server-side implementations which make this dangerous assumption, though, as well, and it would be well to work around those, too.

I cannot find where in the Evolution sources to make this change, because I don’t know C that well, and I have a hard time trying to figure out what any component of the rather large source base is doing at any single point. I would be submitting a patch if I could figure out where to fix the problem.

Revision history for this message
Michael B. Trausch (mtrausch) wrote :

This is a (very bad) workaround which I used to try to find the root of the problem.

I say that this is a very bad workaround, because it simply refuses to send a charset parameter for text/x-java files (and anything other than text/plain, which seemed like a good test case for me since I am not sending text/plain attachments in this situation). This patch SHOULD NOT BE USED by anyone other than to confirm that this works around the problem in the description.

It looks like that the real workaround would have to be to modify evolution-data-server, file camel/camel-mime-utils.c, function camel_header_param_list_format_append(), such that it caches the name= parameter when present and appends it last, when there are multiple parameters passed to it for formatting. In light of this finding, I am going to add EDS to the affected packages momentarily.

Changed in evolution:
status: Unknown → New
Changed in evolution:
assignee: nobody → desktop-bugs
importance: Undecided → Low
status: New → Triaged
Changed in evolution-data-server:
assignee: nobody → desktop-bugs
importance: Undecided → Low
status: New → Triaged
Revision history for this message
C de-Avillez (hggdh2) wrote :

by the way, Michael. Thanks for helping out. We really appreciate it.

Revision history for this message
C de-Avillez (hggdh2) wrote :

rejecting Evolution, since the code change is e-d-s related (on the camel* code, it seems).

Changed in evolution:
status: Triaged → Invalid
Revision history for this message
Michael B. Trausch (mtrausch) wrote :

Here is the proper fix, against EDS.

Created a new helper function (_camel_header_param_format_append) to handle the processing of MIME field parameters, and altered the main function (camel_header_param_list_format_append) to use the new helper and defer processing of the name= MIME field until last.

This debdiff places the actual patch in debian/patches, and I am submitting the patch to upstream so that it can be included there, as well, so hopefully the local patch will be able to go away soon.

Revision history for this message
Michael B. Trausch (mtrausch) wrote :

The policy for requesting sponsorship says to mark the bug as "confirmed", but that would seem to downgrade the bug's status. Should I do so anyway, before requesting sponsorship?

Revision history for this message
C de-Avillez (hggdh2) wrote :

Michael, I am not sure. "Confirmed" was used of old for "this is as far as bugsquad can go"; we later went on to "Triaged" meaning that. I would keep it how it is now, and see if a sponsor complains...

Also -- it might be a good idea to show the output of your change, and state if you checked it against "normal" servers. Upstream will certainly ask for this.

On my immense ignorance of Evo internals, the patch still looks good... thanks again.

Revision history for this message
Michael B. Trausch (mtrausch) wrote : Re: [Bug 194642] Re: name parameter for content-type for attachments should be last

On Fri, 2008-03-07 at 15:29 +0000, hggdh wrote:
> Michael, I am not sure. "Confirmed" was used of old for "this is as
> far as bugsquad can go"; we later went on to "Triaged" meaning that. I
> would keep it how it is now, and see if a sponsor complains...
>
> Also -- it might be a good idea to show the output of your change, and
> state if you checked it against "normal" servers. Upstream will
> certainly ask for this.

Indeed. This is what the header now looks like when re-arranged (I am
pasting these from a 40 MB message I just put together to showcase a
bunch of different types of files.

Content-Type: text/x-java; charset=UTF-8; name=WeeklyPay.java
Content-Type: text/x-csharp; charset=UTF-8; name=fsw.cs
Content-Type: application/x-ms-dos-executable; name=threading.exe
Content-Type: image/png; name=mbt
Content-Type: application/pgp-signature; name=signature.asc
Content-Type: application/pgp-signature; name=UbuntuCodeofConduct-1.0.1.txt.asc
Content-Type: audio/x-vorbis+ogg; name=TWiT0134.ogg

I have been using this patch for several days now and everything appears
to be working just fine; the broken server that I am working around now
silently drops charset MIME header parameters, but no other server I
have sent mail through has done that sort of thing, to my knowledge.

> On my immense ignorance of Evo internals, the patch still looks
> good...
> thanks again.

LOL, you and me both. The good news (and this is praise for the
authors/maintainers of Evolution) is that the source is well organized
enough that I only had to spend probably 30 minutes narrowing down the
code that needed to be modified. I was made quite happy by that.

 --- Mike

--
Michael B. Trausch <email address hidden>
home: 404-592-5746, 1 www.trausch.us
cell: 678-522-7934 im: <email address hidden>, jabber
Ubuntu Unofficial Backports Project: http://backports.trausch.us/

Revision history for this message
Michael B. Trausch (mtrausch) wrote :

It may also be prudent to reiterate that the changes made remain compliant with MIME Part One, RFC 2045, §5, ¶ 3, which states:

"The Content-Type header field specifies the nature of the data in the body of an entity by giving media type and subtype identifiers, and by providing auxiliary information that may be required for certain media types. After the media type and subtype names, the remainder of the header field is simply a set of parameters, specified in an attribute=value notation. The ordering of parameters is not significant."

Since the only change is the ordering to work around nasty parsers in existing software that breaks this portion of the standard, there _should_ be no additional side effects. To date, I have observed none.

Revision history for this message
Michael B. Trausch (mtrausch) wrote :

Removing assignment on bug per sponsorship request process at https://wiki.ubuntu.com/SponsorshipProcess

Changed in evolution-data-server:
assignee: desktop-bugs → nobody
Revision history for this message
C de-Avillez (hggdh2) wrote :

Michael, I will also apply your fix, and see what happens.

Revision history for this message
Michael B. Trausch (mtrausch) wrote :

Updated patch to apply for newly released e-d-s.

Revision history for this message
Daniel Holbach (dholbach) wrote :

Sébastian: could you please take a look at it?

Revision history for this message
Sebastien Bacher (seb128) wrote :

would be better to have upstream opinion on the patch before using it, if the change is correct no reason it could not go to their svn, unsubscribing the sponsor team for now

Revision history for this message
Michael B. Trausch (mtrausch) wrote :

Updating for latest release pushed out recently.

Changed in evolution:
status: New → Fix Released
Revision history for this message
Michael B. Trausch (mtrausch) wrote :

Upstream has committed a patch for this fix; will be providing a debdiff with that patch shortly so that this issue can be closed here in Ubuntu as well.

Changed in evolution-data-server:
assignee: nobody → mtrausch
status: Triaged → In Progress
Revision history for this message
C de-Avillez (hggdh2) wrote :

Michael, any updates here? Thanks.

Revision history for this message
Michael B. Trausch (mtrausch) wrote :

On Mon, 2008-07-14 at 16:25 +0000, hggdh wrote:
> Michael, any updates here? Thanks.

Yep. Sorry, have been buried with all sorts of things, and just
(finally) graduated school.

I will have a debdiff today for both Hardy and Intrepid to close this
bug.

 --- Mike

--
My sigfile ran away and is on hiatus.

Revision history for this message
Michael B. Trausch (mtrausch) wrote :

https://edge.launchpad.net/~mtrausch/+archive --- I have a binary built there which comes from this attached patch (though this patch does not have the ~ppa1 version postfix on it).

This patch is already included upstream, so probably doesn't need to be directly included into Intrepid. It should probably be pushed out for Hardy, if that's possible at all at this point. Something tells me, though, that it probably isn't.

Revision history for this message
Michael B. Trausch (mtrausch) wrote :

Patch has been submitted here and a build with that patch enabled is available on my PPA; removing bug assignment to me.

Changed in evolution-data-server:
assignee: mtrausch → nobody
status: In Progress → Confirmed
Revision history for this message
Michael B. Trausch (mtrausch) wrote :

It would seem that the version that I put together was using the _previous_ released version that I had already on my computer.

This patch is based on the version currently in Hardy as the latest available. A binary build of it is in my PPA, with the ~ppa1 package version suffix.

Revision history for this message
Michael B. Trausch (mtrausch) wrote :

This fix is included in Ubuntu for Intrepid.

Changed in evolution-data-server:
status: Confirmed → Fix Released
Changed in evolution:
importance: Unknown → Medium
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.