Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

General improvements to HTTPS API (Second Edition) #1435

Closed
wants to merge 9 commits into from
Closed
89 changes: 67 additions & 22 deletions source/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -858,6 +858,20 @@ export interface HTTPSOptions {
The passphrase to decrypt the `options.https.key` (if different keys have different passphrases refer to `options.https.key` documentation).
*/
passphrase?: SecureContextOptions['passphrase'];

// TODO add comment
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// TODO: example

ciphers?: SecureContextOptions['ciphers'];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this support an array of ciphers instead of string with :?


// TODO add comment
honorCipherOrder?: SecureContextOptions['honorCipherOrder'];

// TODO add comment
minVersion?: SecureContextOptions['minVersion'];

// TODO add comment
maxVersion?: SecureContextOptions['maxVersion'];

// TODO add comment
pfx?: SecureContextOptions['pfx'];
}

Expand Down Expand Up @@ -1550,6 +1564,10 @@ export default class Request extends Duplex implements RequestEvents<Request> {
assert.any([is.string, is.object, is.array, is.undefined], options.https.key);
assert.any([is.string, is.object, is.array, is.undefined], options.https.certificate);
assert.any([is.string, is.undefined], options.https.passphrase);
assert.any([is.string, is.undefined], options.https.ciphers);
assert.any([is.boolean, is.undefined], options.https.honorCipherOrder);
assert.any([is.string, is.undefined], options.https.minVersion);
assert.any([is.string, is.undefined], options.https.maxVersion);
assert.any([is.string, is.buffer, is.array, is.undefined], options.https.pfx);
}

Expand Down Expand Up @@ -1820,33 +1838,28 @@ export default class Request extends Duplex implements RequestEvents<Request> {
deprecationWarning('"options.family" was never documented, please use "options.dnsLookupIpVersion"');
}

// HTTPS options
// HTTPS Options
if (defaults?.https) {
options.https = {...defaults.https, ...options.https};
}

if ('rejectUnauthorized' in options) {
deprecationWarning('"options.rejectUnauthorized" is now deprecated, please use "options.https.rejectUnauthorized"');
}

if ('checkServerIdentity' in options) {
deprecationWarning('"options.checkServerIdentity" was never documented, please use "options.https.checkServerIdentity"');
}

if ('ca' in options) {
deprecationWarning('"options.ca" was never documented, please use "options.https.certificateAuthority"');
}

if ('key' in options) {
deprecationWarning('"options.key" was never documented, please use "options.https.key"');
}

if ('cert' in options) {
deprecationWarning('"options.cert" was never documented, please use "options.https.certificate"');
}
const deprecatedHttpsOptions = {
rejectUnauthorized: 'rejectUnauthorized',
checkServerIdentity: 'checkServerIdentity',
ca: 'certificateAuthority',
key: 'key',
cert: 'certificate',
passphrase: 'passphrase',
ciphers: 'ciphers',
honorCipherOrder: 'honorCipherOrder',
minVersion: 'minVersion',
maxVersion: 'maxVersion'
};

if ('passphrase' in options) {
deprecationWarning('"options.passphrase" was never documented, please use "options.https.passphrase"');
for (const httpsOption in deprecatedHttpsOptions) {
if (httpsOption in options) {
deprecationWarning(`"options.${httpsOption}" is now deprecated, please use "options.https.${httpsOption}"`);
}
}

if ('pfx' in options) {
Expand Down Expand Up @@ -2414,6 +2427,22 @@ export default class Request extends Duplex implements RequestEvents<Request> {
requestOptions.passphrase = options.https.passphrase;
}

if (options.https.ciphers) {
requestOptions.ciphers = options.https.ciphers;
}

if ('honorCipherOrder' in options.https) {
requestOptions.honorCipherOrder = options.https.honorCipherOrder;
}

if (options.https.minVersion) {
requestOptions.minVersion = options.https.minVersion;
}

if (options.https.maxVersion) {
requestOptions.maxVersion = options.https.maxVersion;
}

if (options.https.pfx) {
requestOptions.pfx = options.https.pfx;
}
Expand Down Expand Up @@ -2458,6 +2487,22 @@ export default class Request extends Duplex implements RequestEvents<Request> {
delete requestOptions.passphrase;
}

if (options.https.ciphers) {
delete requestOptions.ciphers;
}

if ('honorCipherOrder' in options.https) {
delete requestOptions.honorCipherOrder;
}

if (options.https.minVersion) {
delete requestOptions.minVersion;
}

if (options.https.maxVersion) {
delete requestOptions.maxVersion;
}

if (options.https.pfx) {
delete requestOptions.pfx;
}
Expand Down
12 changes: 10 additions & 2 deletions test/helpers/create-https-test-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,15 @@ import net = require('net');
import express = require('express');
import pify = require('pify');
import pem = require('pem');
import {SecureClientSessionOptions} from 'http2';

export type HttpsServerOptions = {
commonName?: string;
requestCert?: SecureClientSessionOptions['requestCert'];
days?: number;
ciphers?: SecureClientSessionOptions['ciphers'];
minVersion?: SecureClientSessionOptions['minVersion'];
maxVersion?: SecureClientSessionOptions['maxVersion'];
};

export interface ExtendedHttpsTestServer extends express.Express {
Expand Down Expand Up @@ -48,8 +53,11 @@ const createHttpsTestServer = async (options: HttpsServerOptions = {}): Promise<
key: serverKey,
cert: serverCert,
ca: caCert,
requestCert: true,
rejectUnauthorized: false // This should be checked by the test
requestCert: options.requestCert,
rejectUnauthorized: false, // This should be checked by the test
ciphers: options.ciphers,
minVersion: options.minVersion,
maxVersion: options.maxVersion
},
server
);
Expand Down
6 changes: 1 addition & 5 deletions test/helpers/with-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ export default generateHook({install: false});

export const withServerAndFakeTimers = generateHook({install: true});

const generateHttpsHook = (options?: HttpsServerOptions, installFakeTimer = false): test.Macro<[RunTestWithHttpsServer]> => async (t, run) => {
const fakeTimer = installFakeTimer ? FakeTimers.install() as GlobalClock : undefined;

export const withHttpsServer = (options?: HttpsServerOptions): test.Macro<[RunTestWithHttpsServer]> => async (t, run) => {
const server = await createHttpsTestServer(options);

const preparedGot = got.extend({
Expand Down Expand Up @@ -100,8 +98,6 @@ const generateHttpsHook = (options?: HttpsServerOptions, installFakeTimer = fals
}
};

export const withHttpsServer = generateHttpsHook;

// TODO: remove this when `create-test-server` supports custom listen
export const withSocketServer: test.Macro<[RunTestWithSocket]> = async (t, run) => {
const socketPath = tempy.file({extension: 'socket'});
Expand Down
2 changes: 2 additions & 0 deletions test/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,8 @@ test('invalid `dnsLookupIpVersion`', withServer, async (t, server, got) => {
});

test.serial('deprecated `family` option', withServer, async (t, server, got) => {
process.removeAllListeners('warning');

server.get('/', (_request, response) => {
response.end('ok');
});
Expand Down