From 8c80406d30b86ce984183ee74d18698a01ddcf6e Mon Sep 17 00:00:00 2001 From: Sean Hernon Date: Thu, 29 Oct 2020 18:33:20 +0000 Subject: [PATCH 1/3] Test that https module defaults to https protocol --- test/response/https.test.ts | 23 +++++++++++++++++++++++ test/utils/createServer.ts | 11 ++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/test/response/https.test.ts b/test/response/https.test.ts index e061638b7..681feeb65 100644 --- a/test/response/https.test.ts +++ b/test/response/https.test.ts @@ -1,6 +1,7 @@ /** * @jest-environment node */ +import { request } from 'https'; import { RequestInterceptor } from '../../src' import { httpsGet, httpsRequest } from '../helpers' import withDefaultInterceptors from '../../src/presets/default' @@ -61,6 +62,28 @@ test('bypasses an HTTPS request issued by "https.request" not handled in the mid expect(resBody).toEqual('/get') }) +test('Correctly handles an HTTPS request issued by "https.request" from options without protocol', async () => { + await new Promise((resolve, reject) => { + const rejectError = (error: Error) => { + reject(error) + } + + const req = request({host: server.getHttpsServerHost(), path: '/get'}, (res) => { + res.on('error', rejectError) + + res.on('end', () => { + resolve() + }) + }) + + req.on('error', rejectError) + + req.end() + }) + + //If we get here then node was happy with the protocol +}) + test('responds to an HTTPS request issued by "https.get" and handled in the middleware', async () => { const { res, resBody } = await httpsGet(server.makeHttpsUrl('/')) diff --git a/test/utils/createServer.ts b/test/utils/createServer.ts index 5dfa4d0b2..f4132c913 100644 --- a/test/utils/createServer.ts +++ b/test/utils/createServer.ts @@ -12,6 +12,7 @@ const SSL_CERT = fs.readFileSync(path.join(__dirname, 'server.cert')) type MakeUrlFunc = (path: string) => string export interface ServerAPI { + getHttpsServerHost(): string getHttpAddress(): string getHttpsAddress(): string makeHttpUrl: MakeUrlFunc @@ -48,9 +49,16 @@ function closeServer(server: http.Server) { function getServerAddress(server: http.Server | https.Server): () => string { return () => { const protocol = server.hasOwnProperty('key') ? 'https:' : 'http:' + + return new URL(`${protocol}//${getServerHost(server)()}`).href + } +} + +function getServerHost(server: http.Server | https.Server): () => string { + return () => { const { port } = server.address() as AddressInfo - return new URL(`${protocol}//localhost:${port}`).href + return `localhost:${port}` } } @@ -93,6 +101,7 @@ export async function createServer( ) return { + getHttpsServerHost: getServerHost(httpsServer), getHttpAddress: getServerAddress(httpServer), getHttpsAddress: getServerAddress(httpsServer), makeHttpUrl: makeServerUrl(httpServer), From ae1c02299bc6f255c2c42d1e14c7976c119e3c6d Mon Sep 17 00:00:00 2001 From: Sean Hernon Date: Fri, 30 Oct 2020 13:02:37 +0000 Subject: [PATCH 2/3] Pass down protocol context --- .../ClientRequest/ClientRequestOverride.ts | 3 +- src/interceptors/ClientRequest/index.ts | 1 + .../utils/normalizeHttpRequestParams.test.ts | 17 +++++++-- .../utils/normalizeHttpRequestParams.ts | 9 +++-- src/utils/getUrlByRequestOptions.test.ts | 25 +++++++----- src/utils/getUrlByRequestOptions.ts | 11 +++--- test/response/https.test.ts | 38 +++++++++++-------- test/utils/createServer.ts | 18 ++++++--- 8 files changed, 78 insertions(+), 44 deletions(-) diff --git a/src/interceptors/ClientRequest/ClientRequestOverride.ts b/src/interceptors/ClientRequest/ClientRequestOverride.ts index bd804557b..a9c4323ab 100644 --- a/src/interceptors/ClientRequest/ClientRequestOverride.ts +++ b/src/interceptors/ClientRequest/ClientRequestOverride.ts @@ -17,6 +17,7 @@ import { normalizeHttpRequestEndParams } from './utils/normalizeHttpRequestEndPa const createDebug = require('debug') export function createClientRequestOverrideClass( + protocol: string, middleware: RequestMiddleware, performOriginalRequest: typeof http.request, originalClientRequest: typeof http.ClientRequest @@ -25,7 +26,7 @@ export function createClientRequestOverrideClass( this: http.ClientRequest, ...args: Parameters ) { - const [url, options, callback] = normalizeHttpRequestParams(...args) + const [url, options, callback] = normalizeHttpRequestParams(protocol, ...args) const usesHttps = url.protocol === 'https:' let requestBodyBuffer: Buffer[] = [] diff --git a/src/interceptors/ClientRequest/index.ts b/src/interceptors/ClientRequest/index.ts index 947ed78ce..d2b5df838 100644 --- a/src/interceptors/ClientRequest/index.ts +++ b/src/interceptors/ClientRequest/index.ts @@ -31,6 +31,7 @@ function handleRequest( } const ClientRequestOverride = createClientRequestOverrideClass( + protocol, middleware, originalMethod, originalClientRequest diff --git a/src/interceptors/ClientRequest/utils/normalizeHttpRequestParams.test.ts b/src/interceptors/ClientRequest/utils/normalizeHttpRequestParams.test.ts index 3e0855700..fec453feb 100644 --- a/src/interceptors/ClientRequest/utils/normalizeHttpRequestParams.test.ts +++ b/src/interceptors/ClientRequest/utils/normalizeHttpRequestParams.test.ts @@ -7,7 +7,7 @@ test('handles [string, callback] input', () => { url, options, callback, - ] = normalizeHttpRequestParams('https://mswjs.io/resource', function cb() {}) + ] = normalizeHttpRequestParams('http', 'https://mswjs.io/resource', function cb() {}) // URL string must be converted to a URL instance expect(url.toJSON()).toEqual(new URL('https://mswjs.io/resource').toJSON()) @@ -33,6 +33,7 @@ test('handles [string, RequestOptions, callback] input', () => { options, callback, ] = normalizeHttpRequestParams( + 'http', 'https://mswjs.io/resource', initialOptions, function cb() {} @@ -50,6 +51,7 @@ test('handles [string, RequestOptions, callback] input', () => { test('handles [URL, callback] input', () => { const [url, options, callback] = normalizeHttpRequestParams( + 'http', new URL('https://mswjs.io/resource'), function cb() {} ) @@ -69,6 +71,7 @@ test('handles [URL, callback] input', () => { test('handles [Absolute Legacy URL, callback] input', () => { const [url, options, callback] = normalizeHttpRequestParams( + 'http', parse('https://cherry:durian@mswjs.io:12345/resource?apple=banana'), function cb() {} ) @@ -90,6 +93,7 @@ test('handles [Absolute Legacy URL, callback] input', () => { test('handles [Relative Legacy URL, RequestOptions without path set, callback] input', () => { const [url, options, callback] = normalizeHttpRequestParams( + 'http', parse('/resource?apple=banana'), {host: 'mswjs.io'}, function cb() {} @@ -109,6 +113,7 @@ test('handles [Relative Legacy URL, RequestOptions without path set, callback] i test('handles [Relative Legacy URL, RequestOptions with path set, callback] input', () => { const [url, options, callback] = normalizeHttpRequestParams( + 'http', parse('/resource?apple=banana'), {host: 'mswjs.io', path: '/other?cherry=durian'}, function cb() {} @@ -128,12 +133,13 @@ test('handles [Relative Legacy URL, RequestOptions with path set, callback] inpu test('handles [Relative Legacy URL, callback] input', () => { const [url, options, callback] = normalizeHttpRequestParams( + 'http', parse('/resource?apple=banana'), function cb() {} ) // Correct WHATWG URL generated - expect(url.toJSON()).toMatch(getUrlByRequestOptions({path: '/resource?apple=banana'}).toJSON()) + expect(url.toJSON()).toMatch(getUrlByRequestOptions('http', {path: '/resource?apple=banana'}).toJSON()) // Check path is in options expect(options).toHaveProperty('protocol', 'http:') @@ -145,11 +151,12 @@ test('handles [Relative Legacy URL, callback] input', () => { test('handles [Relative Legacy URL] input', () => { const [url, options, callback] = normalizeHttpRequestParams( + 'http', parse('/resource?apple=banana') ) // Correct WHATWG URL generated - expect(url.toJSON()).toMatch(getUrlByRequestOptions({path: '/resource?apple=banana'}).toJSON()) + expect(url.toJSON()).toMatch(getUrlByRequestOptions('http', {path: '/resource?apple=banana'}).toJSON()) // Check path is in options expect(options).toHaveProperty('protocol', 'http:') @@ -161,6 +168,7 @@ test('handles [Relative Legacy URL] input', () => { test('handles [URL, RequestOptions, callback] input', () => { const [url, options, callback] = normalizeHttpRequestParams( + 'http', new URL('https://mswjs.io/resource'), { headers: { @@ -196,6 +204,7 @@ test('handles [RequestOptions, callback] input', () => { }, } const [url, options, callback] = normalizeHttpRequestParams( + 'http', initialOptions, function cb() {} ) @@ -212,6 +221,7 @@ test('handles [RequestOptions, callback] input', () => { test('handles [Empty RequestOptions, callback] input', () => { const [_, __, callback] = normalizeHttpRequestParams( + 'http', {}, function cb() {} ) @@ -237,6 +247,7 @@ test('handles [PartialRequestOptions, callback] input', () => { agent: false, } const [url, options, callback] = normalizeHttpRequestParams( + 'http', initialOptions, function cb() {} ) diff --git a/src/interceptors/ClientRequest/utils/normalizeHttpRequestParams.ts b/src/interceptors/ClientRequest/utils/normalizeHttpRequestParams.ts index 97d5337cb..2fe38e20a 100644 --- a/src/interceptors/ClientRequest/utils/normalizeHttpRequestParams.ts +++ b/src/interceptors/ClientRequest/utils/normalizeHttpRequestParams.ts @@ -40,6 +40,7 @@ function isRequestOptions(arg?: RequestOptions|HttpRequestCallback): boolean { * so it always has a `URL` and `RequestOptions`. */ export function normalizeHttpRequestParams( + defaultProtocol: string, ...args: HttpRequestArgs ): [URL, RequestOptions & RequestSelf, HttpRequestCallback?] { let url: URL @@ -87,14 +88,14 @@ export function normalizeHttpRequestParams( debug('given a relative legacy url:', args[0]) return isRequestOptions(args[1]) - ? normalizeHttpRequestParams({path: args[0].path, ...args[1]}, args[2]) - : normalizeHttpRequestParams({path: args[0].path}, args[1] as HttpRequestCallback) + ? normalizeHttpRequestParams(defaultProtocol, {path: args[0].path, ...args[1]}, args[2]) + : normalizeHttpRequestParams(defaultProtocol, {path: args[0].path}, args[1] as HttpRequestCallback) } debug('given an absolute legacy url:', args[0]) //We are dealing with an absolute url, so convert to WHATWG and try again - return normalizeHttpRequestParams(...[new URL(args[0].href), ...args.slice(1)] as HttpRequestArgs) + return normalizeHttpRequestParams(defaultProtocol, ...[new URL(args[0].href), ...args.slice(1)] as HttpRequestArgs) } // Handle a given request options object as-is // and derive the URL instance from it. @@ -102,7 +103,7 @@ export function normalizeHttpRequestParams( options = args[0] debug('given request options:', options) - url = getUrlByRequestOptions(options) + url = getUrlByRequestOptions(defaultProtocol, options) debug('created a URL:', url) callback = resolveCallback(args) diff --git a/src/utils/getUrlByRequestOptions.test.ts b/src/utils/getUrlByRequestOptions.test.ts index 08378f6cd..abd61b551 100644 --- a/src/utils/getUrlByRequestOptions.test.ts +++ b/src/utils/getUrlByRequestOptions.test.ts @@ -8,7 +8,7 @@ test('returns a URL based on the basic RequestOptions', () => { host: '127.0.0.1', path: '/resource', } - const url = getUrlByRequestOptions(options) + const url = getUrlByRequestOptions('http', options) expect(url).toBeInstanceOf(URL) expect(url).toHaveProperty('port', '') @@ -21,7 +21,7 @@ test('inherits protocol and port from http.Agent, if set', () => { path: '/', agent: new HttpAgent(), } - const url = getUrlByRequestOptions(options) + const url = getUrlByRequestOptions('http', options) expect(url).toBeInstanceOf(URL) expect(url).toHaveProperty('protocol', 'http:') @@ -37,7 +37,7 @@ test('inherits protocol and port from https.Agent, if set', () => { port: 3080, }), } - const url = getUrlByRequestOptions(options) + const url = getUrlByRequestOptions('http', options) expect(url).toBeInstanceOf(URL) expect(url).toHaveProperty('protocol', 'https:') @@ -45,12 +45,19 @@ test('inherits protocol and port from https.Agent, if set', () => { expect(url).toHaveProperty('href', 'https://127.0.0.1:3080/') }) -test('resolves protocol to "http" given no explicit protocol and no certificate', () => { +test('resolves protocol from default given no explicit protocol and no certificate', () => { const options: RequestOptions = { host: '127.0.0.1', path: '/', } - const url = getUrlByRequestOptions(options) + let url = getUrlByRequestOptions('https', options) + + expect(url).toBeInstanceOf(URL) + expect(url).toHaveProperty('protocol', 'https:') + expect(url).toHaveProperty('port', '') + expect(url).toHaveProperty('href', 'https://127.0.0.1/') + + url = getUrlByRequestOptions('http', options) expect(url).toBeInstanceOf(URL) expect(url).toHaveProperty('protocol', 'http:') @@ -64,7 +71,7 @@ test('resolves protocol to "https" given no explicit protocol, but certificate', path: '/secure', cert: '', } - const url = getUrlByRequestOptions(options) + const url = getUrlByRequestOptions('http', options) expect(url).toBeInstanceOf(URL) expect(url).toHaveProperty('protocol', 'https:') @@ -79,7 +86,7 @@ test('inherits "port" if given', () => { port: 4002, path: '/', } - const url = getUrlByRequestOptions(options) + const url = getUrlByRequestOptions('http', options) expect(url).toBeInstanceOf(URL) expect(url).toHaveProperty('port', '4002') @@ -94,7 +101,7 @@ test('inherits "username" and "password"', () => { path: '/user', auth: 'admin:abc-123', } - const url = getUrlByRequestOptions(options) + const url = getUrlByRequestOptions('http', options) expect(url).toBeInstanceOf(URL) expect(url).toHaveProperty('username', 'admin') @@ -104,7 +111,7 @@ test('inherits "username" and "password"', () => { }) test('resolves hostname to localhost if none provided', () => { - const url = getUrlByRequestOptions({}) + const url = getUrlByRequestOptions('http', {}) expect(url).toBeInstanceOf(URL) expect(url).toHaveProperty('protocol', 'http:') diff --git a/src/utils/getUrlByRequestOptions.ts b/src/utils/getUrlByRequestOptions.ts index 71c3d496b..5fa75180c 100644 --- a/src/utils/getUrlByRequestOptions.ts +++ b/src/utils/getUrlByRequestOptions.ts @@ -7,9 +7,7 @@ const debug = require('debug')('utils getUrlByRequestOptions') type IsomorphicRequestOptions = RequestOptions & RequestSelf export const DEFAULT_PATH = '/' -const DEFAULT_PROTOCOL = 'http:' const DEFAULT_HOST = 'localhost' -const DEFAULT_PORT = 80 function getAgent( options: IsomorphicRequestOptions @@ -18,6 +16,7 @@ function getAgent( } function getProtocolByRequestOptions( + defaultProtocol: string, options: IsomorphicRequestOptions ): string { if (options.protocol) { @@ -31,7 +30,7 @@ function getProtocolByRequestOptions( return agentProtocol } - return options.cert ? 'https:' : options.uri?.protocol || DEFAULT_PROTOCOL + return options.cert ? 'https:' : options.uri?.protocol || `${defaultProtocol}:` } function getPortByRequestOptions( @@ -44,7 +43,7 @@ function getPortByRequestOptions( const optionsPort = options.port if (optionsPort || agentPort) { - const explicitPort = optionsPort || agentPort || DEFAULT_PORT + const explicitPort = optionsPort || agentPort return Number(explicitPort) } } @@ -63,10 +62,10 @@ function getAuthByRequestOptions(options: IsomorphicRequestOptions) { /** * Creates a `URL` instance from a given `RequestOptions` object. */ -export function getUrlByRequestOptions(options: IsomorphicRequestOptions): URL { +export function getUrlByRequestOptions(defaultProtocol: string, options: IsomorphicRequestOptions): URL { debug('request options', options) - const protocol = getProtocolByRequestOptions(options) + const protocol = getProtocolByRequestOptions(defaultProtocol, options) const host = getHostByRequestOptions(options) const port = getPortByRequestOptions(options) const path = options.path || DEFAULT_PATH diff --git a/test/response/https.test.ts b/test/response/https.test.ts index 681feeb65..749430173 100644 --- a/test/response/https.test.ts +++ b/test/response/https.test.ts @@ -63,23 +63,31 @@ test('bypasses an HTTPS request issued by "https.request" not handled in the mid }) test('Correctly handles an HTTPS request issued by "https.request" from options without protocol', async () => { - await new Promise((resolve, reject) => { - const rejectError = (error: Error) => { - reject(error) - } - - const req = request({host: server.getHttpsServerHost(), path: '/get'}, (res) => { - res.on('error', rejectError) - - res.on('end', () => { - resolve() + try { + await new Promise((resolve, reject) => { + const rejectError = (error: Error) => { + reject(error) + } + + const req = request({ + host: server.getHttpsServerHostName(), + port: server.getHttpsServerPort(), + path: '/get' + }, (res) => { + res.on('error', rejectError) + + res.on('end', () => { + resolve() + }) }) + + req.on('error', rejectError) + + req.end() }) - - req.on('error', rejectError) - - req.end() - }) + } catch (error) { + expect(error.message).toMatch(/certificate/i) + } //If we get here then node was happy with the protocol }) diff --git a/test/utils/createServer.ts b/test/utils/createServer.ts index f4132c913..e08aadfbf 100644 --- a/test/utils/createServer.ts +++ b/test/utils/createServer.ts @@ -12,7 +12,8 @@ const SSL_CERT = fs.readFileSync(path.join(__dirname, 'server.cert')) type MakeUrlFunc = (path: string) => string export interface ServerAPI { - getHttpsServerHost(): string + getHttpsServerHostName(): string + getHttpsServerPort(): number getHttpAddress(): string getHttpsAddress(): string makeHttpUrl: MakeUrlFunc @@ -50,15 +51,19 @@ function getServerAddress(server: http.Server | https.Server): () => string { return () => { const protocol = server.hasOwnProperty('key') ? 'https:' : 'http:' - return new URL(`${protocol}//${getServerHost(server)()}`).href + return new URL(`${protocol}//${getServerHostName()()}:${getServerPort(server)()}`).href } } -function getServerHost(server: http.Server | https.Server): () => string { +function getServerHostName(): () => string { return () => { - const { port } = server.address() as AddressInfo + return 'localhost' + } +} - return `localhost:${port}` +function getServerPort(server: http.Server | https.Server): () => number { + return () => { + return (server.address() as AddressInfo).port } } @@ -101,7 +106,8 @@ export async function createServer( ) return { - getHttpsServerHost: getServerHost(httpsServer), + getHttpsServerHostName: getServerHostName(), + getHttpsServerPort: getServerPort(httpsServer), getHttpAddress: getServerAddress(httpServer), getHttpsAddress: getServerAddress(httpsServer), makeHttpUrl: makeServerUrl(httpServer), From d5219ce2acc1eb11190b7c2e9e21caa14a07870a Mon Sep 17 00:00:00 2001 From: Sean Hernon Date: Fri, 30 Oct 2020 13:07:54 +0000 Subject: [PATCH 3/3] Fix comment --- test/response/https.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/response/https.test.ts b/test/response/https.test.ts index 749430173..f79c9c41c 100644 --- a/test/response/https.test.ts +++ b/test/response/https.test.ts @@ -86,10 +86,9 @@ test('Correctly handles an HTTPS request issued by "https.request" from options req.end() }) } catch (error) { + // If we get a message about a bad certificate, then node was happy with the protocol expect(error.message).toMatch(/certificate/i) } - - //If we get here then node was happy with the protocol }) test('responds to an HTTPS request issued by "https.get" and handled in the middleware', async () => {