This repository has been archived by the owner. It is now read-only.

Socket.remoteAddress is undefined on windows. #1345

Closed
Floby opened this Issue Jul 15, 2011 · 22 comments

Comments

Projects
None yet
6 participants

Floby commented Jul 15, 2011

I tested the following code on windows with the node 0.5.1 executable released yesterday.

var http = require('http');
var util = require('util');

var server = http.createServer(function(req, res) {
    res.writeHead(200);
    res.end('Hello World! \n');
    console.log('got request from %s', req.connection.remoteAddress);
});
server.listen(8080);
console.log('http server listening on port 8080');

var net = require('net');
net.createServer(function(socket) {
    console.log('connection from %s', socket.remoteAddress);
    socket.end();
}).listen(8000);
console.log('tcp server listening on port 8000');

Hitting the server on both http and tcp ends logs connection from undefined or got connection from undefined. Note that I did not try to hit the servers from a distant machine. Everything is from localhost.

Expected behavior : socket.remoteAddress is set to the client remote ip address (in this case 127.0.0.1 or equivalent).

Node Version: 0.5.1
uv: 0.1
Windows XP pro SP3.

I would be glad if an update can be given on this, since it impedes the ability to test node sockets on Windows. I'm on Windows 7 SP1

plexel commented Aug 5, 2011

Confirmed on XP pro sp3 for req.connection.remoteAddress. Gives undefined.

Member

bnoordhuis commented Aug 5, 2011

Does test/simple/test-net-remote-address-port.js work for you guys?

plexel commented Aug 11, 2011

On XP with node.exe the above test just exits right after starting in console and didn't output anything or throw anything. There is no server listening so I'm not sure what to make of the test?

Member

bnoordhuis commented Aug 11, 2011

Right, turns out there's a bug in that test (see f52a8db) so let's disregard that. Maybe I can sneak in a patch for today's release but consider it broken for now.

Member

bnoordhuis commented Aug 11, 2011

79f064f should fix it. If someone has the chance to test it on Windows, that'd be awesome.

plexel commented Aug 11, 2011

Looks like remoteAddress is still undefined. Here's what I got in XP pro sp3:-

C:\Nodejs>node testRemoteAddress.js

assert.js:93
throw new assert.AssertionError({
^
AssertionError: "undefined" == "127.0.0.1"
at Server. (C:\Nodejs\testRemoteAddress.js:10:10)
at Server.emit (events.js:67:17)
at TCP.onconnection (net_uv.js:631:8)

C:\Nodejs>

Member

bnoordhuis commented Aug 11, 2011

@plexel: what commit is your master branch at and do you still have the problem after a make distclean?

plexel commented Aug 12, 2011

@ bnoordhuis: sorry but I have no idea what you are talking about?

Member

bnoordhuis commented Aug 12, 2011

@plexel: I assume you're using the pre-built binaries? I was looking for people who build node from source (and from our repository).

Floby commented Aug 12, 2011

Sorry, I used the prebuilt windows binary when I reported this bug. Windows
is not my primary OS and I don't have any developping toolchain installed.
Guess I'm pretty useless here.

On 12 August 2011 16:03, bnoordhuis <
reply@reply.github.com>wrote:

@plexel: I assume you're using the pre-built binaries? I was looking for
people who build node from source (and from our repository).

Reply to this email directly or view it on GitHub:
joyent#1345 (comment)

Member

bnoordhuis commented Aug 12, 2011

@Floby: No worries. We're releasing WIndows builds on a weekly basis. Just make sure you're running the latest and greatest when reporting issues.

Thanks bnoordhuis. Works perfectly in Node 0.5.4 :).

On Fri, 12 Aug 2011 14:47:20 -0000, bnoordhuis
reply@reply.github.com
wrote:

@Floby: No worries. We're releasing WIndows builds on a weekly basis.
Just make sure you're running the latest and greatest when reporting
issues.

@bnoordhuis bnoordhuis closed this Aug 12, 2011

plexel commented Aug 12, 2011

@ bnoordhuis: Yes node.exe version 0.5.3 I believe.

I now tried your test with node.exe version 0.5.4 and socket.remoteAddress is showing 127.0.0.1

I am running an http and an https node 0.5.4 server on my laptop at localhost:80 and localhost:443 respectively. Both are accessible from the internet. What I want to know is the IP address of callers but that is still not possible. I have to use the following queries for requests at present:-

http server
console.log(request.connection.remoteAddress)
console.log(request.connection.address().address)

https server
console.log(request.connection.socket.remoteAddress)
console.log(request.connection.socket.address().address)

I do not know why .socket must be used with https and not http but that is another issue.

If I do remote internet requests to these two node 0.5.4 servers using http://web-sniffer.net I get this:-

http server
192.168.1.100 (request.connection.remoteAddress)
192.168.1.100 (request.connection.address().address)

https server
192.168.1.100 (request.connection.socket.remoteAddress)
192.168.1.100 (request.connection.socket.address().address)

192.168.1.100 is the IP address of my laptop in my LAN behind the adsl router.

But what I want to know is the IP of web-sniffer.net. Shouldn't that should be the remoteAddress?

Is it right that socket.remoteAddress is the same as socket.address().address?

On 12/08/2011 3:03 pm, bnoordhuis wrote:

@plexel: I assume you're using the pre-built binaries? I was looking for people who build node from source (and from our repository).

@bnoordhuis bnoordhuis reopened this Aug 12, 2011

Member

bnoordhuis commented Aug 12, 2011

Right you are, it's broken with --use-uv. For reference:

function server(peer) {
  console.error(peer.remoteAddress, peer.remotePort)
  peer.write("OK\n", function() { peer.destroy() })
}
require('net').createServer(server).listen(8080)

nc -4vs 127.127.127.127 127.0.0.1 8080 prints:

127.127.127.127 19722 (legacy)
127.0.0.1 8080 (libuv)

libuv passes in the server address, not the remote address.

GottZ commented Aug 16, 2011

i approve this behavior. it prints the servers ip instead of the clients ip. also the port it tells is the server port

plexel commented Aug 16, 2011

You can get the server ip from socket.address().address so why get it also from socket.remoteAddress. remoteAddress should be the client ip address, otherwise where can you get it?

Member

bnoordhuis commented Aug 16, 2011

@plexel: I suspect @GottZ means 'confirm' when he says 'approve'.

plexel commented Aug 16, 2011

@bnoordhuis ah yes, sorry. Is it hard to change the remote address to the client address? How long for a fix?

On 16/08/2011 15:31, bnoordhuis wrote:

@plexel: I suspect @GottZ means 'confirm' when he says 'approve'.

GottZ commented Aug 27, 2011

now in 0.5.5 it still shows the server ip and port instead of the client ip and port.

@ghost ghost assigned bnoordhuis Aug 29, 2011

Member

bnoordhuis commented Aug 29, 2011

Targeted for 0.5.6. Needs getpeername() wrappers for Windows and the Unices.

GottZ commented Aug 30, 2011

thank you for targeting it.

without it login forms would be attackable by brute force.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.