Closed Bug 200647 Opened 22 years ago Closed 22 years ago

false/no reaction to receive of SMTP code 421 after sending EHLO/EHLO and loss of connection too

Categories

(MailNews Core :: Networking: SMTP, defect)

defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: info, Assigned: ch.ey)

Details

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3) Gecko/20030312
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3) Gecko/20030312

Hello !

The situation:

In Germany there is a big provider called T-Online. If you are a customer at
T-online you automatically get an email address namexyz@t-online.de
You are only able to read the mails of this mailaccount via pop3 if you have
dialed in using T-Online as provider. 
If you send a mail using their normal SMTP-server, the from: adress of your
mailclient-config is overridden/overwritten with namexyz@t-online.de.
In order enable T-online customers to choose their From: value T-Online has
introduced (years ago) a special SMTP-server (smtprelay) which (again) can only
be reached if you dialed in via T-Online. In both cases there is NO SMTP-Auth
needed inside the mailclient because they seem to be able to ty your identity to
your phone-number.

Now whats the problem? 

Beginning with Apr 01 2003 T-Online only allows registered users to use the
smtp-relay-server in order to send out mails. You still do not need a
smtp-authentication ... it seems like they still use the phone-number/or
dialup-username of your connection in order to tell if you're allowed to use the
special smtp-server or not.

Now to the point:

In Netscape 4.79 my girlfriend got an errormessage popup when trying to send out
mails. The message was like "You are not allowed to use smtprelay.t-online.de
etc". It looked like a customized error message regarding the text. 

MozillaMail 1.3final does not show any error ... the mails seem to get delivered
without an error and are copied to the Sent folder. But they never arrived.

Perhaps the t-online smtprelay-server sends back a special errormessage/code
which MozillaMail is can't neither fetch nor pop up ... while Netscape 4.79 can.

This seems to be a very special problem, but as T-Online is germanys leading
Provider this problem could strike many german t-online users who don't have a
relaying host on their own.

Reproducible: Always

Steps to Reproduce:
not very easy
1. cancel smtprelay-payment subscription at t-online
2. wait some time until subscription is cancelled
3. try to send out mails with outgoing server smtprelay.t-online.de


Actual Results:  
MozillaMail 1.3final: no errormessage ... user thinks his mails get delivered
NetscapeMessenger 4.79: "your are not registered for use of smtprelay.t-online.de"  

Expected Results:  
pop up error message that smtp-server did not grant access/use


this problem is not a config one ... in my opinion the t-online way of telling
if a user is allowed to use the smtp-server is quite unusual and non-standard
because it is not done by smtp-auth ... it is not smtpafterpop either because my
MozillaMailAccount checks the t-online mail at startup ... and remember: they
only allow those customers who pay ... so smtpafterpop would allow all users to
use the smtprelay.
.
Assignee: sspitzer → mscott
Component: Mail Window Front End → Networking: SMTP
QA Contact: esther → nbaca
Can confirm this on 20030407 on Win and Linux.
Mail seems to be sent correctly but it isn't.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 2000 → All
Hardware: PC → All
In answer to EHLO request the T-Online server responds with "421 You are not
registered for using smtprelay.t-online.de (look at
http://www.t-online.de/service/).\r\n".

In nsSmtpProtocol::SendEhloResponse Mailnews only tests m_responseCode for !=
250 and if this is true, continues with sending HELO message.

I think "normal" servers don't respond with codes != 250 here, unless they don't
support the service extensions. But since T-Online can already determine if a
client is authorized at this stage they do although they understand EHLO.

I'm not sure why Mailnews thinks the mail has been send properly and does the
copy operations after trying HELO then. Shouldn't this only follow if at least
the send operation has been done?

Reading through RFC 2821 it looks to me as if EHLO isn't supported by the
server, no other response codes than 500, 501, 502, 504 or 550 are allowed.

So we should either only try HELO if we get one of these codes and handle
everything other than these and 250 as failure. Or at least handle 421 especially.

Remark: Handling 421 also applies to HELO! So I don't completely agree with the
comment in nsSmtpProtocol.cpp#566.
Raising severity because of potential loss of data (not generate a local copy
but Bcc to oneself in prefs -> nothing sent, no copy to oneself).
Severity: normal → critical
Summary: no message if smtp user unregistered (special german case: smtprelay without smtp-auth and smtpafterpop) → false reaction to receive of SMTP code 421 after sending EHLO/EHLO (smtprelay without smtp-auth and smtpafterpop)
Ok, this problem is a little bit harder than I thought. It consists of two parts.
Part one is that we should notice the error messages and display them to the user.
Part two is that we should notice when connection gets dropped at any time.

There is a "fix" for bug 158059 which fixed that we don't notice if the server
just hangs up at the beginning. But it ignored that this was only a part of a
bigger problem, although Patrick called the attention to this in his comment 22.

Maybe it could help forcing Mailnews to disconnect itself after receiving the
421 and setting the status to failure. So nsSmtpProtocol::OnStopRequest could
react upon it. But that's not really nice.

Remark: Still have a look at bug 188569.
Attached patch proposed patch (obsolete) — Splinter Review
Ok, so here's a patch for both problems.
I decided to handle all errors between 500 and 550 in
nsSmtpProtocol::SendEhloResponse as "don't know what EHLO means" from the
server (so try HELO). Every other (especially the 421 here) is a hard failure
and will stop sending the mail.

We can talk about the range of the codes which lead to trying HELO.


The second problem is handled in nsSmtpProtocol::OnStopRequest. m_nextState
will be set to SMTP_FREE in nsSmtpProtocol::ProcessProtocolState in exactly two
situations: 1. sending finished with no error (reached SMTP_DONE) 2. and
failure case which has already been handled by code and UrlState was set to
error (reached SMTP_ERROR_DONE).
In every other case it's anything else when entering OnStopRequest(). So I
decided to take m_nextState != SMTP_FREE as indicator for trouble caused by
connection closed by the other side.

The first if (which comes from fix of bug #158059) in this function is a
special case of this bug (as commented). We don't need it as this case could
also be handled by the new if. But since it gives a more specific failure code
I kept it.
If anybody doesn't think that's good/necessary it can be deleted for my part.


Ah, a last one. As I wrote, handling 421 (and maybe others) also applies to
HELO! So I don't completely agree with the comment in nsSmtpProtocol.cpp#576.
So we might copy some of the error handling code from SendEhloResponse().
-> to me
Assignee: mscott → ch.ey
Summary: false reaction to receive of SMTP code 421 after sending EHLO/EHLO (smtprelay without smtp-auth and smtpafterpop) → false/no reaction to receive of SMTP code 421 after sending EHLO/EHLO and loss of connection too
Attachment #120407 - Flags: review?(dmose)
just another aspect of the "moves to 'sent' folder although not delivered"-problem:


Yesterday I tried to send with Mozilla 1.3final using the email server of my
employer and my laptop. The mailserver returned a "450 system error" ... but the
mail was moved to the "sent"-folder, too. 
In the laptop I have Win2000 + Norton Antivirus installed ... the error message
in this case said 
that Norton Antivirus was unable to deliver the email (450 system error). I
deinstalled Norton Antivirus in order to get it out of the mail delivery process
and then I tried again to send a mail ... unfortunately the mailserverproblem
was fixed after my reboot, so I could not check the behaviour without Norton
Antivirus.

Perhaps Norton Antivirus brings in some problems too ... perhaps NAV takes the
mail from Mozilla and Mozilla thinks (or is made think) that this handover means
"Mail was delivered", even if then NAV fails to finally let the mail out after
the scan.
Hm, code 450 should actually only be send in answer to RCPT command. Mozilla
handles this case the right way (sourcecode looks good and i tested this) and
keeps the message.

If 450 has been received by Mozilla at another stage it might have caused the
move to sent.
But nevertheless, my patch should help in these situation too.

But if you can reproduce your problem, mail me a good description.
Comment on attachment 120407 [details] [diff] [review]
proposed patch

In general, this looks great.  A few small things:

* Please wrap any lines you add or change around column 78; it makes things
easier to read.

* So I would vote to get rid of the special case if (ie the old,
partial fix) in OnStopRequest.	For one thing, it uses
NS_ERROR_CONNECTION_REFUSED, and that's not actually what's going on.
You could make the remaining log statement more general by including
m_totalAmountRead in the log message.

* One could argue that 554 should also cause us to try and fall back
to HELO, though I don't know if this is actually necessary for any
real-life servers.  And 550 itself seems like it should be a hard
error and not attempt fallback.

+	     nsresult rv = nsExplainErrorDetails(m_runningURL,
NS_ERROR_SMTP_SERVER_ERROR, m_responseText.get());
+	     // I'd use NS_ERROR_COULD_NOT_LOGIN_TO_SMTP_SERVER,
+	     // but this doesn't display the servers message
+	     NS_ASSERTION(NS_SUCCEEDED(rv), "failed to explain SMTP error");

Can you conditionalize the declaration and assignment of rv using
#ifdef DEBUG? Otherwise, this will probably generate an "unused
variable" warning when compiled without DEBUG.

Thanks!
Attachment #120407 - Flags: review?(dmose) → review-
> > One could argue that 554 should also cause us to try and fall back
> > to HELO, though I don't know if this is actually necessary for any
> > real-life servers.

> In RFC 2821 I read of sending 554 only in connection establishment instead of
> 220 and in answer to DATA.

Yeah, you're right.  Forget 554.

> > And 550 itself seems like it should be a hard error and not attempt
> > fallback.
>
> Hm, chapter 7.7 of RFC 2821 reads like this, yes. An other source I read
> reffered to 550 in response to EHLO as "Not implemented", that was the
> reason I included it.
>
> So is if(m_responseCode >= 500 && m_ responseCode < 550) ok?

Sure, that sounds fine.

> [re: conditionalizing debug stuff]
>
> Er, besides from that this two lines appear five times in the source file 
> without #ifdef, I don't know how I'd conditionalize the assignment.

Well, I'd like to avoid making the problem worse.  :-)

> Or do you think of something like this:
> #ifdef DEBUG
>            nsresult rv =
> #endif
>            nsExplainErrorDetails(m_runningURL,
>                          NS_ERROR_SMTP_SERVER_ERROR, m_responseText.get());
> #ifdef DEBUG
>            NS_ASSERTION(NS_SUCCEEDED(rv), "failed to explain SMTP error");
> #endif

Yes, except that NS_ASSERTION is already DEBUG-only, so it doesn't need the
#ifdef around it.
Attached patch cleaned patch (obsolete) — Splinter Review
patch with changes according to comment #10.
Attachment #120407 - Attachment is obsolete: true
Attachment #121186 - Flags: review?(dmose)
Comment on attachment 121186 [details] [diff] [review]
cleaned patch 

How about changing the error message to "SMTP connection dropped after %ld
total bytes read"?

Make that change, and you've got r=dmose.
Attachment #121186 - Flags: review?(dmose) → review+
patch with change according to comment #13.
Attachment #121186 - Attachment is obsolete: true
Attachment #121220 - Flags: review?(dmose)
Comment on attachment 121220 [details] [diff] [review]
better-log-message patch

sr=bienvenu - I think dmose gave you an implicit r= if you made the change to
the log error message, so this is probably ready to check in.
Attachment #121220 - Flags: superreview+
Comment on attachment 121220 [details] [diff] [review]
better-log-message patch

Since I already marked it +, it would have also been ok for you to just carry
the review forward.

Anyway, r=dmose.
Attachment #121220 - Flags: review?(dmose) → review+
Patch committed to trunk.
Patch was checked some time ago and works, so fixed.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: