Server.stop can be postponed indefinitely by a remote client keeping a connection active [Security, DoS]

[geek]

Here is the server code:

var Http = require('http');

var server = Http.createServer(function (req, res) {

    res.writeHead(200);
    res.end('Hello');
});

server.listen(8080, '127.0.0.1');

process.on('SIGUSR2', function () {

    console.time('stop');
    console.log('Stopping...');

    server.close(function () {

        console.timeEnd('stop');
    });
});

After running the server, run the following client code to keep the connection active:

var Net = require('net');

var client = Net.connect(8080, function () {

    setInterval(function () {

        client.write('GET / HTTP/1.1\r\n\r\n');
    }, 1000);
});

client.on('data', function(data) {

    console.log(data.toString());
});

Now send a SIGUSR2 signal to the server pid:

kill -s SIGUSR2 [pid]

Wait forever and the server will never be shutdown. It is finally stopped when the client is closed so that the connection is closed. Here is the output:

$node -v
v0.10.2
$node close.js
Stopping...
stop: 713285ms

As you can see, I let it run for about 12 minutes before finally terminating the client process. Any server that wishes to close can be forced to remain running. There should be a mechanism to forcefully destroy any open connections. This allows a server to optionally wait for a few seconds for connections to close and if they don't then the server can continue with a shutdown.

[geek]

I would also expect that any idle connection that exists at the time server.close is called is prevented from going into timers.active.

[bnoordhuis]

htp.Server#close() and net.Server#close() stop the server from accepting new connections and that's all they do. If you're worried about hostile clients, close their connections manually. Working as intended, closing.

[geek]

To stop the connections manually will also require keeping a pool of all connections, as this mechanism also doesn't exist at the moment. That is fine, but shouldn't the server.close prevent an open connection that is currently idle from becoming active again? If all future connections are prevented, shouldn't existing idle connections just be closed?

[bnoordhuis]

shouldn't existing idle connections just be closed?

It's actually explicitly designed (and documented) this way because no matter what scheme you think up, some kind of auto-close functionality will never cover all possible use cases (in other words, never please all people.)

It's the old UNIX saw: provide mechanism, not policy.

[isaacs]

I'd be ok with a Server.destroy() method to forcibly close all connections, but as @bnoordhuis mentioned, there's not very much chance of a change in policy here. Many programs depend on the "stop listening, but don't forcibly kill connections" behavior.

[bnoordhuis]

I'd be ok with a Server.destroy() method to forcibly close all connections

I would not be okay with that. It's a feature precious few people need but the overhead of tracking all connections is something you impose on all users. It's bad enough we have moronic stuff like net.Server#connections in there but at least that's a simple integer.

There's no reason for it to be in core. It's perfectly possible to do in user land and it's not difficult either.

[isaacs]

Yeah, fair enough. To the npms!

[geek]

Sounds reasonable. Does the timers module track the current sockets? If that is the case, perhaps it isn't too much to ask to expose the timers list object?

[bnoordhuis]

Does the timers module track the current sockets?

No. A timer doesn't know what type of object it's embedded in. It's not just sockets.

to expose the timers list object?

Not all timers are in that list. I don't want to sound unfriendly but what on earth makes you think that scanning the list of timers is the best approach to tracking connections?

[isaacs]

@wpreul Here you go: http://npm.im/server-destroy

[geek]

Thanks for the help with this. My point with the timers was more about the fact that there already is overhead with tracking the active sockets, so whats the harm in exposing these active sockets in some form.