diff options
author | Ilkka Myller <ilkka.myller@nodefield.com> | 2016-09-18 14:34:48 +0300 |
---|---|---|
committer | Ilkka Myller <ilkka.myller@nodefield.com> | 2016-09-21 03:22:15 +0300 |
commit | db5a8791fac4d7e088114488f9f5fda4c660ed5a (patch) | |
tree | 7e09e984233c6fbe6d673470458de7ef8cab96ba /test/parallel/test-https-agent-sockets-leak.js | |
parent | c5ce7f4d9e36fc9f06547de1e41cdf4b2ac02720 (diff) | |
download | android-node-v8-db5a8791fac4d7e088114488f9f5fda4c660ed5a.tar.gz android-node-v8-db5a8791fac4d7e088114488f9f5fda4c660ed5a.tar.bz2 android-node-v8-db5a8791fac4d7e088114488f9f5fda4c660ed5a.zip |
https: fix memory leak with https.request()
If calling `https.request()` with `options.headers.host` defined
and `options.servername` undefined, `https.Agent.createSocket` mutates
connection `options` after `https.Agent.addRequest` has created empty
socket pool array with mismatching connection name. This results in two
socket pool arrays being created and only the last one gets eventually
deleted by `removeSocket` - causing a memory leak.
This commit fixes the leak by making sure that `addRequest` does the
same modifications to `options` object as the `createSocket`.
`createSocket` is intentionally left unmodified to prevent userland
regressions.
Test case included.
PR-URL: https://github.com/nodejs/node/pull/8647
Fixes: https://github.com/nodejs/node/issues/6687
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jackson Tian <shvyo1987@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Diffstat (limited to 'test/parallel/test-https-agent-sockets-leak.js')
-rw-r--r-- | test/parallel/test-https-agent-sockets-leak.js | 52 |
1 files changed, 52 insertions, 0 deletions
diff --git a/test/parallel/test-https-agent-sockets-leak.js b/test/parallel/test-https-agent-sockets-leak.js new file mode 100644 index 0000000000..27b37dc4fe --- /dev/null +++ b/test/parallel/test-https-agent-sockets-leak.js @@ -0,0 +1,52 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) { + common.skip('missing crypto'); + return; +} + +const fs = require('fs'); +const https = require('https'); +const assert = require('assert'); + +const options = { + key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'), + cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem'), + ca: fs.readFileSync(common.fixturesDir + '/keys/ca1-cert.pem') +}; + +const server = https.Server(options, common.mustCall((req, res) => { + res.writeHead(200); + res.end('https\n'); +})); + +const agent = new https.Agent({ + keepAlive: false +}); + +server.listen(0, common.mustCall(() => { + https.get({ + host: server.address().host, + port: server.address().port, + headers: {host: 'agent1'}, + rejectUnauthorized: true, + ca: options.ca, + agent: agent + }, common.mustCall((res) => { + res.resume(); + server.close(); + + // Only one entry should exist in agent.sockets pool + // If there are more entries in agent.sockets, + // removeSocket will not delete them resulting in a resource leak + assert.strictEqual(Object.keys(agent.sockets).length, 1); + + res.req.on('close', common.mustCall(() => { + // To verify that no leaks occur, check that no entries + // exist in agent.sockets pool after both request and socket + // has been closed. + assert.strictEqual(Object.keys(agent.sockets).length, 0); + })); + })); +})); |