231786 - Extend PR_GetAddrInfoByName() to support PR_AF_INET

Status: RESOLVED FIXED

(NSPR :: NSPR, enhancement)

Description

[John G. Myers]

Comment 1

[John G. Myers]

Attachment: Proposed fix

Comment 2

[Lorenzo Colitti]

Comment on attachment 139641 [details] [diff] [review]
Proposed fix

>     /* restrict input to supported values */
>-    if (af != PR_AF_UNSPEC || flags != PR_AI_ADDRCONFIG) {
>+    if ((af != PR_AF_INET && af != PR_AF_UNSPEC) || flags != PR_AI_ADDRCONFIG) {
>         PR_SetError(PR_INVALID_ARGUMENT_ERROR, 0);
[...]
>-        hints.ai_family = AF_UNSPEC;
>+        hints.ai_family = (af == PR_AF_INET) ? AF_INET : AF_UNSPEC;

Since you're already doing checking whether af is INET or UNSPEC, why why not
just do:
    hints.ai_family = af;
instead of
    hints.ai_family = (af == PR_AF_INET) ? AF_INET : AF_UNSPEC;

Comment 3

[Wan-Teh Chang]

Comment on attachment 139641 [details] [diff] [review]
Proposed fix

r=wtc.

Lorenzo, the reason for
    hints.ai_family = (af == PR_AF_INET) ? AF_INET : AF_UNSPEC;
is that PR_AF_XXX is not documented to have the same value as
AF_XXX.  We have tried to define PR_AF_XXX to have the same
value as AF_XXX where possible, but it is tricky to do that for
AF_INET6.  John's code is defensive programming that does not
depend on undocumented properties.

I am wondering if the motivation for this patch (bug 68796) is
caused by our not passing AI_ADDRCONFIG flag to getaddrinfo on
Mac OS X 10.3 (Panther).  I don't think any of the people still
affected by bug 68796 are on an IPv6 network; I believe they
just have IPv6 stack enabled in their kernel.  So we may want
to ask those Mac OS X 10.3 users to test an NSPR patch that
passes AI_ADDRCONFIG to getaddrinfo.

Comment 4

[Lorenzo Colitti]

Ok. John, sorry for butting in. :)

wtc, a couple of thoughts on your suggestion:

1. I was looking at the FreeBSD CVS repository recently, looking for the
getaddrinfo() implementation, and I vaguely seem to remember CVS log comments
the general gist of which was that AI_ADDRCONFIG was ambiguous and was not
implemented. So I'm not sure that AI_ADDRCONFIG would work (since OS X is
derived from FreeBSD code).

I may remember incorrectly and/or have misunderstood, but if someone wants to do
this, it may be a good idea to look into this before writing a patch.

2. bug 68796 affects all platforms, including platforms which don't support
AI_ADDRCONFIG. And of course using AI_ADDRCONFIG won't help if IPv6 is turned on.

So in the interests of IPv6 adoption the better solution would be the workaround
proposed by John, since IPv6 users would still suffer from doubleclick.net (and
probably more sites like it) being horribly slow even with AI_ADDRCONFIG. For
example, I use IPv6 on Windows and I survive only because I have put
*.doubleclick.net in the hosts file... :)

Comment 5

[John G. Myers]

I run MacOS X 10.3.2, do not have an IPv6 address configured, and do not see bug
68796.

Comment 6

[Wan-Teh Chang]

Patch checked in on the NSPR trunk (NSPR 4.5.1) and
NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.7a).