Closed Bug 1091295 Opened 10 years ago Closed 10 years ago

[Email] Failure to normalize newlines in composed messages. May cause some SMTP servers (ex: qmail) to refuse to send mail with a 451 error.

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.1+, b2g-v2.0 unaffected, b2g-v2.1 verified, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S9 (21Nov)
blocking-b2g 2.1+
Tracking Status
b2g-v2.0 --- unaffected
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: pcheng, Assigned: asuth)

Details

(Keywords: regression)

Attachments

(3 files)

Attached file logcat of issue on Flame 2.1 (deleted) —
Bug was found while branch checking for bug 1084216. Prerequisite: 1) Have an email account for Yahoo.co.jp 2) Add a Chinese or Japanese keyboard in Settings 3) Have a photo taken by Flame's camera STR: 1) Set up the Yahoo.co.jp email account with a 6-character-or-more Chinese or Japanese name 2) Send an email with a photo attachment prepared at (3) above Expected: Email can be sent Actual: Email cannot be sent. An error message 'Error occurred. Email saved to outbox' is displayed. Repro rate: 5/5 Attaching a logcat. Notes: 1) Issue occurs on Flame 2.1 and 2.2 and does not occur on 2.0 (for 2.0 there is another bug 1084216) 2) Issue does NOT occur on affected branches if a photo is NOT attached (however there's still another bug in this scenario, see bug 1084216 comment 11) 3) Issue does NOT occur on gmail
Attached image screenshot of issue (deleted) —
Occurred on: 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 ----- Not occurred on (instead bug 1084216 occurred): 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 ----- Attaching a screenshot.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: regression
NI to Email owner for blocking call
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell) → needinfo?(edchen)
It would likely help to get a test account configured correctly for use by dev. Please feel free to privately send the info we could use to set up account in the email app to jburke@mozilla.com and I can share it with the other email developers.
I try to use Yahoo! TW mail account, this issue cannot reproduce. Please send JP account to us and than we can figure out what's going on this issue.
Flags: needinfo?(edchen)
(In reply to James Burke [:jrburke] from comment #3) > It would likely help to get a test account configured correctly for use by > dev. Please feel free to privately send the info we could use to set up > account in the email app to jburke@mozilla.com and I can share it with the > other email developers. Test account credentials has been sent to your email.
Further testing shows that unlike bug 1084216 requires, I actually don't need a 6 chinese characters name to repro the bug. I tried with 4 characters and it repros this bug.
Assignee: nobody → bugmail
Status: NEW → ASSIGNED
From the log we're getting a 451 on the SMTP send, but we're failing to extract anything more useful from that. Since we have a regression in actually saving/using the (display) name on the account, this is probably something different.
I ran with a slightly modified smtpclient and the SMTP response line we're getting is: 451 See http://pobox.com/~djb/docs/smtplf.html. This indicates that qmail (which yahoo.co.jp seems to use all over the place) saw an \n when it wanted \r\n. It's also worth noting that email.js is freaking out when doing an IMAP sync of a welcome mail where there's unexpected whitespace in the BODYSTRUCTURE result. Filing a separate bug for that, hopefully it will be fixed by our upgrade bug.
We're failing to normalize the body payload newlines under v2.1+ in certain circumstances; I modified mail-fakeservers' SMTP server to get sad when it sees \n instead or \r\n, and sad it got. For v2.0 and earlier versions this appears to not be a problem because mailcomposer would pass the body payload through mimelib's encodeQuotedPrintable method which would perform newline normalization. The wrinkle is that mailbuild.js which we're using for v2.1+ got smarter. Without us explicitly specifying a transfer-encoding it now checks whether it needs to use quoted-printable or not. When it doesn't need to do so, it just claims 7bit encoding and sends things verbatim, which means our \n's stay \n instead of becoming correct \r\n's. Probably the simplest/safest thing is for us to just run a \n => \r\n transform before issuing calls to setContent on the MimeNode. Unless I hear otherwise, I'll go with that and clean up the mail-fakeserver's mechanism so it causes tests to fail instead of just getting sad.
Summary: [Email] Unable to send an email attachment when sender uses Japanese/Chinese name + yahoo jp email account → [Email] Failure to normalize newlines in composed messages. May cause some SMTP servers (ex: qmail) to refuse to send mail with a 451 error.
Note that this is not the end of the yahoo.co.jp stuff. It's still angry about a BODYSTRUCTURE thing. More fixes coming, plus the test I made to let us ensure that the generated rfc2822 Blob we create looks like what we expect. That will happen on different bugs for sanity purposes, however. One extra thing on this bug is tunneling through the server's error messages as errorData.
Attachment #8519128 - Flags: review?(jrburke)
Comment on attachment 8519128 [details] Normalize newlines, modify smtpd server (in mail-fakeservers) to fail without fix, pass with fix r+, commented in pull request, but it is more of a comment validating the regexp choice.
Attachment #8519128 - Flags: review?(jrburke) → review+
Comment on attachment 8519128 [details] Normalize newlines, modify smtpd server (in mail-fakeservers) to fail without fix, pass with fix [Approval Request Comment] [Bug caused by] (feature/regressing bug #): the email.js changes in bug 885110 (which can't be backed out, so don't be getting any ideas!) [User impact] if declined: People using mail servers that require conformance to internet standards will be unable to send email unless they put something in their message body that is not 7-bit friendly. The qmail SMTP server (which is popular), for one, gets really upset about people violating standards, which is what is causing the yahoo.co.jp problem. [Web impact] if declined: Standards will be violated and we will look dumb. [Testing completed]: Back-end tests have new coverage. Trunk b2g-desktop tests against yahoo.co.jp. [Risk to taking this patch] (and alternatives if risky): No risk. [String changes made]: No l10n string changes made.
Attachment #8519128 - Flags: approval-gaia-v2.1?
BLocking given the regression and QA comments. :edchen/:piwei, can you help verify this issue on trunk/2.1 ?
blocking-b2g: --- → 2.1+
Attachment #8519128 - Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
Flags: needinfo?(pcheng)
Flags: needinfo?(edchen)
(In reply to bhavana bajaj [:bajaj] from comment #14) > :edchen/:piwei, can you help verify this issue on trunk/2.1 ? Issue is verified fixed on trunk. Email can now be sent with attachment & Chinese name. Even the issue in bug bug 1084216 is fixed on trunk. 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 2.1 still needs to be checked after it's been uplifted.
Flags: needinfo?(pcheng)
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #16) > v2.1: > https://github.com/mozilla-b2g/gaia/commit/ > 43139177439333d20df5ca4f240ec66f0f9faf5f The URL doesn't look like a 2.1 merge?
(In reply to Pi Wei Cheng [:piwei] from comment #17) > (In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #16) > > v2.1: > > https://github.com/mozilla-b2g/gaia/commit/ > > 43139177439333d20df5ca4f240ec66f0f9faf5f > > The URL doesn't look like a 2.1 merge? it's good: $ git branch -a --contains 43139177439333d20df5ca4f240ec66f0f9faf5f remotes/origin/v2.1
Flags: needinfo?(edchen)
(In reply to Andrew Sutherland [:asuth] from comment #18) > it's good: > > $ git branch -a --contains 43139177439333d20df5ca4f240ec66f0f9faf5f > remotes/origin/v2.1 Thanks for checking. Issue is verified fixed on 2.1, and the issue for bug 1084216 is not happening on 2.1 either. 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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: