Fix socket connect listener memory leak (#406)

pietermees · sindresorhus · commit 0c5e44ce3cdb · 2017-11-08T15:52:52.000+07:00
diff --git a/index.js b/index.js
@@ -221,7 +221,7 @@ function requestAsEventEmitter(opts) {
 					total: uploadBodySize
 				});
 
-				req.connection.on('connect', () => {
+				req.connection.once('connect', () => {
 					const uploadEventFrequency = 150;
 
 					progressInterval = setInterval(() => {
diff --git a/test/agent.js b/test/agent.js
@@ -0,0 +1,150 @@
+import {Agent as HttpAgent} from 'http';
+import {Agent as HttpsAgent} from 'https';
+import test from 'ava';
+import pem from 'pem';
+import pify from 'pify';
+import sinon from 'sinon';
+import got from '..';
+import {createServer, createSSLServer} from './helpers/server';
+
+let http;
+let https;
+
+const pemP = pify(pem, Promise);
+
+test.before('setup', async () => {
+	const caKeys = await pemP.createCertificate({
+		days: 1,
+		selfSigned: true
+	});
+
+	const caRootKey = caKeys.serviceKey;
+	const caRootCert = caKeys.certificate;
+
+	const keys = await pemP.createCertificate({
+		serviceCertificate: caRootCert,
+		serviceKey: caRootKey,
+		serial: Date.now(),
+		days: 500,
+		country: '',
+		state: '',
+		locality: '',
+		organization: '',
+		organizationUnit: '',
+		commonName: 'sindresorhus.com'
+	});
+
+	const key = keys.clientKey;
+	const cert = keys.certificate;
+
+	https = await createSSLServer({key, cert}); // eslint-disable-line object-property-newline
+	http = await createServer();
+
+	// HTTPS Handlers
+
+	https.on('/', (req, res) => {
+		res.end('https');
+	});
+
+	https.on('/httpsToHttp', (req, res) => {
+		res.writeHead(302, {
+			location: http.url
+		});
+		res.end();
+	});
+
+	// HTTP Handlers
+
+	http.on('/', (req, res) => {
+		res.end('http');
+	});
+
+	http.on('/httpToHttps', (req, res) => {
+		res.writeHead(302, {
+			location: https.url
+		});
+		res.end();
+	});
+
+	await http.listen(http.port);
+	await https.listen(https.port);
+});
+
+const createAgentSpy = Cls => {
+	const agent = new Cls({keepAlive: true});
+	const spy = sinon.spy(agent, 'addRequest');
+	return {agent, spy};
+};
+
+test('non-object agent option works with http', async t => {
+	const {agent, spy} = createAgentSpy(HttpAgent);
+	t.truthy((await got(`${http.url}/`, {
+		rejectUnauthorized: false,
+		agent
+	})).body);
+	t.true(spy.calledOnce);
+	// Make sure to close all open sockets
+	agent.destroy();
+});
+
+test('non-object agent option works with https', async t => {
+	const {agent, spy} = createAgentSpy(HttpsAgent);
+	t.truthy((await got(`${https.url}/`, {
+		rejectUnauthorized: false,
+		agent
+	})).body);
+	t.true(spy.calledOnce);
+	// Make sure to close all open sockets
+	agent.destroy();
+});
+
+test('redirects from http to https work with an agent object', async t => {
+	const {agent: httpAgent, spy: httpSpy} = createAgentSpy(HttpAgent);
+	const {agent: httpsAgent, spy: httpsSpy} = createAgentSpy(HttpsAgent);
+	t.truthy((await got(`${http.url}/httpToHttps`, {
+		rejectUnauthorized: false,
+		agent: {
+			http: httpAgent,
+			https: httpsAgent
+		}
+	})).body);
+	t.true(httpSpy.calledOnce);
+	t.true(httpsSpy.calledOnce);
+	// Make sure to close all open sockets
+	httpAgent.destroy();
+	httpsAgent.destroy();
+});
+
+test('redirects from https to http work with an agent object', async t => {
+	const {agent: httpAgent, spy: httpSpy} = createAgentSpy(HttpAgent);
+	const {agent: httpsAgent, spy: httpsSpy} = createAgentSpy(HttpsAgent);
+	t.truthy((await got(`${https.url}/httpsToHttp`, {
+		rejectUnauthorized: false,
+		agent: {
+			http: httpAgent,
+			https: httpsAgent
+		}
+	})).body);
+	t.true(httpSpy.calledOnce);
+	t.true(httpsSpy.calledOnce);
+	// Make sure to close all open sockets
+	httpAgent.destroy();
+	httpsAgent.destroy();
+});
+
+test('socket connect listener cleaned up after request', async t => {
+	const {agent} = createAgentSpy(HttpsAgent);
+	await got(`${https.url}`, {
+		rejectUnauthorized: false,
+		agent
+	});
+	Object.keys(agent.freeSockets).forEach(k => agent.freeSockets[k]
+		.forEach(sock => t.is(sock.listenerCount('connect'), 0)));
+	// Make sure to close all open sockets
+	agent.destroy();
+});
+
+test.after('cleanup', async () => {
+	await http.close();
+	await https.close();
+});
diff --git a/test/redirects.js b/test/redirects.js
@@ -1,9 +1,6 @@
-import {Agent as HttpAgent} from 'http';
-import {Agent as HttpsAgent} from 'https';
 import test from 'ava';
 import pem from 'pem';
 import pify from 'pify';
-import sinon from 'sinon';
 import got from '..';
 import {createServer, createSSLServer} from './helpers/server';
 
@@ -190,25 +187,6 @@ test('redirects from https to http works', async t => {
 	t.truthy((await got(`${https.url}/httpsToHttp`, {rejectUnauthorized: false})).body);
 });
 
-test('redirects from https to http works with an agent object', async t => {
-	const httpAgent = new HttpAgent({keepAlive: true});
-	const httpsAgent = new HttpsAgent({keepAlive: true});
-	const httpSpy = sinon.spy(httpAgent, 'addRequest');
-	const httpsSpy = sinon.spy(httpsAgent, 'addRequest');
-	t.truthy((await got(`${https.url}/httpsToHttp`, {
-		rejectUnauthorized: false,
-		agent: {
-			http: httpAgent,
-			https: httpsAgent
-		}
-	})).body);
-	t.true(httpSpy.calledOnce);
-	t.true(httpsSpy.calledOnce);
-	// Make sure to close all open sockets
-	httpAgent.destroy();
-	httpsAgent.destroy();
-});
-
 test('redirects works with lowercase method', async t => {
 	const body = (await got(`${http.url}/relative`, {method: 'head'})).body;
 	t.is(body, '');
