Crash when using req.connection.remoteAddress (cluster) #2870

Closed
VasylMarchuk opened this Issue Mar 5, 2012 · 6 comments

Projects

None yet

3 participants

@VasylMarchuk

mac os x 10.7.3
nginx proxying web requests to socket.sock

location / {
    proxy_pass http://unix:/var/something/socket.sock:$uri$is_args$args;
}

node v0.6.12

if(!cluster.isMaster) {
    process.on('uncaughtException', function(e){
        console.error(e);
    });

    http.Server(function(req, res) {
        try {
            console.log(req.connection.remoteAddress); //crashes here
            res.writeHead(200);
            res.end("hello world\n");
        }
        catch(e) {
            console.error(e);
        }
    }).listen('./socket.sock');
}

Upon connection, dies with:

Assertion failed: (0 && "bad address family"), function GetPeerName, file ../src/tcp_wrap.cc, line 237.

Neither uncaughtException nor try catch the error :(

Owner

This is a bug in Node obviously but note that req.connection.remoteAddress won't do the right thing anyway. It returns the name of the socket (in this case ./socket.sock), not the address of the client.

@bnoordhuis Of course, in this particular case I need to use X-Forwarded-For header, but it shouldn't crash anyway

Owner

@vasman: Quick sanity check, do you also get the crash when you don't use the the cluster module?

@bnoordhuis Same configuration, no cluster:

var http = require('http');

process.on('uncaughtException', function(e){
    console.error(e);
});

http.Server(function(req, res) {
    try {
        console.log(req.connection.remoteAddress); //prints 'undefined'
        res.writeHead(200);
        res.end("hello world\n"); //prints 'hello world' to the browser
    }
    catch(e) {
        console.error(e);
    }
}).listen('./socket.sock');

prints undefined
prints hello world to the browser

Sorry guys, I would love to help with a pull request, but it's in ../src/tcp_wrap.cc and I don't know c :)

Member

I have started debugging this. I'm currently using this testcase:

var cluster = require('cluster'),
    net = require('net');

var usePath = true ? './socket.sock' : 20005;
var useCluster = true;

if (cluster.isMaster && useCluster) {
  cluster.fork();

} else if (cluster.isWorker || !useCluster) {
  net.createServer(function(socket) {

    console.log(socket.remoteAddress); //crashes here
    socket.end("hello world\n");

  }).listen(usePath, function () {
    net.connect(usePath).pipe(process.stdout);
  });
}

What I have currently conclude is that when using usePath = true and useCluster = false the socket.remoteAddress property is not ./socket.sock but undefined. It shows that handle.getpeername dont exist in this case, however when using cluster is for some reason do.

I'm guessing this is caused by they way the server handle is send, so when received it lacks some property there should have indicated that getpeername shoudn't be set as a method.

@bnoordhuis bnoordhuis added a commit that closed this issue Mar 9, 2012
@bnoordhuis bnoordhuis cluster: support passing of named pipes
Fixes triggered assertion:

  Assertion failed: (0 && "bad address family"), function GetPeerName,
  file ../src/tcp_wrap.cc, line 237.

Fixes #2870.
296b7a5
@bnoordhuis bnoordhuis closed this in 296b7a5 Mar 9, 2012
Owner

Fixed in libuv in joyent/libuv@8c78cb4, fixed in node in 296b7a5.

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