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

RESOLVED FIXED in mozilla21

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: emk, Assigned: emk)

Tracking

(Depends on 1 bug, {regression})

unspecified
mozilla21
x86_64
Windows 8
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox20 affected)

Details

Attachments

(4 attachments)

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.
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
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Attachment #696667 - Flags: review?(honzab.moz)
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.
The test should be possible only on localhost.
(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?
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
(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
(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 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?
Attachment #696667 - Flags: review?(honzab.moz) → review+
Depends on: 834013
(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.
(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!
We should really have a test to prevent future regression.
Flags: in-testsuite?
Filed bug 834490 for a further improvement.
https://hg.mozilla.org/mozilla-central/rev/d6469d32b385
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.