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: #8647Fixes: #6687 Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jackson Tian <[email protected]> Reviewed-By: James M Snell <[email protected]>

Author: imyller

lib/_http_agent.js

@@ -123,6 +123,14 @@ Agent.prototype.addRequest = function(req, options){
 options=util._extend({},options);
 options=util._extend(options,this.options);
 
+if(!options.servername){
+options.servername=options.host;
+consthostHeader=req.getHeader('host');
+if(hostHeader){
+options.servername=hostHeader.replace(/:.*$/,'');
+}
+}
+
 varname=this.getName(options);
 if(!this.sockets[name]){
 this.sockets[name]=[];

test/parallel/test-https-agent-sockets-leak.js

@@ -0,0 +1,52 @@
+'use strict';
+
+constcommon=require('../common');
+if(!common.hasCrypto){
+common.skip('missing crypto');
+return;
+}
+
+constfs=require('fs');
+consthttps=require('https');
+constassert=require('assert');
+
+constoptions={
+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')
+};
+
+constserver=https.Server(options,common.mustCall((req,res)=>{
+res.writeHead(200);
+res.end('https\n');
+}));
+
+constagent=newhttps.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);
+}));
+}));
+}));