From 9db02dcc8589f56173d8f1ff08c06d243d14f639 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 31 May 2017 23:15:41 +0200 Subject: [PATCH] async_hooks,http: fix socket reuse with Agent Under very specific circumstances the `http` implementation could be brought to crash, because the Agent did not re-assign the async id field properly after setting up a socket for reuse. Fixes: https://github.com/nodejs/node/issues/13325 PR-URL: https://github.com/nodejs/node/pull/13348 Reviewed-By: Refael Ackermann Reviewed-By: James M Snell Reviewed-By: Andreas Madsen --- lib/_http_agent.js | 1 + test/parallel/test-async-hooks-http-agent.js | 79 ++++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 test/parallel/test-async-hooks-http-agent.js diff --git a/lib/_http_agent.js b/lib/_http_agent.js index d791a961c76fe2..60fff343dc66cf 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -168,6 +168,7 @@ Agent.prototype.addRequest = function addRequest(req, options, port/*legacy*/, var socket = this.freeSockets[name].shift(); // Assign the handle a new asyncId and run any init() hooks. socket._handle.asyncReset(); + socket[async_id_symbol] = socket._handle.getAsyncId(); debug('have free socket'); // don't leak diff --git a/test/parallel/test-async-hooks-http-agent.js b/test/parallel/test-async-hooks-http-agent.js new file mode 100644 index 00000000000000..348b97b111658a --- /dev/null +++ b/test/parallel/test-async-hooks-http-agent.js @@ -0,0 +1,79 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const async_id_symbol = process.binding('async_wrap').async_id_symbol; +const http = require('http'); + +// Regression test for https://github.com/nodejs/node/issues/13325 +// Checks that an http.Agent properly asyncReset()s a reused socket handle, and +// re-assigns the fresh async id to the reused `net.Socket` instance. + +// Make sure a single socket is transpartently reused for 2 requests. +const agent = new http.Agent({ + keepAlive: true, + keepAliveMsecs: Infinity, + maxSockets: 1 +}); + +const server = http.createServer(common.mustCall((req, res) => { + req.once('data', common.mustCallAtLeast(() => { + res.writeHead(200, {'Content-Type': 'text/plain'}); + res.write('foo'); + })); + req.on('end', common.mustCall(() => { + res.end('bar'); + })); +}, 2)).listen(0, common.mustCall(() => { + const port = server.address().port; + const payload = 'hello world'; + + // First request. This is useless except for adding a socket to the + // agent’s pool for reuse. + const r1 = http.request({ + agent, port, method: 'POST' + }, common.mustCall((res) => { + // Remember which socket we used. + const socket = res.socket; + const asyncIdAtFirstRequest = socket[async_id_symbol]; + assert.ok(asyncIdAtFirstRequest > 0, `${asyncIdAtFirstRequest} > 0`); + // Check that request and response share their socket. + assert.strictEqual(r1.socket, socket); + + res.on('data', common.mustCallAtLeast(() => {})); + res.on('end', common.mustCall(() => { + // setImmediate() to give the agent time to register the freed socket. + setImmediate(common.mustCall(() => { + // The socket is free for reuse now. + assert.strictEqual(socket[async_id_symbol], -1); + + // Second request. To re-create the exact conditions from the + // referenced issue, we use a POST request without chunked encoding + // (hence the Content-Length header) and call .end() after the + // response header has already been received. + const r2 = http.request({ + agent, port, method: 'POST', headers: { + 'Content-Length': payload.length + } + }, common.mustCall((res) => { + const asyncId = res.socket[async_id_symbol]; + assert.ok(asyncId > 0, `${asyncId} > 0`); + assert.strictEqual(r2.socket, socket); + // Empty payload, to hit the “right” code path. + r2.end(''); + + res.on('data', common.mustCallAtLeast(() => {})); + res.on('end', common.mustCall(() => { + // Clean up to let the event loop stop. + server.close(); + agent.destroy(); + })); + })); + + // Schedule a payload to be written immediately, but do not end the + // request just yet. + r2.write(payload); + })); + })); + })); + r1.end(payload); +}));