825501 - Cannot connect to any hosts via a local proxy on http://localhost if the localhost is a dual-stack host

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Masatoshi Kimura [:emk]]

The first broken changeset:
http://hg.mozilla.org/integration/mozilla-inbound/rev/e4f69649d417

If I set the IP address (either "127.0.0.1" or "::1") as a proxy location instead of the DNS name ("localhost"), the problem does not occur.

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: NSPR log (nsHttp:5,nsSocketTransport:5,nsHostResolver:5)

Comment 2

[Masatoshi Kimura [:emk]]

Attachment: NSPR log when setting proxy address to "::1"

Comment 3

[Masatoshi Kimura [:emk]]

Attachment: NSPR log with last good build (changeset 866217416943)

Comment 4

[Masatoshi Kimura [:emk]]

https://hg.mozilla.org/mozilla-central/rev/e4f69649d417#l3.12
>    3.12 +    if ((af != PR_AF_INET) && (flags & RESOLVE_DISABLE_IPV4))
>    3.13 +        af = PR_AF_INET6;
>    3.14 +

This change cannot work because NSPR doesn't support PR_AF_INET6!

https://mxr.mozilla.org/mozilla-central/source/nsprpub/pr/src/misc/prnetdb.c?rev=a032bf27fa05&mark=1983-1983#1978

Comment 5

[Masatoshi Kimura [:emk]]

Attachment: Remove IPv4 records manually because PR_GetAddrInfoByName dislike PR_AF_INET6

Comment 6

[Masatoshi Kimura [:emk]]

https://tbpl.mozilla.org/?tree=Try&rev=5d8ac073a6e9

Comment 7

[Honza Bambas (:mayhemer)]

I cannot test this patch, so I'm not very much willing to review it.

I'm going to back bug 725587 out, since it is too late in the release cycle now.

Comment 8

[Masatoshi Kimura [:emk]]

The test should be possible only on localhost.

Comment 9

[Honza Bambas (:mayhemer)]

(In reply to Masatoshi Kimura [:emk] from comment #8)
> The test should be possible only on localhost.

As I understand you are on Windows and are using a proxy that is capable of accepting connections on ::1, right?  What proxy is that?

Comment 10

[Masatoshi Kimura [:emk]]

A self-made proxy :)
Seriously, nsServerSocket could create a server on IPv6 addresses, no?
https://mxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsServerSocket.cpp?rev=7f5fad93ef78#265

Comment 11

[bhavana bajaj [:bajaj]]

(In reply to Honza Bambas (:mayhemer) from comment #7)
> I cannot test this patch, so I'm not very much willing to review it.
> 
> I'm going to back bug 725587 out, since it is too late in the release cycle
> now.

can you please confirm this was backed out in which case this bug will no longer be tracked as Firefox20 will be unaffected. Thanks!

It appears to me it was backed out https://bugzilla.mozilla.org/show_bug.cgi?id=725587#c20 but will be helpful if you can confirm

Comment 12

[Honza Bambas (:mayhemer)]

(In reply to bhavana bajaj [:bajaj] from comment #11)
> (In reply to Honza Bambas (:mayhemer) from comment #7)
> > I cannot test this patch, so I'm not very much willing to review it.
> > 
> > I'm going to back bug 725587 out, since it is too late in the release cycle
> > now.
> 
> can you please confirm this was backed out in which case this bug will no
> longer be tracked as Firefox20 will be unaffected. Thanks!
> 
> It appears to me it was backed out
> https://bugzilla.mozilla.org/show_bug.cgi?id=725587#c20 but will be helpful
> if you can confirm

Yes, it has been backed out, untracking.

Comment 13

[Honza Bambas (:mayhemer)]

Comment on attachment 696667 [details] [diff] [review]
Remove IPv4 records manually because PR_GetAddrInfoByName dislike PR_AF_INET6

Review of attachment 696667 [details] [diff] [review]:
-----------------------------------------------------------------

r=honzab

Masatoshi, thanks!  

Can you please open a new bug for making this work directly in NSPR?

Comment 14

[Masatoshi Kimura [:emk]]

(In reply to Honza Bambas (:mayhemer) from comment #13)
> Can you please open a new bug for making this work directly in NSPR?

Filed bug 834013. But do we still need the NSPR abstraction here? All supported platforms natively support getaddrinfo() now.

Another idea is using always (PR_)AF_UNSPEC. At present our hostname cache have a cache entry for each address family even if the hostname is the same. We are sending DNS queries to the server twice (one for AF_UNSPEC, another one for either AF_INET6 or AF_INET). I think it's suboptimal.

Comment 15

[Honza Bambas (:mayhemer)]

(In reply to Masatoshi Kimura [:emk] from comment #14)
> (In reply to Honza Bambas (:mayhemer) from comment #13)
> > Can you please open a new bug for making this work directly in NSPR?
> 
> Filed bug 834013. But do we still need the NSPR abstraction here? All
> supported platforms natively support getaddrinfo() now.
> 
> Another idea is using always (PR_)AF_UNSPEC. At present our hostname cache
> have a cache entry for each address family even if the hostname is the same.
> We are sending DNS queries to the server twice (one for AF_UNSPEC, another
> one for either AF_INET6 or AF_INET). I think it's suboptimal.

Yes, worth to confirm this and probably change the logic a bit.  As I understand, you want to do UNSPEC query but let NECKO select families from the list based on demanded flags?

If that is so, it sounds definitely as a good idea.

Anyway, it's a larger project and we would like to land the h-eyeball fix soon.  So let's take this patch now.

Please create one more new bug for your idea on the optimization.  Thanks!

Comment 16

[Masatoshi Kimura [:emk]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d6469d32b385

Comment 17

[Masatoshi Kimura [:emk]]

We should really have a test to prevent future regression.

Comment 18

[Masatoshi Kimura [:emk]]

Filed bug 834490 for a further improvement.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d6469d32b385