Skip to content

Commit

Permalink
Add timeout for secureConnect event for HTTPS requests (#536)
Browse files Browse the repository at this point in the history
  • Loading branch information
jstewmon authored and sindresorhus committed Jul 25, 2018
1 parent 9d87e9f commit da7f055
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 95 deletions.
1 change: 1 addition & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ This also accepts an `object` with the following fields to constrain the duratio

- `lookup` starts when a socket is assigned and ends when the hostname has been resolved. Does not apply when using a Unix domain socket.
- `connect` starts when `lookup` completes (or when the socket is assigned if lookup does not apply to the request) and ends when the socket is connected.
- `secureConnect` starts when `connect` completes and ends when the handshaking process completes (HTTPS only).
- `socket` starts when the socket is connected. See [request.setTimeout](https://nodejs.org/api/http.html#http_request_settimeout_timeout_callback).
- `response` starts when the request has been written to the socket and ends when the response headers are received.
- `send` starts when the socket is connected and ends with the request has been written to the socket.
Expand Down
16 changes: 16 additions & 0 deletions source/timed-out.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,22 @@ module.exports = (request, options) => {
});
}

if (delays.secureConnect !== undefined && options.protocol === 'https:') {
request.once('socket', socket => {
if (socket.connecting) {
socket.once('connect', () => {
const cancelTimeout = addTimeout(
delays.secureConnect,
timeoutHandler,
'secureConnect'
);
cancelers.push(cancelTimeout);
socket.once('secureConnect', cancelTimeout);
});
}
});
}

if (delays.send !== undefined) {
request.once('socket', socket => {
const timeRequest = () => {
Expand Down
30 changes: 1 addition & 29 deletions test/agent.js
Original file line number Diff line number Diff line change
@@ -1,43 +1,15 @@
import util from 'util';
import {Agent as HttpAgent} from 'http';
import {Agent as HttpsAgent} from 'https';
import test from 'ava';
import pem from 'pem';
import sinon from 'sinon';
import got from '../source';
import {createServer, createSSLServer} from './helpers/server';

let http;
let https;

const createCertificate = util.promisify(pem.createCertificate);

test.before('setup', async () => {
const caKeys = await createCertificate({
days: 1,
selfSigned: true
});

const caRootKey = caKeys.serviceKey;
const caRootCert = caKeys.certificate;

const keys = await 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
https = await createSSLServer();
http = await createServer();

// HTTPS Handlers
Expand Down
32 changes: 30 additions & 2 deletions test/helpers/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ const util = require('util');
const http = require('http');
const https = require('https');
const getPort = require('get-port');
const pem = require('pem');

exports.host = 'localhost';
const {host} = exports;

const createCertificate = util.promisify(pem.createCertificate);

exports.createServer = async () => {
const port = await getPort();

Expand All @@ -25,17 +28,42 @@ exports.createServer = async () => {
return s;
};

exports.createSSLServer = async options => {
exports.createSSLServer = async () => {
const port = await getPort();

const s = https.createServer(options, (request, response) => {
const caKeys = await createCertificate({
days: 1,
selfSigned: true
});

const caRootKey = caKeys.serviceKey;
const caRootCert = caKeys.certificate;

const keys = await 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;

const s = https.createServer({cert, key}, (request, response) => {
s.emit(request.url, request, response);
});

s.host = host;
s.port = port;
s.url = `https://${host}:${port}`;
s.protocol = 'https';
s.caRootCert = caRootCert;

s.listen = util.promisify(s.listen);
s.close = util.promisify(s.close);
Expand Down
35 changes: 3 additions & 32 deletions test/https.js
Original file line number Diff line number Diff line change
@@ -1,40 +1,11 @@
import util from 'util';
import test from 'ava';
import pem from 'pem';
import got from '../source';
import {createSSLServer} from './helpers/server';

let s;
let caRootCert;

const createCertificate = util.promisify(pem.createCertificate);

test.before('setup', async () => {
const caKeys = await createCertificate({
days: 1,
selfSigned: true
});

const caRootKey = caKeys.serviceKey;
caRootCert = caKeys.certificate;

const keys = await 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;

s = await createSSLServer({key, cert});
s = await createSSLServer();

s.on('/', (request, response) => response.end('ok'));

Expand All @@ -51,15 +22,15 @@ test('make request to https server without ca', async t => {

test('make request to https server with ca', async t => {
const {body} = await got(s.url, {
ca: caRootCert,
ca: s.caRootCert,
headers: {host: 'sindresorhus.com'}
});
t.is(body, 'ok');
});

test('protocol-less URLs default to HTTPS', async t => {
const {body, requestUrl} = await got(s.url.replace(/^https:\/\//, ''), {
ca: caRootCert,
ca: s.caRootCert,
headers: {host: 'sindresorhus.com'}
});
t.is(body, 'ok');
Expand Down
30 changes: 1 addition & 29 deletions test/redirects.js
Original file line number Diff line number Diff line change
@@ -1,41 +1,13 @@
import util from 'util';
import {URL} from 'url';
import test from 'ava';
import pem from 'pem';
import got from '../source';
import {createServer, createSSLServer} from './helpers/server';

let http;
let https;

const createCertificate = util.promisify(pem.createCertificate);

test.before('setup', async () => {
const caKeys = await createCertificate({
days: 1,
selfSigned: true
});

const caRootKey = caKeys.serviceKey;
const caRootCert = caKeys.certificate;

const keys = await 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});
https = await createSSLServer();
http = await createServer();

// HTTPS Handlers
Expand Down
40 changes: 37 additions & 3 deletions test/timeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import test from 'ava';
import pEvent from 'p-event';
import delay from 'delay';
import got from '../source';
import {createServer} from './helpers/server';
import {createServer, createSSLServer} from './helpers/server';

let s;
let ss;

const slowDataStream = () => {
const slowStream = new stream.PassThrough();
Expand Down Expand Up @@ -36,7 +37,7 @@ const keepAliveAgent = new http.Agent({
});

test.before('setup', async () => {
s = await createServer();
[s, ss] = await Promise.all([createServer(), createSSLServer()]);

s.on('/', async (request, response) => {
request.on('data', () => {});
Expand All @@ -58,7 +59,11 @@ test.before('setup', async () => {
response.end('OK');
});

await s.listen(s.port);
ss.on('/', async (request, response) => {
response.end('OK');
});

await Promise.all([s.listen(s.port), ss.listen(ss.port)]);
});

test('timeout option (ETIMEDOUT)', async t => {
Expand Down Expand Up @@ -248,6 +253,35 @@ test('connect timeout (ip address)', async t => {
);
});

test('secureConnect timeout', async t => {
await t.throws(
got(ss.url, {
timeout: {secureConnect: 1},
retry: 0,
rejectUnauthorized: false
}),
{
...errorMatcher,
message: `Timeout awaiting 'secureConnect' for 1ms`
}
);
});

test('secureConnect timeout not breached', async t => {
const secureConnect = 200;
await got(ss.url, {
timeout: {secureConnect},
retry: 0,
rejectUnauthorized: false
}).on('request', request => {
request.on('error', error => {
t.fail(`error emitted: ${error}`);
});
});
await delay(secureConnect * 2);
t.pass('no error emitted');
});

test('lookup timeout', async t => {
await t.throws(
got({
Expand Down

0 comments on commit da7f055

Please sign in to comment.