242718 - message send sometimes spins

Status: RESOLVED FIXED

(MailNews Core :: MIME, defect)

Description

[David :Bienvenu]

I managed to get myself in a state where I couldn't send a message - it spun
sending the message. I tracked it down to the temp message constructed not
ending with CRLF, so we weren't sending a CRLF.CRLF to the smtp server. 

To get myself in this state, I sent myself a message with some chars > 0x80, and
in my prefs, I specified using quoted printable. It turned out to use base 64
bit encoding for the message body when I replied to the original message, but
didn't terminate the message body with a CRLF. Proposed fix upcoming.

Comment 1

[David :Bienvenu]

Attachment: proposed fix

I really don't know if this is the right thing, but it seems like we should end
with a CRLF...maybe only in the case that it's the message body?

Comment 2

[David :Bienvenu]

Comment on attachment 147753 [details] [diff] [review]
proposed fix

review, or advice welcomed!

Comment 3

[David :Bienvenu]

Attachment: proposed fix

after talking with JF, he suggested this fix as being safer, and it works

Comment 4

[David :Bienvenu]

Comment on attachment 147762 [details] [diff] [review]
proposed fix

r=ducarroz over aim

Comment 5

[David :Bienvenu]

Comment on attachment 147762 [details] [diff] [review]
proposed fix

fairly safe fix - I reproduced this problem in 1.7...

Comment 6

[Mike Kaply [:mkaply]]

Comment on attachment 147762 [details] [diff] [review]
proposed fix

a=mkaply

Comment 7

[David :Bienvenu]

fixed on trunk.

Comment 8

[Robert Accettura [:raccettura]]

Is this the same problem that caused problems with MAPI send?

Comment 9

[David :Bienvenu]

same underlying failure, but two separate problems. The contract with the stmp
service is that the message sent ends with a CRLF, and it's up to the caller to
make sure that's true. It would be nice to fix it in the smtp code too but it's
not trivial.

Comment 10

[[e-mail]]

Comment on attachment 147762 [details] [diff] [review]
proposed fix

>@@ -762,6 +763,7 @@
>   {
>     status = MIME_EncoderDestroy(m_encoder_data, PR_FALSE);
>     m_encoder_data = nsnull;
>+    needToWriteCRLFAfterEncodedBody = !m_parent;
>     if (status < 0)
>       goto FAIL;
>   }
Won't (status < 0) fail on 64-bit machines?

Comment 11

[Christian Eyrich]

David, I just stumpled over this bug.
Would you please have a look at bug 237828? It looks to me as this is the same
problem and the now patched function is one of the guessed in comment #29.

Comment 12

[David :Bienvenu]

*** Bug 237828 has been marked as a duplicate of this bug. ***

Comment 13

[David :Bienvenu]

Good catch, that function should be an int function, and not an nsresult - or
should be switched to return nsresults. This patch doesn't change anything about
that, though...