1111724 - [email/backend] Attachments of type text/* (including text/plain) are treated as body parts by mailcomposer.js, potentially breaking composer.js' Blob hack. Correct and verify attachment disposition is always explicit

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Daniel Holbert [:dholbert]]

STR:
 1. Install "Run, Bike, Hike" app from Firefox OS Marketplace:
    https://marketplace.firefox.com/app/run-bike-hike
 2. Record a track using the app. (after the app has established a GPS fix.)
 3. View your track (using upper-right "bulleted-list-hamburger" button from app's start screen, and then clicking on the track)
 4. Click "Share" button.
 5. Choose "Email" from dropdown list on share dialog, and click through to arrive at email app.

  --> IMPORTANT: Note the sizes of the attachments on the email. In my case, there's a 161k GPX file and a 85k JPG file.

 6. Send the email to yourself, and inspect the attachments on the other end.

EXPECTED RESULTS:
The attachments should be valid, and their sizes should match what you saw in step 5, in your email app.

ACTUAL RESULTS:
 - The received GPX attachment only 40 bytes, and its contents are something like:
> {{blob!0.56163309258718531418669397963}}
 - The received JPG attachment is 161 KB, and it appears to contain the actual GPX data. (Note that its size exactly matches the size of the GPX file that I observed in step 5.)  It's not a valid JPG file, and if I view it in a textfile, I can see that it contains GPX XML data.


NOTE: in step 5, if I delete the .gpx file attachment before I send the email, *then* the JPG file gets through successfully (and it's a picture of a map of your route).  I only get problems if I leave the .gpx file on the email.

I originally reported this as a bug in the Run, Bike, Hike app, but I'm now pretty sure it's a bug in the Gaia email app, because the attachments in the email-Compose are shown with the correct sizes, and JPG file is clearly being corrupted by the email app (since it gets sent correctly or incorrectly depending on whether I remove the other attachment).

Comment 1

[Daniel Holbert [:dholbert]]

I'm using Firefox OS 2.1 build, up-to-date as of last night (on nightly update channel).
 Build Identifier: 20141213161202
 Git Commit Info:
   2014-12-10 15:32:51
   97873dca

And I'm using "Run, Bike, Hike" version 0.1.10 (the version currently on marketplace.)

Here's the issue that I filed for this in the app's bug-tracker:
 https://github.com/nicodel/Run-Bike-Hike/issues/77
though as noted at end of comment 0, I'm pretty sure this is really a bug (or several bugs) in Gaia's email app.

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

The GPX Blob is being created with MIME type text/plain.  This is likely resulting in the mailcomposer library thinking that the node is a body part and not an attachment and breaking our Blob re-splicing hack.  This is probably more email.js changeover fallout since before attachments were explicitly indicated as such.  (I'm guessing the line gets wrapped or encoded in such a way that the string replace fails.)

We'll want to add backend tests to verify that we can attach text/plain and text/html attachments and that they roundtrip as attachments.  We may also want expect-style MIME output to verify that they are getting explicit "disposition: attachment" going on.

In the interim, it might be advisable for the Run-Bike-Hike app to indicate the Blob's MIME type is application/gpx+xml, which I understand to be the most correct MIME type.  This will also avoid any app thinking the XML file is a good thing to display inline.  This is probably also the only way this functionality will work on v2.1, release management seems unlikely to grant us any more uplifts to v2.1.

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

I've prepared a pull request to perform the MIME change at https://github.com/nicodel/Run-Bike-Hike/pull/78.  I haven't tested it, but it seems like a good guess that can't hurt!

Comment 4

[Daniel Holbert [:dholbert]]

That was quick -- thanks!  That addresses my specific use-case here.

Presumably we should leave this bug open to track the actual bug in the mail app, though -- yes? (since there is a bug there, and other apps may run into it)

It seems likely that my STR from comment 0 will no longer trigger the bug, once Run,Bike,Hike ships an update with your fix; do you have any suggested alternate STR that would be worth posting here, so that when this bug gets circled back to, someone can still figure out how to trigger it? (Or maybe you've already got enough information in comment 2?)

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

Yes, this is definitely a bug here in the email app.  I'm changing the bug subject to match the underlying problem since hopefully no one will run into the Run,Bike,Hike issue anymore.

We have enough to add back-end tests and fix this in the email app.  The STR on the device would be to download a text/plain somehow and use the v2.2/trunk version of the download manager to trigger a "share" activity.  Otherwise one would need to use custom code to trigger a new/share activity with the specific MIME type since the usual suspects (gallery, videos, camera, music) don't deal in text/* MIME types.

Comment 6

[Nicolas Delebecque (:nicodel)]

patch has been commited.
New version (0.1.11) of Run, Bike, Hike... has been submitted today.

Thank you

Comment 8

[Jose Manuel Cantera]

Nomination rationale:

Version 2.2 is providing a new feature (requested by operators in the target markets) which is the ability to share a contact via e-mail and MMS. See bug 1113605. 

Such a feature is going to be available, through the 'share' activity supported by the email app as well. Actually a text/vcard can be attached to an email message through the 'share' activity but the vcard sent is not received correctly by the recipient. 

Consequently, this bug is blocking a must feature announced for v2.2 and as a result it should be a blocker itself. 

(Aside note)

We are willing to help with fixing this but as it seems to be a email backend issue it could be more difficult for us to deal with it and to set up the proper dev environments to fix it up. In any case please let us know if you need any help.

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

Yes, this definitely wants to be fixed for v2.2, especially given the effort to make email support all attachment types.  I also agree it makes the most sense for the email team to provide the fix since the fix is fairly involved (understanding-wise, actual fix shouldn't be that bad, although tests are always the hardest!)

Comment 10

[Dylan Oliver [:doliver]]

triage: not blocking, but this bug will be part of the attachments fix for bug 825318, a feature which we are targeting for 2.2.

Comment 11

[Noemí Freire (:noemi)]

Hi,

Please notice that this bug blocks a 2.2 blocker (Bug 1113605). Moreover, in case this bug is not fixed, we should work on removing Email option from sharing contacts functionality since it is not properly working.

Comment 12

[Andrew Sutherland [:asuth] (he/him)]

This was fixed by the landing of https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/367 and https://github.com/mozilla-b2g/gaia/pull/28531 on bug 825318 which included a fix and a test.