Socket.getAddrInfo does not work properly on Windows 7

[GoogleCodeExporter]

On Windows 7, attempting to use the http client with a hostname fails.
This is because Socket.getAddrInfo doesn't work unless Socket.PF_INET is 
explicitly stated. The address lookup in the http module allows the address 
family to default and this causes getAddrInfo to return nonsense.

A workaround is to explicitly state the address family in http.js at line 260.

A simple test case using shell.js follows:

> var Socket = require("socket").Socket;
> system.stdout(Socket.getAddrInfo("localhost"));
´┐¢´┐¢-☺>
> system.stdout(Socket.getAddrInfo("localhost", Socket.PF_INET));
127.0.0.1>

Original issue reported on code.google.com by andy.bis...@gmail.com on 11 Nov 2011 at 9:57

[GoogleCodeExporter]

What behavior do you suggest?

1) current - no second argument = undefined "hints" structure passed to 
getaddrinfo() = undefined behavior (consistent with native getaddrinfo)

2) default PF_INET when no second argument specified

3) throw when no second argument specified

4) something different?

Original comment by ondrej.zara on 11 Nov 2011 at 1:49

• Changed state: Accepted

[GoogleCodeExporter]

I would suggest two fixes:
Option 2 provides a sensible default and fixes a silent error.
Also, http.js really ought to work on IPV4 and IPV6 so it needs
another configuration parameter or, better, it should try IPV6 then
IPV4 to resolve the name.

Oops! I had in mind something like:

    try {
        ip = Socket.getAddrInfo(host, Socket.PF_INET6);
    } catch(ex){
        ip = Socket.getAddrInfo(host, Socket.PF_INET);
    }

but this doesn't work either because Socket.getAddrInfo(host,
Socket.PF_INET6); silently returns garbage if it fails to resolve.
Returning an empty string would actually be a better way to indicate
failure to resolve as this would also avoid the expense of an
exception
(and failure to resolve is not really exceptional behaviour.)

I am not an expert on network protocols but this *seems* sensible to
me.  If a functioning IPV6 route exists then I guess you would want to
use that preferentially.

Andy

Original comment by andy.bis...@gmail.com on 11 Nov 2011 at 3:02

[GoogleCodeExporter]

This makes sense. To sum this up:

a) the failure to resolve should be indicated by a "null" as a returned value;

b) when no second argument is specified, the call does basically

getAddrInfo(PF_INET6) || getAddrInfo(PF_INET),

returning either IPv6 address (preferrably) or IPv4 address.

Original comment by ondrej.zara on 14 Nov 2011 at 7:01

[GoogleCodeExporter]

That sounds good.
If the check for IPv6 then IPv4 goes in getAddrInfo then I am not so
opposed to throwing an exception
for a failure to resolve because this probably *is* an error.
I had envisaged the check going into http.js which would have entailed
using exceptions for flow control as shown in my previous example.
Putting the check in socket.cc is a much better idea and avoids this
issue.

Also, I have found the bug that prevents IPv6 name resolution working:
line 139 of socket.cc should read

  WSAAddressToString(addr, sizeof(struct sockaddr_in6), NULL, buf, &len);

not

  WSAAddressToString(addr, sizeof(struct sockaddr), NULL, buf, &len);

Andy

Original comment by andy.bis...@gmail.com on 14 Nov 2011 at 11:07

[GoogleCodeExporter]

Just realized that "automagic" default in #2 cannot actually work: an address 
resolved via automatic ipv6/ipv4 detection does not provide info about what 
protocol family was used; a user does not know what PF_ constant should be 
passed to Socket constructor.

This means that solution #2 implies chosing one fixed default value (ipv4?) 
when undefined.

Original comment by ondrej.zara on 14 Nov 2011 at 7:13

[GoogleCodeExporter]

Changing line 139 as suggested does not work, either :/ IPv6 resolution on 
Windows apparently needs some attention :)

Original comment by ondrej.zara on 15 Nov 2011 at 10:55

[GoogleCodeExporter]

Presently I am using the following code in http.js

    var protocol;
    try {
        ip = Socket.getAddrInfo(host, Socket.PF_INET6).split('%', 1)[0];
        protocol = Socket.PF_INET6;
        system.trace('IPV6 -> ' + ip + '\r\n');
    } catch(ex){
        ip = Socket.getAddrInfo(host, Socket.PF_INET);
        system.trace('IPV4 -> ' + ip + '\r\n');
        protocol = Socket.PF_INET;
    }

I can then use the protocol to open the socket. (the system.trace
calls are just something I have added to write Windows trace messages)

Unfortunately, even though this resolves correctly I don't appear  to
be able to connect to anything but localhost.
Not sure why this is but it has been suggested that you may have to
explicitly enable external ethernet interfaces.

Original comment by andy.bis...@gmail.com on 17 Nov 2011 at 9:58

[GoogleCodeExporter]

Correction - It would seem that the failure to work with a remote host
is happening in our node.js server and not the V8cgi client.

Original comment by andy.bis...@gmail.com on 17 Nov 2011 at 10:29

[GoogleCodeExporter]

Would you please mind testing the attached socket.dll? Resolving works fine for 
me on WinXP with this updated version.

Goals:
- working PF_INET
- working PF_INET6
- throwing when unable to resolve using given protocol family
- PF_INET as a default value when none specified

Original comment by ondrej.zara on 24 Nov 2011 at 7:48

Attachments:

• socket.dll

[GoogleCodeExporter]

I have downloaded the new library and I will let you know how it goes.

Thanks,
Andy

Original comment by andy.bis...@gmail.com on 25 Nov 2011 at 10:28

[GoogleCodeExporter]

As I understand it my existing changes to http.js should still work
but they don't always

My changes to the send function:

ClientRequest.prototype.send = function(follow) {
    var Socket = require("socket").Socket;
    var Buffer = require("binary").Buffer;
    var items = this._splitUrl();
    var host = items[0];
    var port = items[1];
    var url = items[2];
    var ip = '';
    var protocol;
    try {
        ip = Socket.getAddrInfo(host, Socket.PF_INET6).split('%', 1)[0];
        protocol = Socket.PF_INET6;
        system.trace(host + ":" + 'IPV6 -> ' + ip + '\r\n');
    } catch(ex){
        ip = Socket.getAddrInfo(host, Socket.PF_INET);
        system.trace(host + ":" + 'IPV4 -> ' + ip + '\r\n');
        protocol = Socket.PF_INET;
    }
    this.header({"Host":host});

trace output
[1488] localhost:IPV6 -> ::1 

error that occurs:

Error: No such file or directory
    at [object Object].send (C:\Users\andrewb\Documents\Prog\APIServer\v8cgi\lib\http.js:318:4)

317     var s = new Socket(protocol, Socket.SOCK_STREAM, Socket.IPPROTO_TCP);
318     s.connect(ip, port);
319     s.send(data);

This works correctly if I comment out the IPv6 stuff:

//  try {
//      ip = Socket.getAddrInfo(host, Socket.PF_INET6).split('%', 1)[0];
//      protocol = Socket.PF_INET6;
//      system.trace(host + ":" + 'IPV6 -> ' + ip + '\r\n');
//  } catch(ex){
        ip = Socket.getAddrInfo(host, Socket.PF_INET);
        system.trace(host + ":" + 'IPV4 -> ' + ip + '\r\n');
        protocol = Socket.PF_INET;
//  }

So the address resolution seems fine but the socket seems to choke on the IPv6 
address

Andy

Original comment by andy.bis...@gmail.com on 25 Nov 2011 at 11:08

[GoogleCodeExporter]

First of all, your modification of http.js seems correct and should not be 
changed.

Are you experiencing this behavior with the new testing socket.dll? Which 
version of Windows? My own experiments show different and weird results :-(

On Windows 7 with new socket library:

- ipv4 resolving and usage WORKS;
- ipv6 resolving and usage (both listening and sending) WORKS ONLY on localhost 
(::1);
- ipv6 resolving DOES NOT WORK on remote IP (www.root.cz as a reference host).

My machine has a working IPv6 stack, but the ISP does not support IPv6. Using 
"nslookup" returns ipv6 addr, though.

I have also acces to Windows XP on a full ipv6 network; will try some further 
experiments there. In the meantime, can you please try a simple listening IPv6 
server: 

var S = require("socket").Socket;
var s = new S(S.PF_INET6, S.SOCK_STREAM, S.IPPROTO_TCP);
s.bind("::1", 8888);
s.listen();
var s2 = s.accept();
s2.send("bai");

and a corresponding client:

var s = new S(S.PF_INET6, S.SOCK_STREAM, S.IPPROTO_TCP);
s.connect("::1", 8888);
var x = s.receive(1024);
system.stdout(x);

Original comment by ondrej.zara on 25 Nov 2011 at 8:25

[GoogleCodeExporter]

This works perfectly on Windows 7.

Original comment by andy.bis...@gmail.com on 30 Nov 2011 at 9:35

[GoogleCodeExporter]

Okay. I will have another testing version of socket.dll available soon - also 
with proper error messages (the description strings were totally irrelevant, 
such as "No such file or directory" when calling Socket::connect).

Original comment by ondrej.zara on 30 Nov 2011 at 9:58

[GoogleCodeExporter]

Another dll for testing. I strongly believe that all windows-network-related 
issues are fixed now.

Original comment by ondrej.zara on 1 Dec 2011 at 12:27

Attachments:

• socket.dll

[GoogleCodeExporter]

Thanks.  Not sure I'm going to get a chance to test it today but I will if I
can and I certainly will early next week.

Andy

Original comment by andy.bis...@gmail.com on 2 Dec 2011 at 8:00

[GoogleCodeExporter]

This now works with the code in comment 11, locally and on other machines on 
our network.

Thanks,
Andy

Original comment by andy.bis...@gmail.com on 6 Dec 2011 at 12:21

[GoogleCodeExporter]

Probably solved in revision 944 :)

Original comment by ondrej.zara on 7 Dec 2011 at 7:20

• Changed state: Fixed