Race condition when getting remoteAddress of connection

[LinusU]

If the connection has already been closed before the connect event is handled by user code, the remoteAddress of the connection will not be available.

Reproducing this is very hard but I managed to get it working sometimes with gnu netcat version 0.7.1 with the close flag, executed on another machine:

hostname | nc -nc <ip> 1337

Testcase

var net = require('net');
var assert = require('assert');

net.createServer(function (c) {
  assert(c.remoteAddress);
}).listen(1337);

Output

assert.js:92
  throw new assert.AssertionError({
        ^
AssertionError: "undefined" == true
    at Server.<anonymous> (/private/tmp/socket-race-condition/test.js:6:3)
    at Server.EventEmitter.emit (events.js:95:17)
    at TCP.onconnection (net.js:1191:8)

More info

Information about the socket can be provided when calling accept, however libuv ignores that data:

https://github.com/joyent/libuv/blob/9b4f2b84f10c96efa37910f324bc66e27aec3828/src/unix/core.c#L406

Instead, when doing c.remoteAddress a call to getpeername is executed.

1. remoteAddress https://github.com/joyent/node/blob/940974ed039d3c9a8befe608d9c95b2ffdb457d3/lib/net.js#L569
2. _getpeername https://github.com/joyent/node/blob/940974ed039d3c9a8befe608d9c95b2ffdb457d3/lib/net.js#L555
3. TCPWrap::GetPeerName https://github.com/joyent/node/blob/940974ed039d3c9a8befe608d9c95b2ffdb457d3/src/tcp_wrap.cc#L182
4. uv_tcp_getpeername https://github.com/joyent/libuv/blob/9b4f2b84f10c96efa37910f324bc66e27aec3828/src/unix/tcp.c#L187

I think that the only reliable way to get the remoteAddress is to get it from the accept call and store it along with the connection.

Additional error

In addition to the assertion error, if I only log the c.remoteAddress and don't terminate the process, I will get the following error. It's not emitted on the server (net.createServer) and thus I can't trap it with server.on('error', ...).

Error: read ECONNRESET
    at errnoException (net.js:904:11)
    at TCP.onread (net.js:558:19)

syscall: read
code: ECONNRESET

[bnoordhuis]

Your analysis is correct but...

Information about the socket can be provided when calling accept, however libuv ignores that data

That's intentional, see commit joyent/libuv@752ac30. The kernel's copying out of the struct sockaddr has a performance cost and it's frequently unused. Besides, you'd have to store it somewhere. That's not possible without breaking the uv_tcp_t ABI.

Even if libuv stored the struct sockaddr, you'd still have the getpeername() issue with outbound connections.

Lastly, I bet there are people out there doing crazy things like remapping network interfaces on the fly and expecting the result of socket.remoteAddr to reflect that.

[tjfontaine]

When doing either net.connect or creating a new socket from a server accepting a socket it's easy to always have remoteAddress available, because they are implicit. Finding out localAddress on the other hand requires an extra syscall to find that information.

The fact that libuv's uv_accept doesn't have the optional struct sockaddr has come up, and while it means we can't fix this in a stable release, we can do something about it going forward.

In any, yes it's unfortunate that right now you may not get that information -- we know about it and we're working on a solution that improves the situation.

[LinusU]

I guess that we could fix Error: read ECONNRESET right away. I think it's because we are calling read on a socket that is closed. Since this isn't a user triggered action (I haven't asked it to read anything from the socket, it just does) I don't think that it should trigger an error.

The correct behaviour (I think) is to instead of emit('error', 'ECONNRESET') do emit('end')?

[wildchild]

It seems my issue is related #16523

Any status update on it? It's a kind of weird expectation that socket.remoteAddress can be undefined on 'connection' event.

[sammy007]

I managed to detect, that you can get undefined socket.remoteAddress even on connection where 'data' event fired and data received from client. My app can receive and handle packets from clients with undefined IP. I have some policy based on remoteAddress and I can't serve such clients because I can't detect their IPs inside node app and apply specific rules. Seriously, any progress on it?

[sashahilton00]

Is there any update on this? Am seeing this happening 25% of the time in nodetunes, wondered if there is a fix due out?

[gibfahn]

This repo is an archive, if you're still seeing this please open an issue at nodejs/node (if there isn't one there already).