Skip to content

Commit

Permalink
Merge pull request #31 from Nicolas-Duboc-IBM/fix/socket-leak-on-redi…
Browse files Browse the repository at this point in the history
…rects

bugfix: don't leak sockets when handling redirects, and honor provide…
  • Loading branch information
laurisvan committed Nov 23, 2019
2 parents 7aa68c5 + b803847 commit 8de4407
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 1 deletion.
2 changes: 2 additions & 0 deletions src/Request.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ export default class Request {
// Form the transport options from input options
const transOpts = {
method,
agent: options.agent,
hostname: url.hostname,
port: url.port,
path: url.path,
Expand Down Expand Up @@ -326,6 +327,7 @@ export default class Request {
if (status >= 301 && status <= 303) {
const location = res.headers.location;
const options = _this.options;
res.resume();

// If we're out of the redirect quota, reject
if (options.maxRedirects < 1) {
Expand Down
3 changes: 2 additions & 1 deletion test/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"strict": [2, "global"],
"lines-around-directive": 0,
"func-names": 0,
"no-unused-expressions": 0
"no-unused-expressions": 0,
"prefer-rest-params": 0
}
}
26 changes: 26 additions & 0 deletions test/RequestSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

const expect = require('chai').expect;
const proxyquire = require('proxyquire');
const http = require('http');
const https = require('https');
const net = require('net');
const EventEmitter = require('events').EventEmitter;
const Stream = require('stream');
const Request = require('../lib/Request');
Expand Down Expand Up @@ -209,15 +212,38 @@ describe('Request - test against httpbin.org', () => {
});
});

it('Honors http agent provided by user', () => {
const callTrace = [];
const agent = new http.Agent();
agent.createConnection = function () {
callTrace.push(1);
return net.createConnection.apply(null, arguments);
};
request = new Request('GET', 'http://httpbin.org/get', {
agent,
});

return request.run().then(response => {
expect(response).to.exist;
expect(callTrace).to.have.lengthOf(1);
});
});

it('Supports 301-303 redirects', () => {
url = 'https://httpbin.org/redirect-to';
const keepAliveAgent = new https.Agent({ keepAlive: true });
request = new Request('GET', url, {
json: true,
qs: { url: 'https://httpbin.org/get' },
agent: keepAliveAgent,
});


return request.run().then(response => {
expect(response).to.exist;
const socketName = keepAliveAgent.getName({ host: 'httpbin.org', port: 443 });
expect(keepAliveAgent.freeSockets[socketName]).to.have.lengthOf(2);
expect(keepAliveAgent.sockets[socketName]).to.not.exist;
});
});

Expand Down

0 comments on commit 8de4407

Please sign in to comment.