72831 - can't use localhost as an smtp server

Status: RESOLVED FIXED

(MailNews Core :: Networking: SMTP, defect)

Description

[Christopher Blizzard (:blizzard)]

If you set your SMTP server as localhost you get the mysterious message "sending
of message failed."  It apparently doesn't ever actually contact the server, either.

Comment 1

[Christopher Blizzard (:blizzard)]

Setting it to 127.0.0.1 works.

Comment 2

[Christopher Blizzard (:blizzard)]

I would love to try to get this working for 0.8.1 but it's pretty late in the
game.  I'm waiting for my debug build to finish.  Maybe there's a friendly
assertion or something.

Comment 3

[Christopher Blizzard (:blizzard)]

tor pointed out that this might be because nsDNSService::Resolve() has a special
case for localhost.  I'm going to guess that this is probably why things are
failing.

The special case rewrites localhost as the hostname and uses that IP address
instead.  Why?  That's going to fail on my machine because sendmail doesn't
listen to any interfaces except for 127.0.0.1.  If I had wanted to use the
external IP I would have put the name for that interface in my SMTP server
configuration.

Is this here because of windows horkage wrt network configration or something?

Comment 4

[Christopher Blizzard (:blizzard)]

Adding gagan who wrote the bad bad code.

Comment 5

[Christopher Blizzard (:blizzard)]

Apparently this went in when proxy auto config went in.  I don't know how it's
related.

Comment 6

[Christopher Blizzard (:blizzard)]

Attachment: patch

Comment 7

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

sr=shaver.  PAC doesn't even work right now anyway.

Comment 8

[Scott MacGregor]

I don't feel comfortable super reviewing this until the DNS guys have looked at
it. I'm sure they understand why the code may have been in there in the first place.

Please wait for gagan and Gordon to both sign off on this before landing.Thanks.

Comment 9

[Scott MacGregor]

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

Comment 10

[Brendan Eich [:brendan]]

I feel very comfortable asserting that any rewriting of localhost is ipso facto
broken.  Gagan?

/be

Comment 11

[Christopher Blizzard (:blizzard)]

If PAC needs that for some reason the PAC code should be doing it.  I feel
pretty confidant that it shouldn't do that in the DNS service.

Comment 12

[tenthumbs]

The string "localhost" is just that, a string in /etc/hosts. It has no
special meaning. It is not required to refer to the loopback interface,
although that is the conventional usage. I do not believe there is an
RFC or any other standard that specifies the meaning of "localhost." Of
course, the loopback IPV4 address *is* special.

In any event, mozilla should not assume has exclusive rights to, or can
control in any way, any interface; not for any reason, not ever.

Comment 13

[Gagan]

I totally agree with you all. Some historical facts-- 

This went in around the time for PAC purely for the fact that PAC needs the two 
functions Resolve and IsInNet. That explains the correlation. 

As for the localhost problem-- I am totally to be blamed for that. This was 
added to get to "myIPAddress" which correctly should really have been a separate 
interface. So I am ok with the patch the way it is... except that this will 
definitely break PAC with someone using/asking for myIPAddress (different than 
127.0.0.1 loopback) So my suggestion is that we add another function to the 
nsDNSService that returns our IP address. 

Blizzard if you want to do that then go for it... or you could wait until later 
tonight when I might be able to attach the patch for doing the same.

Comment 14

[Gagan]

Attachment: patch that adds a myIPAddress function as well as fix this problem.

Comment 15

[Gagan]

oops... I missed an intended-but-later-than-diffs line of
NS_ENSURE_ARG_POINTER(o_ip) in the new function.

Comment 16

[Christopher Blizzard (:blizzard)]

Are there any callers that require the new call?  If not, sr=blizzard.

Comment 17

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

review comments:
  - make MyIPAddress an attribute, not a method
  - what does ``my'' mean?  What happens for machines with multiple IP addresses
    (like, say, _every_ Linux box in the universe)?  I'd prefer 
    ``localAddress'', but I also want to know what happens in that case.

For 0.8.1, let's just rip out the localhost->local-hostname crap and leave the
rest of this patch out.

Comment 18

[Gagan]

shaver: I like the idea of making myIPAddress an attribute. I'll submit a new
patch soon. As for the case of multiple IP addresses-- AFAIK we use the default
(or primary) hostname to get to the IP address. 
blizzard: the only caller of the "myIPAddress" functionality is in the PAC (bug
53080) and I will make sure I add the corresponding patch there as well. Thanks
for the sr.

Comment 19

[Gagan]

Attachment: patch with shaver's suggestion

Comment 20

[Gagan]

fix is in.

Comment 21

[Christopher Blizzard (:blizzard)]

Checked into 0.8.1.

Comment 22

[esther]

Reporter can you verify this works for you now