391499 - PR_NetAddrToString doesn't work on MacOS

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P1)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

PR_NetAddrToString doesn't work on MacOS; the following test program shows -1 as the failure from PR_NetAddrToString

The reason is that getnameinfo returns 4 (EAI_FAIL), and that is because the sockaddr structure on MacOS looks different than NSPR's PRNetAddr. However, the code just casts a PRNetAddr to a struct sockaddr* (see URL field)

The sockaddr structure on Mac has a sa_len field as first member (followed by family and the data); PRNetAddr immediately has the family.

--

#include <prnetdb.h>
#include <stdio.h>

int main() {
  PRNetAddr addr;
  char buf[100];
  int rv;

  PR_StringToNetAddr("127.0.0.1", &addr);

  rv = PR_NetAddrToString(&addr, buf, sizeof(buf));
  printf("rv=%x addr=%s\n", rv, buf);
  return 0;
}

Comment 1

[Wan-Teh Chang]

Thanks for the bug report, Christian.  You tracked down this bug last
month in bug 374361 comment 9.  I will try to create a patch next week,
but please feel free to write a patch using the approach I described
in bug 374361 comment 13.

This bug was introduced in the patch for bug 34843.  Kai, Nathan,
Noriko, if you apply that patch to your NSPR 4.6.x builds, you may
want to monitor this bug and apply any patch we produce for this bug.
This bug only affects platforms such as Mac OS X for which the NSPR
feature test macro _PR_HAVE_SOCKADDR_LEN is defined, but it is good
for you to stay as close to the upstream as possible:
http://lxr.mozilla.org/nspr/search?string=_PR_HAVE_SOCKADDR_LEN

(You only need to look at configure.in and pr/include/md/_*.h in
the search results, and ignore the search hit in _linux.h, which
only applies to a GNU/Linux system built with FreeBSD kernel.)

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

I'm not sure that anyone on the NSPR/NSS team has a MAC on which to 
build & test this.  Christian, hopefully you can help test any patches.

Comment 3

[Julien Pierre]

I think Christophe or Glen may have one.

Comment 4

[Wan-Teh Chang]

Attachment: Proposed patch

Please review and test this patch.  It works on my Mac.

I also fixed two unrelated problems.

1. Put the pr_GetAddrInfoByNameFB function inside an ifdef so
that it is only defined when it is needed.  This eliminates a
"defined but not used function" compiler warning.

2. In PR_EnumerateAddrInfo, added a while loop to skip socket
addresses returned by getaddrinfo that are larger than PRNetAddr.
This is a sanity check to protect the memcpy call below.

There is also some code to convert AF_INET6 to/from PR_AF_INET6.
This is because while we guarantee that PR_AF_INET equals AF_INET,
PR_AF_INET6 may not be equal to AF_INET6.  (We define PR_AF_INET6
with the same value as AF_INET6 whenever we can, if the OS has
the AF_INET6 macro.)

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 276122 [details] [diff] [review]
Proposed patch

Looks good.

I also tested this in my Mac and it works.

Comment 6

[Noriko Hosoi]

Comment on attachment 276122 [details] [diff] [review]
Proposed patch

Your diff looks good to me, too.  And I tested the patched nspr with the FDS/Mozilla LDAP client on the supported platforms.  There is no regression.
Thank you, Wan-Teh!

Comment 7

[Wan-Teh Chang]

I checked in the patch on the NSPR trunk (NSPR 4.7).

Checking in prnetdb.c;
/cvsroot/mozilla/nsprpub/pr/src/misc/prnetdb.c,v  <--  prnetdb.c
new revision: 3.54; previous revision: 3.53
done

We still need to create a new NSPR static tag for mozilla/client.mk
so that Mozilla trunk can pick up this bug fix.  Kai, could you please
do that?

Comment 8

[Justin Dolske [:Dolske]]

FTP is currently broken on Mac Trunk, probably due to this. I see a PASV request being made in a sniffer, but no attempt at a data connection. The code's probably trying to convert the PASV reply to an IP address with this function?

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

no, the code converts the socket peer address to a string to later pass it to a connect-like function which takes the address/hostname as a string.

Comment 10

[Kai Engert (:KaiE:)]

(In reply to comment #7)
> 
> We still need to create a new NSPR static tag for mozilla/client.mk
> so that Mozilla trunk can pick up this bug fix.  Kai, could you please
> do that?

Bug 392852