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.

.

Can confirm this on 20030407 on Win and Linux. Mail seems to be sent correctly but it isn't.

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).

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.

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

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!

> > 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.

patch with changes according to comment #10.

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.

patch with change according to comment #13.

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.

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.

Patch committed to trunk.

Patch was checked some time ago and works, so fixed.