Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Socket.remoteAddress is undefined on windows. #1345

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

Socket.remoteAddress is undefined on windows. #1345

Floby opened this issue Jul 15, 2011 · 22 comments

Comments

@Floby
Copy link

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.

@fadugyamfi
Copy link

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
Copy link

plexel commented Aug 5, 2011

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

@bnoordhuis
Copy link
Member

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

@plexel
Copy link

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?

@bnoordhuis
Copy link
Member

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.

@bnoordhuis
Copy link
Member

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

@plexel
Copy link

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>

@bnoordhuis
Copy link
Member

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

@plexel
Copy link

plexel commented Aug 12, 2011

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

@bnoordhuis
Copy link
Member

@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
Copy link
Author

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 <
[email protected]>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:
#1345 (comment)

@bnoordhuis
Copy link
Member

@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.

@fadugyamfi
Copy link

Thanks bnoordhuis. Works perfectly in Node 0.5.4 :).

On Fri, 12 Aug 2011 14:47:20 -0000, bnoordhuis
[email protected]
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.

@plexel
Copy link

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
@bnoordhuis
Copy link
Member

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
Copy link

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
Copy link

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?

@bnoordhuis
Copy link
Member

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

@plexel
Copy link

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
Copy link

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
@bnoordhuis
Copy link
Member

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

@GottZ
Copy link

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.
Projects
None yet
Development

No branches or pull requests

6 participants