237118 - mailserver error or message corruption if text-like attachment has no line breaks

Status: RESOLVED FIXED

(MailNews Core :: Attachments, defect)

Description

[cirdi]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040113
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040113

Send some specific file (ascii text file without <CR> or <LF>, 2100 bytes), as
attachment failed. Mozilla or Netscape error message is:

An error occured while sending mail.
The mail server responded: DATA line too long (Max: 1024)

No problem with Outlook, or Netscape Messenger Express (WEBMAIL)

Reproducible: Always
Steps to Reproduce:
1. Attach the test file (myattach.edi)
2. Fill the subject and to field
3. Send

Actual Results:  
Error Message:

An error occured wile sending mail.
The mail server responded: DATA line too long (Max 1024).

Comment 1

[cirdi]

Attachment: Can't send email with this attached file.

Comment 2

[Christian Franke]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a4) Gecko/20040927]
Confirming bug with Mozilla 1.8a4 on Win98SE.

The bug can be reproduced with any long plain ascii file without newlines.
It can also be reproduced with same files converted to EBCDIC.

Mozilla sends the attachment as follows:

--------------090702050401010305080409--
Content-Type: text/plain; name="somename"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="somename"

UNA:+,? 'UNB+UNOC:3+FR...[ONE LONG LINE WITH 2621 CHARS]... UNZ+1+00040301'
--------------090702050401010305080409--

This may exceed max line length of SMTP servers (See Bug 235319 comment 1).


The bug does not occur if the file contains at least one newline. Then Mozilla
uses "Content-Transfer-Encoding: quoted-printable" and sane line length.

Suggest to change Summary to something like:
Attachments without newlines send as text/plain may exceed max. line length

Comment 3

[Christian Franke]

Workarounds:

1. Save the mail to "Drafts" after attaching the file, then reopen and send.
Then mozilla converts  "Content-Transfer-Encoding: 7bit" to "quoted-printable".
I've seen the opposite (things getting worse after storing into Drafts) in Bug
220772 comment 0.

or:

2. Set "mail.file_attach_binary=true" in profile.

Comment 4

[Christian Franke]

Attachment: Patch for nsMsgAttachmentHandler::AnalyzeDataChunk()

nsMsgAttachmentHandler::AnalyzeDataChunk() does not update m_column_count if
newline is missing at EOF. (Cannot test patch now, sorry)

> The bug does not occur if the file contains at least one newline.
Not true, it does also occur if only the last line is too long and newline is
missing.

Comment 5

[Christian Franke]

Depending on SMTP/POP3/... servers involved, Mail attachment may be destroyed
(by line wrap[sendmail] or truncation) or mail may be lost (error return may
result in an error). Request blocking 1.7.x and 1.8a5.

Comment 6

[Christian Franke]

See also Bug 161806, comment 5 for a related issue in
nsMsgAttachmentHandler::AnalyzeDataChunk()

Comment 7

[Patrick McCormick]

I stumbled onto the "destroyed by line wrap" problem Christian describes in
comment #5.  I was trying to transfer a savefile from some proprietary software.
 It turns out that the savefile is a bunch of ASCII text with no newlines.

Thunderbird turns it into text/plain and apparently the mail gateway I'm sending
to quietly wraps the lines, trashing the file in the process.

A workaround I found (via bug 249049) is to force all files with a given
extension to have a particular content-type:

  HKEY_CLASSES_ROOT\.pro
    Content Type="application/octet-stream"

with this, thunderbird properly base64's the attachment and all is well.

Why does Thunderbird assume that a file without a content-type specified can be
sent as text/plain?  We should be defaulting to application/octet-stream unless
there is good reason to believe that the text/plain encoding will not damage the
payload.

confirming bug.  I saw this with Mozilla Thunderbird 0.8 (Windows/20040913).  I
believe that Christian's workarounds will work, but this bug definitely defeats
the principle of "least surprise."

Comment 8

[Patrick McCormick]

Here's one more workaround: wrap the file in a ZIP or TGZ archive before attaching.

Comment 9

[Mike Cowperthwaite]

Christian Franke, assuming your patch is still good, you should find another 
reviewer; Seth doesn't do much in Bugzilla, despite the fact that his name is 
all over.

I'm curious whether your patch works for the extremely-long files at bug 153557.


(In reply to comment #7)
> A workaround I found (via bug 249049) is to force all files with a given
> extension to have a particular content-type:
> 
>   HKEY_CLASSES_ROOT\.pro
>     Content Type="application/octet-stream"

I would recommend not using application/octet-stream as the MIME type associated 
with *anything*; better to make up a MIME type such as applicaton/x-mytype.  
Also, even tho I posted that comment at 249049, I have not been able to make 
such changes to the registry get consistently used by TB; see bug 244618.

Comment 10

[Christian Franke]

(In reply to comment #9)
> .. assuming your patch is still good, you should find another reviewer
done

> I'm curious whether your patch works for the extremely-long files at bug 153557.

It should work:
If a chunk does not contain any newline, the (member!-)variable m_current_column
is incremented by the chunk length during each call of AnalyzeDataChunk().
With the patch applied, m_max_column is set to m_current_column on exit.
If the file does not contain any newline, m_max_column is set to the file size
on exit of AnalyzeSnarfedFile(),
If m_max_column exceeds 900, PickEncoding() should select (at least) quoted
printable.

Comment 11

[Christian Eyrich]

Comment on attachment 165256 [details] [diff] [review]
Patch for nsMsgAttachmentHandler::AnalyzeDataChunk()

I'd say the patch is the right thing to do.

But since I don't have a compilable source tree and I'm not the module owner or
designated "peer", please ask one of them.

Comment 12

[David :Bienvenu]

Comment on attachment 165256 [details] [diff] [review]
Patch for nsMsgAttachmentHandler::AnalyzeDataChunk()

makes sense, thx. I can check this into the trunk today or tomorrow.

Comment 13

[Christian Franke]

(In reply to comment #12)
> makes sense, thx. I can check this into the trunk today or tomorrow.
Thx. But re-examining the issue one year later, I prepared are more general
patch in bug 269390, comment 5 (attachment 200055 [details] [diff] [review]). Suggest to use this instead.

If not, the following alternative to attachment 165256 [details] [diff] [review] is possibly better:

  if (pPrefBranch) 
     pPrefBranch->GetBoolPref ("mail.file_attach_binary", &forceB64);
   
-  if (!mMainBody && (forceB64 || mime_type_requires_b64_p (m_type)))
+  if (!mMainBody && (forceB64 || mime_type_requires_b64_p (m_type) ||
+    m_current_column != 0))
   {
   /* If the content-type is "image/" or something else known to be binary,

This selects base64 instead of quoted printable on incomplete last line and may
fix also related bugs listed in bug 269390, comment 2.

Comment 14

[Christian Franke]

Comment on attachment 165256 [details] [diff] [review]
Patch for nsMsgAttachmentHandler::AnalyzeDataChunk()

This fix is no longer necessary.
The issue will be fixed by patch from bug 269390 comment 7 (attachment 200327 [details] [diff] [review])
which has r+,sr+.

Comment 15

[Mike Cowperthwaite]

TB 1.6a1-1022: this bug is now fixed on the trunk, thanks to the patch for 
bug 269390.  The long attachment is converted to base64 automatically, but 
persists as text/plain.  When viewed inline, it shows wrapped at the whitespace, 
but when saved, the file is correct, byte-for-byte.