Error when answer to EHLO comes in multiple packets.

VERIFIED FIXED in mozilla1.4final

Status

MailNews Core
Networking: SMTP
P2
major
VERIFIED FIXED
15 years ago
9 years ago

People

(Reporter: Staszek Salik, Assigned: Christian Eyrich)

Tracking

Trunk
mozilla1.4final

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2.1) Gecko/20021130
Build Identifier: http://ftp.mozilla.org/pub/mozilla/releases/mozilla1.3/mozilla-win32-1.3-installer.exe

Problem sending mail to some SMTP servers.

The SMTP gives long answers to ehlo command. Here is a hand-crafted example
(telnetting to an SMTP port):

ehlo 10.10.10.1
250-router.ost-poland.com.pl Hello [10.10.10.9], pleased to meet you
250-ENHANCEDSTATUSCODES
250-PIPELINING
250-EXPN
250-VERB
250-8BITMIME
250-SIZE
250-DSN
250-ETRN
250-DELIVERBY
250 HELP

This used to work allright with mozilla 1.2.1 I'm using currently, however, it
fails with 1.3 final. I tried diagnosing the problem with a sniffer. I've found
out, that the server response is splitted into two packets, witch 100 bytes
lenght each.

Reproducible: Always

Steps to Reproduce:
1. Set up the SMTP server to produce long (multiline) responses to EHLO
2. Use the server as an outgoing server in MailNews
3. Send an email.

Actual Results:  
Dialog that says the SMTP server is not responding.

Expected Results:  
Accept the long EHLO answer and continue transimitting the message.

Comment 1

15 years ago
A much longer EHLO response was working fine for me when I worked on bug 151279.
My build is hosed at the moment, can't test it right now.

Comment 2

15 years ago
Actually, WFM on trunk build 2003031808. I get the usual relaying denied message
when trying that server which means it has passed the ehlo phase.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → WORKSFORME
(Reporter)

Comment 3

15 years ago
Well, installed Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4a)
Gecko/20030318 (on windows 2k). And the problem is still there. Probably this is
a platform specific bug.
Status: RESOLVED → UNCONFIRMED
Resolution: WORKSFORME → ---
(Reporter)

Comment 4

15 years ago
The same with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4a)
Gecko/20030323. "The message could not be sent because connecting to SMTP server
failed...".

Comment 5

15 years ago
Could you provide a complete sniffer log?
(Reporter)

Comment 6

15 years ago
Created attachment 118283 [details]
Faked SMTP server that reproduces the bug.

This probram reproduces the bug. Start it, connecto to the 2025 port with
MailNews and see what happens. See comments inside serve_client().
(Reporter)

Comment 7

15 years ago
k, now I am able to provide more information. If server replies in one packet -
there is no problem, no matter the size of the EHLO reply. If the server
response comes splitted in two packets - MailNews says server is unable to answer.

Run the test case attached (main.c). The correct behaviour of MailNews using
this faked SMTP server is to hung up waiting for a reply to "MAIL TO:" request.
However, if the EHLO reply is transmitted using two write() invocations (or
splitted into multiple packets any other way), MailNews thinks the resver is not
answering properly.

Proper summary of the problem is: "Error when answer to EHLO comes in multiple
packets."
Summary: Unable to send emails to SMTP servers that give long answers to EHLO. → Error when answer to EHLO comes in multiple packets.

Comment 8

15 years ago
Hi, as far as I can say, the bug is not only limited to the EHLO response.
I've written a short perl-smtp server to verify this. Basically the script does:

print SOCKET "220 my.host.name ready to speak ESMTP";
sleep 5;
print SOCKET " at <some timestring here>",$crlf;

So the break is in the first line from the SMTP-Server and mozilla reports an
error as if the break is in the EHLO response.
The sleep is important because some tcp-stacks might combine serveral write()'s
into one packet if time between is short.

If I comment out the sleep-line, sending the mail works fine.

I suspect the function in mozilla that reads response lines from the SMTP-Server
is buggy. Maybe doesn't wait until the CRLF at the end of the line comes in.

I've tested my script (will attach it) against

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4a) Gecko/20030401 on SuSE 8.0.



Comment 9

15 years ago
Created attachment 119963 [details]
perlscript emulating smtpserver sending 220-greeting in 2 parts
(Assignee)

Comment 10

15 years ago
That's a regression from patch of bug 189672. There was some code added to the
global stream pump to prevent a listener to be called again and again although
he doesn't read the data from the stream and so produce a infinite loop.
The code just checks, if the called routine read at least one byte or not. If
not, it stops the whole thing.

The smtp linereader tests the presented input data for a \n as a indicator of a
complete line. If there's no \n, no bytes will be read from the input. The
routine will be left until new data arrives. And this behaviour of course
collides with the loop check ...


Two suggestions for a solution:
1. We always read the data available in the smtp line reader, regardless of it's
a full smtp line or not, and concatenate the new bytes until we have a full line.

2. We add a flag in which the called listeners (e.g. the smtp code) can inform
the anti-loop-code, that it didn't read the data by intention. So the
anti-loop-code won't punish it for this behaviour.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 11

15 years ago
the "flag" in option #2 would be an explicit call to Suspend on the
nsInputStreamPump.

Comment 12

15 years ago
i'll work on a patch...
Assignee: mscott → darin

Updated

15 years ago
Status: NEW → ASSIGNED
Keywords: nsbeta1
Priority: -- → P2
Target Milestone: --- → mozilla1.4beta
(Assignee)

Updated

15 years ago
OS: Windows 2000 → All
Hardware: PC → All
(Assignee)

Comment 13

15 years ago
Created attachment 122613 [details] [diff] [review]
proposed patch

This is absolutely different from what I wrote in comment #10. But it should
solve our problem with less code.

I just replaced SMTPs own ReadLine() function with the ReadNextLine() used by
POP3, IMAP and NNTP protocol handler. This one doesn't wait if no complete line
available but reads as much as is in the receivebuffer. So we won't be
suspended by the InputStreamPump.

I successfully tested this with Thorstens perlscript and a real server sending
lines splitted across packets (see bug 204060).

Since the ReadNextLine routine is in use for long time by the other protocol
handlers I'd classify it as low risk patch. To this it fixes a regression from
early 1.3a, saves code (as I wrote) and makes it more standard like. Enough
point's for 1.4b?

Comment 14

15 years ago
if you do that, I think you need to fix the cram-md5 code not to subtract two
bytes for the CRLF because the line reading code will eat the CRLF's, right?
Otherwise, I think this is a good idea.
(Assignee)

Comment 15

15 years ago
Yes, I need to remove this subtract - and I already did this in the patch:
-        m_responseText.Length() - 2 /* subtract CRLF */, nsnull);
+        m_responseText.Length(), nsnull);

Comment 16

15 years ago
Comment on attachment 122613 [details] [diff] [review]
proposed patch

sorry, my mistake, I looked for it and I still didn't see it the first time :-(
looks good.
Attachment #122613 - Flags: review+

Comment 17

15 years ago
Comment on attachment 122613 [details] [diff] [review]
proposed patch

whoops, sorry, I don't see us freeing m_lineStreamBuffer, and we should.
Attachment #122613 - Flags: review+ → review-
(Assignee)

Comment 18

15 years ago
Created attachment 122628 [details] [diff] [review]
patch with delete of m_lineStreamBuffer

Ok, this time you're right. Here it is.
Attachment #122613 - Attachment is obsolete: true

Comment 19

15 years ago
Comment on attachment 122628 [details] [diff] [review]
patch with delete of m_lineStreamBuffer

r=bienvenu, but please remove the null check before the delete - it's not
needed since delete checks for null implicitly. Thx for fixing this!
Attachment #122628 - Flags: review+
(Assignee)

Comment 20

15 years ago
Created attachment 122629 [details] [diff] [review]
patch v3

Ok, without check now.
Attachment #122628 - Attachment is obsolete: true

Comment 21

15 years ago
I doubt there's any chance of this making it into 1.4b, correct?

Comment 22

15 years ago
correct, it will go into 1.4 final

Comment 23

15 years ago
-> Christian
Assignee: darin → ch.ey
Status: ASSIGNED → NEW

Comment 24

15 years ago
Comment on attachment 122629 [details] [diff] [review]
patch v3

sr=bienvenu
Attachment #122629 - Flags: superreview+
Comment on attachment 122629 [details] [diff] [review]
patch v3

r/a=sspitzer for 1.4 final
Attachment #122629 - Flags: review+
Attachment #122629 - Flags: approval1.4+
for final
Status: NEW → ASSIGNED
Target Milestone: mozilla1.4beta → mozilla1.4final

Comment 27

15 years ago
fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago15 years ago
Resolution: --- → FIXED
stash@lo7.wroc.pl, can you verify that this is fixed for you?

Comment 29

15 years ago
*** Bug 205009 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 30

15 years ago
Yes, it is working for me. Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.4b) Gecko/20030515 crashes for completely different reasons.

Comment 31

15 years ago
Verifying per comments from stash@lo7.wroc.pl, thanks.
Status: RESOLVED → VERIFIED
(Assignee)

Comment 32

15 years ago
*** Bug 209938 has been marked as a duplicate of this bug. ***
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.