Support legacy url correct default protocol

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<typeof http.request>
   ) {
-    const [url, options, callback] = normalizeHttpRequestParams(...args)
+    const [url, options, callback] = normalizeHttpRequestParams(protocol, ...args)
     const usesHttps = url.protocol === 'https:'
     let requestBodyBuffer: Buffer[] = []
 

src/interceptors/ClientRequest/index.ts

@@ -31,6 +31,7 @@ function handleRequest(
   }
 
   const ClientRequestOverride = createClientRequestOverrideClass(
+    protocol,
     middleware,
     originalMethod,
     originalClientRequest

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() {}
   )

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,22 +88,22 @@ 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.
   else if (isObject(args[0])) {
     options = args[0]
     debug('given request options:', options)
 
-    url = getUrlByRequestOptions(options)
+    url = getUrlByRequestOptions(defaultProtocol, options)
     debug('created a URL:', url)
 
     callback = resolveCallback(args)

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,20 +37,27 @@ 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:')
   expect(url).toHaveProperty('port', '3080')
   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: '<!-- SSL certificate -->',
   }
-  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:')

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

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,35 @@ 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 () => {
+  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()
+    })
+  } catch (error) {
+    // If we get a message about a bad certificate, then node was happy with the protocol
+    expect(error.message).toMatch(/certificate/i)
+  }
+})
+
 test('responds to an HTTPS request issued by "https.get" and handled in the middleware', async () => {
   const { res, resBody } = await httpsGet(server.makeHttpsUrl('/'))
 

test/utils/createServer.ts

@@ -12,6 +12,8 @@ const SSL_CERT = fs.readFileSync(path.join(__dirname, 'server.cert'))
 type MakeUrlFunc = (path: string) => string
 
 export interface ServerAPI {
+  getHttpsServerHostName(): string
+  getHttpsServerPort(): number
   getHttpAddress(): string
   getHttpsAddress(): string
   makeHttpUrl: MakeUrlFunc
@@ -48,9 +50,20 @@ function closeServer(server: http.Server) {
 function getServerAddress(server: http.Server | https.Server): () => string {
   return () => {
     const protocol = server.hasOwnProperty('key') ? 'https:' : 'http:'
-    const { port } = server.address() as AddressInfo
 
-    return new URL(`${protocol}//localhost:${port}`).href
+    return new URL(`${protocol}//${getServerHostName()()}:${getServerPort(server)()}`).href
+  }
+}
+
+function getServerHostName(): () => string {
+  return () => {
+    return 'localhost'
+  }
+}
+
+function getServerPort(server: http.Server | https.Server): () => number {
+  return () => {
+    return (server.address() as AddressInfo).port
   }
 }
 
@@ -93,6 +106,8 @@ export async function createServer(
   )
 
   return {
+    getHttpsServerHostName: getServerHostName(),
+    getHttpsServerPort: getServerPort(httpsServer),
     getHttpAddress: getServerAddress(httpServer),
     getHttpsAddress: getServerAddress(httpsServer),
     makeHttpUrl: makeServerUrl(httpServer),