394013 - vfyserv hostname lookup failure

Status: RESOLVED FIXED

(NSS :: Tools, defect, P2)

Description

[Kai Engert (:KaiE:)]

I'm on Linux and I tried the vfyserv utility, both from 3.11.x and pre-3.12

This simple execution produces a failure:

$ vfyserv paypal.com
Error in function PR_GetHostByName: -5973
 - A directory lookup on a network address has failed.

Bob R was able to reproduce this failure.

Using "host paypal.com" works fine.

Running "vfyserv ip-address-of-paypal.com" is not really a good workaround, because it complains about hostname mismatch (of course).

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

May be an NSPR bug on Linux?  Maybe a Linux bug?

Kai, Since you run Linux and I don't, let me ask you to delve into this
with a debugger.  

Bug 366614 will change vfyserv to use an IPv6-capable replacement for
PR_GetHostByName soon.  So, if this is a bug in PR_GetHostByName, we
should identify that (and turn this into an NSPR bug) ASAP, before 
bug 366614 is fixed.

Comment 2

[Robert Relyea]

I had the same failure on my windows box.

Comment 3

[Kai Engert (:KaiE:)]

Sigh. Maybe it should have taken me less time to find out, but after an hour of debugging, the answer is:

gethostbyname_r returns:
       ERANGE          34      /* Math result not representable */

vfyserv uses a 256 bytes buffer when calling PR_GetHostByName. This seems to be too small.

I will attach a patch to increase that buffer and make it work for me...

I have also filed bug 394375 where I propose that NSPR should set a better error code.

Comment 4

[Kai Engert (:KaiE:)]

Attachment: Patch v1

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 279023 [details] [diff] [review]
Patch v1

I'm baffled.  
This program has always worked for me!  Never any DNS lookup error.
I'm not THAT special. :)  So, why does it work for me and fail for you?

r=nelson.  I request one slight change:

>-	prStatus = PR_GetHostByName(hostName, buffer, 256, &hostEntry);
>+	prStatus = PR_GetHostByName(hostName, buffer, 1024, &hostEntry);

Please change that to:

>+	prStatus = PR_GetHostByName(hostName, buffer, sizeof buffer, &hostEntry);

So that we don't have to change this line again if we change the size of buffer.

Comment 6

[Kai Engert (:KaiE:)]

Attachment: Patch v2

Actually there is an even better patch. There is a constant that can be used for the size of the buffer. Actually, there is another occurrence of that in the same source file which makes already use of that constant!!!

So here is a better patch, which uses the constant and the sizeof(buffer).

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 279025 [details] [diff] [review]
Patch v2

This is good.

>-	prStatus = PR_GetHostByName(hostName, buffer, 256, &hostEntry);
>+	prStatus = PR_GetHostByName(hostName, buffer, sizeof(buffer), &hostEntry);

Note that the parentheses around buffer are unnecessary. 
Parentheses are required only if the argument to sizeof is a type name, not
if it is an expression.  But it's OK this way.

Comment 8

[Kai Engert (:KaiE:)]

Comment on attachment 279025 [details] [diff] [review]
Patch v2

I propose to land on 3.11 branch, too, requesting 2nd review.

Comment 9

[Kai Engert (:KaiE:)]

trunk:
/cvsroot/mozilla/security/nss/cmd/vfyserv/vfyserv.c,v  <--  vfyserv.c
new revision: 1.15; previous revision: 1.14

Comment 10

[Robert Relyea]

Comment on attachment 279025 [details] [diff] [review]
Patch v2

r+ rrelyea.

yes, much better then the original code.

Comment 11

[Kai Engert (:KaiE:)]

3.11 branch:
/cvsroot/mozilla/security/nss/cmd/vfyserv/vfyserv.c,v  <--  vfyserv.c
new revision: 1.11.2.4; previous revision: 1.11.2.3

marking fixed

Comment 12

[Julien Pierre]

This wasn't failing on Solaris either, FYI.