1084216 - [Email] v2.0-specific Issue with folded header field

Status: RESOLVED FIXED

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

Description

[Sharaf]

Account details:

Yahoo, POP3+SMTP

STR:
* Add yahoo account: While adding give long Japanese name as account name (first field.)
* Add another yahoo account
* Open the inbox of first account
* Compose a mail to second account.
* Attach an image which is more than 1.3MB size.
* send the mail
* Open the inbox of second account
* Sync mails.

Expected behavior: New mail is received with same name as we entered while creating account

Actual behavior: The received mail author name is altered. We can see the characters "\ " after every 4 character 

Observations:
* Happens with yahoo pop3 server
* If we sent just text as body issue is not happened
* if we attach and send smaller images issue is not happened.
* Reproduced in v2.0
* In master, it seems the author name is not sent.

Comment 1

[Sharaf]

After doing some analysis the issue happens because yahoo server sents us the below header:

From: "=?UTF-8?Q?=E5=A0=B4=E6=89=80=E7=98=B0=E9=96=93?=\
 =?UTF-8?Q?=E5=B1=B1=E3=81=8B=E8=B0=B7=E7=94=B0?=\
 =?UTF-8?Q?=E3=82=84=E9=96=93=E5=B1=B1=E3=81=8B?=" <[e-mail]>

But when we send plain text mail without attachment we are getting

From: "=?UTF-8?Q?=E5=A0=B4=E6=89=80=E7=98=B0=E9=96=93?=
 =?UTF-8?Q?=E5=B1=B1=E3=81=8B=E8=B0=B7=E7=94=B0?=
 =?UTF-8?Q?=E3=82=84=E9=96=93=E5=B1=B1=E3=81=8B?="
 <[e-mail]>

When we unfold this header line we are taking the last "\" also.
Is this expected?

I checked with Thunderbird, and it seems thunderbird is discarding the last isolated "\"

ni? to asuth, for his comments

Comment 2

[Sharaf]

Two more observations

* Issue happens with long Chinese name also
* Mainly issue happens with yahoo.co.jp servers

Comment 3

[Rachelle Yang [:ryang][[e-mail]]]

Hi Jim, Could you please kindly help with this issue ?
looks like there will be some errors of email components while using email with attachment more than 1.3MB size. 
Could you please help clarify it ? Thank you very much !

Comment 4

[Jim Porter (:squib)]

I don't work on the email client anymore, and have never even glanced at the POP3 code.

Comment 5

[Rachelle Yang [:ryang][[e-mail]]]

Hi Andrew, 

Could you please kindly help with this issue ?
looks like there will be some errors of email components while using email with attachment more than 1.3MB size. 
Could you please help clarify it ? Thank you very much !

Comment 6

[Rachelle Yang [:ryang][[e-mail]]]

Hi Evan ,

Could you please kindly help us find someone to look into the issue ?

Thanks a lot !!

Comment 7

[Evan Tseng [:evanxd]]

Hi Rachelle,

I think we could make sure this issues still exists in master branch, or we might cherry-pick the patch to fix this.

Thanks.

Comment 8

[Mike Lien[:mlien]]

I cannot reproduce this issue on Flame with v2.0 gaia/gecko

Comment 9

[Evan Tseng [:evanxd]]

Hi Mike,
How about current master branch?

Comment 10

[Evan Tseng [:evanxd]]

Thanks.

Comment 11

[Pi Wei Cheng [:piwei] (inactive)]

Issue is reproducible on Flame 2.0 and Flame base image v188.

Observed behavior: An unwanted back slash and a space "\ " are added to the name of the sender after following steps (key repro points at comment 2 was also followed).

Device: Flame 2.0 (shallow flash)
BuildID: 20141029060208
Gaia: 9f5b6f025e528fabfcc068782cb9b492cb51a7f9
Gecko: e652d8489ee8
Version: 32.0 (2.0)
Firmware: v188
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

-----

Unable to follow the STR on Flame 2.1 and Flame 2.2 because it always shows "Error occurred. Email saved to outbox" message when attempting to send an email at step 6.

The email can only be sent without attachment and in this case, sender's name is displayed in actual email address (example: [e-mail]) and NOT the name they had set when setting up the email.

Device: Flame 2.1 (shallow flash)
BuildID: 20141029082611
Gaia: 2099fb0df60548cf7d4afc367f5048622cc29b3e
Gecko: f02f3fbd0bb0
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2 Master (shallow flash)
BuildID: 20141029054810
Gaia: a9a847920b51b79c4ad4ad339f0a005777a6228c
Gecko: c6989e473f97
Version: 36.0a1 (2.2 Master)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Comment 12

[Pi Wei Cheng [:piwei] (inactive)]

Filed bug 1091295 for the issue encountered on comment 11 on Flame 2.1 and 2.2.

Comment 13

[Rachelle Yang [:ryang][[e-mail]]]

Hi Mike, and Pei-Wei, 

based on your testing result, looks like you have different observation  

Pei-Wei concluded this issue is reproducible on v2.0 Flame. May you please kindly share the log , thanks!


Hi Mike, is this issue also reproducible on master ?

Thanks !

Comment 14

[Mike Lien[:mlien]]

(In reply to Rachelle Yang [:ryang][[e-mail]] from comment #13)
> Hi Mike, and Pei-Wei, 
> 
> based on your testing result, looks like you have different observation  
> 
> Pei-Wei concluded this issue is reproducible on v2.0 Flame. May you please
> kindly share the log , thanks!
> 
> 
> Hi Mike, is this issue also reproducible on master ?
> 
> Thanks !

I cannot reproduce it might because I use [e-mail] account
From comment 11, I believe Pi Wei already has the master result

Comment 15

[Rachelle Yang [:ryang][[e-mail]]]

Hi Evan , looks like this issue also reproducible on master. 
Would you please  kindly check it ?
This is an IOT blocker issue.

Comment 16

[Wayne Chang [:wchang]]

Let's open this up.

Comment 17

[Rachelle Yang [:ryang][[e-mail]]]

hi Evan, per comment 11,

this issue happened as well on master. Would you please look into the bug?

Thank you very much!

Comment 18

[Pi Wei Cheng [:piwei] (inactive)]

Attachment: logcat of issue on Flame 2.0

(In reply to Rachelle Yang [:ryang][[e-mail]] from comment #13)
> Pei-Wei concluded this issue is reproducible on v2.0 Flame. May you please
> kindly share the log , thanks!

Sure. Log attached.

Comment 19

[Rachelle Yang [:ryang][[e-mail]]]

[Blocking Requested - why for this release]:
partner IOT blocking issue with high priority.

Comment 20

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

Unsurprisingly, the backslashes are being added by the yahoo.co.jp outbound process.  What we APPEND to the Sent folder (when using IMAP, not the current ISPDB default) our output is as expected.  Besides the backslashes, the headers in the corrupt case seem to suggest that the presence of the large attachment results in a different processing flow on the yahoo.co.jp back-end.  Interestingly, the transform results in the DKIM and DomainKeys headers ending up invalid (which potentially tells us about where the transform is happening) and mails sent to gmail from yahoo.co.jp seem to not end up corrupt suggesting a different flow in that case.  (Or that gmail is destructively transforming things.)

I'm currently looking at the standards-related issues and possible mitigations.  Both Thunderbird and the gmail web UI generate unquoted base64-encoded MIME words.  Interestingly, when doubling up on the name 場所瘰間山か谷田や間山か so it's 場所瘰間山か谷田や間山か場所瘰間山か谷田や間山か, Thunderbird generates 2 base64 mime-words but does not wrap them.  Gmail wraps them.

Thunderbird (1-line!):
From: =?UTF-8?B?5aC05omA55iw6ZaT5bGx44GL6LC355Sw44KE6ZaT5bGx44GL5aC05omA55iw?= =?UTF-8?B?6ZaT5bGx44GL6LC355Sw44KE6ZaT5bGx44GL?= <[e-mail]>

gmail (2-lines, tab whitespace):
From: =?UTF-8?B?5aC05omA55iw6ZaT5bGx44GL6LC355Sw44KE6ZaT5bGx44GL5aC05omA55iw6ZaT5bGx44GL?=
	=?UTF-8?B?6LC355Sw44KE6ZaT5bGx44GL?= <[e-mail]>

Comment 21

[Candice Serran (:cserran)]

not blocking this late in 2.0. Will fix and land on master. Can decide on uplift when fix is in place later this week

Comment 22

[Pi Wei Cheng [:piwei] (inactive)]

This issue has been fixed on 2.2 after the fix for bug 1091295 has landed. Flipping tracking flag to unaffected on 2.2.

Device: Flame 2.2 Master
BuildID: 20141110053355
Gaia: e02facadb0bc3fe32227b52b31ca47822f7f4322
Gecko: c0d559389a5c
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Comment 23

[Pi Wei Cheng [:piwei] (inactive)]

2.1 is fixed by bug 1091295 as well.

Device: Flame 2.1
BuildID: 20141111072405
Gaia: 4c159e75a1568afbbf0c83c1235ec56facfbe87d
Gecko: d1930a36f9c3
Version: 34.0 (2.1)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 24

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

After addressing all the v2.1/v2.2 yahoo.co.jp email.js issues (on a branch), I've found that the issue does not reproduce on trunk using quoted-printable mime-words.

The difference appears to be that in v2.0 the MIME words live within a quoted-string.  On v2.1/v2.2 they do not, but are still quoted-printable encoded.

To summarize, more or less:

== v2.2/trunk sent:

From: =?UTF-8?Q?=E5=A0=B4=E6=89=80=E7=98=B0=E9=96=93?=
 =?UTF-8?Q?=E5=B1=B1=E3=81=8B=E8=B0=B7=E7=94=B0?=
 =?UTF-8?Q?=E3=82=84=E9=96=93=E5=B1=B1=E3=81=8B?= <[e-mail]>

== v2.2/trunk received, a rewrite happens and the line gets unfolded, but no backslashes are introduced:

From: =?UTF-8?Q?=E5=A0=B4=E6=89=80=E7=98=B0=E9=96=93?= =?UTF-8?Q?=E5=B1=B1=E3=81=8B=E8=B0=B7=E7=94=B0?= =?UTF-8?Q?=E3=82=84=E9=96=93=E5=B1=B1=E3=81=8B?= <[e-mail]>

== v2.0 (no fixes) sent:

From: "=?UTF-8?Q?=E5=A0=B4=E6=89=80=E7=98=B0=E9=96=93?=
 =?UTF-8?Q?=E5=B1=B1=E3=81=8B=E8=B0=B7=E7=94=B0?=
 =?UTF-8?Q?=E3=82=84=E9=96=93=E5=B1=B1=E3=81=8B?=" <[e-mail]>

== v2.0 (no fixes) received:

From: "=?UTF-8?Q?=E5=A0=B4=E6=89=80=E7=98=B0=E9=96=93?=\
 =?UTF-8?Q?=E5=B1=B1=E3=81=8B=E8=B0=B7=E7=94=B0?=\
 =?UTF-8?Q?=E3=82=84=E9=96=93=E5=B1=B1=E3=81=8B?=" <[e-mail]>


=== Conclusion:

I can and will provide a v2.0(m) mitigation fix that gets rid of the quotes.

Comment 25

[Rachelle Yang [:ryang][[e-mail]]]

Hello Andrew, per comment 24, Could you please kindly share any update on the 2.0 fix?

Do you think any chance we could get it done by the end of next week? Since this is highly requested and expected by partner. Thank you very much !!

Comment 26

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

Attachment: v2.0-specific fix for address field quoting

Here's the patch.  Extensive discussion of what's in it for the reviewer at the pull request.  Use https://github.com/mozilla-b2g/gaia/pull/26172/files?w=1 to not have to deal with the whitespace snafu.

I've tested it using the yahoo.co.jp account on a v2.0 b2g-desktop build.

This is a v2.0 patch and it should cherry-pick to v2.0m cleanly since the only difference in email between v2.0 and v2.0m consist of a few CSS font-weight differences.

Comment 27

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

Of course it would be good if someone other than me can also test too, to help sanity check.  I'm just requesting :jrburke do a cursory inspection pass to make sure there's no obvious boneheaded errors on my part when applying upstream patches.

Comment 28

[James Burke [:jrburke]]

Comment on attachment 8523641 [details] [review]
v2.0-specific fix for address field quoting

r=me, based on visual inspection. The main issue with the patch is trying to fit in a particular fix from code on master branches for upstream libs that have changed significantly since the state of the v2.0 code.  :asuth is commended for trying to bridge the gap.

Testing should definitely be done, since this code fix is primarily about data manipulation, and heavily dependent on the range of that data input. As it is, I do not have the right sorts of mail accounts and contacts to test the change.

Comment 29

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

Thanks for the review, James!

needinfo-ing involved parties to test the change and do some smoke testing with the change applied before requesting approval.  Please let me know if you need me to spin a zip build that can be side-loaded using the devtools.

Comment 30

[Rachelle Yang [:ryang][[e-mail]]]

Hi Andrew, thank you very much for your work !

Partner could test the patch at their side as well. Thanks again !

Comment 31

[Sharaf]

Hi Andrew and Rachelle,

I checked it, and I am not able to reproduce the issue any more. But I have some observation regarding this patch

* The existing mails are not modified, only the new mails that we sent from a device having this patch only showed properly.

* If I send a mail from flame device issue still happens.

Comment 32

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

(In reply to Sharaf from comment #31)
> * The existing mails are not modified, only the new mails that we sent from
> a device having this patch only showed properly.

Right.  We've only corrected the composition process here.  We have not added tolerance for the resulting error to the parsing process.

The composition process in v2.0 without this fix is in violation of RFC 2047 since an "encoded-word" appears in a "quoted-string" (violating section 5 point 3) so it was particularly important to address.

And although it would be nice to handle the corrupted rewrite produced by yahoo.co.jp, the complexity for v2.0 is too high and too risky since it involves changes to the mailparser, mimelib, and addressparser libs in order to support an illegal "quoted-pair" construct.  (Specifically RFC 2822 specifies that quoted-pair can't be used with CR/LF.  The obsolete grammars technically allow it, but it's very wild, wild west.)

Given the complexity factor, the low incidence of clients that generate the corrupted "from" fields, yahoo.co.jp being the only server that performs the rewrite that we're aware of, and that the rewrite only occurs with sufficiently large attachments, 


> * If I send a mail from flame device issue still happens.

A flame device running what code?

Comment 33

[Pi Wei Cheng [:piwei] (inactive)]

Issue is verified fixed with the patch applied over Flame 2.0. Emails sent from @yahoo.co.jp to @yahoo.co.jp account with long Chinese name + attachment no longer has issue displaying sender's name.

Patch was applied on top of:
Device: Flame
BuildID: 20141117092134
Gaia: 2f9fa0d7ae95efd06a45c0c8577adfd1d9641be0
Gecko: 2bea026d4f86
Version: 32.0 (2.0)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 34

[Pi Wei Cheng [:piwei] (inactive)]

unsetting tracking flag since this hasn't landed.

Comment 35

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

Comment on attachment 8523641 [details] [review]
v2.0-specific fix for address field quoting

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Not a regression, forever broken.  Just not a lot of exposure to CJK and other high-unicode character situations that get quoted-printable encoded, and definitely little exposure to specific server situations like yahoo.co.jp and its sketchy rewriting.
[User impact] if declined: Users with non-ASCII names using a Firefox device to communicate with other Firefox OS users who use the yahoo.co.jp SMTP server (or other qmail servers with some kind of weird rewrite step) may see backslashes in their own name and other names they send.
[Testing completed]:  A bunch:
- I extensively investigated the problem and verified our fix produces the expected output.  Our fix is derived from upstream fixes.  James Burke did a very thorough review of the changes and we had non-trivial discussions.
- I tested on b2g-desktop for v2.0 against yahoo.co.jp with very long names.
- Sharaf tested (probably against yahoo.co.jp)
- :piwei tested on v2.0 flame against yahoo.co.jp
[Risk to taking this patch] (and alternatives if risky): Low risk.  We are taking some changes to the encoding mechanism based on very popular, tested upstream node.js modules that still use the code as-is, which helps increase confidence.  The primary area of concern/uncertainty relates to a complicated regexp; however its failure will simply pass through the text unencoded.  So it's not fatal like explosion, it's fatal like "garbage in garbage out".  And I do mean "garbage in"; the regexp uses character classes in such a way that any failures would be very esoteric.  I think we should land this.
[String changes made]: No surfaced string changes.  Long answer you probably don't care about:  The "group" address construct has a failsafe internal string, but it will categorically not be used since the only way to get a group into the email app is for us to have received it from another client, in which case the group must (by definition/grammar) have a name, etc. etc.

Comment 36

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

Landed on gaia/2.0:
https://github.com/mozilla-b2g/gaia/pull/26172
https://github.com/mozilla-b2g/gaia/commit/f9d6e3d83c3922e9399a6c27f5ce4cdd27bdfd05

From https://wiki.mozilla.org/Release_Management/B2G_Landing#v2.0M I understand that the patch will magically make its way to v2.0m and I don't have to do anything.  I am marking v2.0m as affected for tracking, though.