Skip to content

Commit

Permalink
Fix socket connect listener memory leak (sindresorhus#406)
Browse files Browse the repository at this point in the history
  • Loading branch information
pietermees authored and sindresorhus committed Nov 8, 2017
1 parent ae1a0fe commit 0c5e44c
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 23 deletions.
2 changes: 1 addition & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ function requestAsEventEmitter(opts) {
total: uploadBodySize
});

req.connection.on('connect', () => {
req.connection.once('connect', () => {
const uploadEventFrequency = 150;

progressInterval = setInterval(() => {
Expand Down
150 changes: 150 additions & 0 deletions test/agent.js
Original file line number Diff line number Diff line change
@@ -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();
});
22 changes: 0 additions & 22 deletions test/redirects.js
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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, '');
Expand Down

0 comments on commit 0c5e44c

Please sign in to comment.