From 22472b38e7526dc1681011fb4f893a01350da89a Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Tue, 13 Aug 2024 15:53:22 +0200 Subject: [PATCH] fix(ClientRequest): use `urlToHttpOptions` from `node:url` --- .../utils/normalizeClientRequestArgs.test.ts | 26 ++++++++--------- .../utils/normalizeClientRequestArgs.ts | 24 +++++++-------- src/utils/getRequestOptionsByUrl.ts | 29 ------------------- test/helpers.ts | 14 ++++----- 4 files changed, 31 insertions(+), 62 deletions(-) delete mode 100644 src/utils/getRequestOptionsByUrl.ts diff --git a/src/interceptors/ClientRequest/utils/normalizeClientRequestArgs.test.ts b/src/interceptors/ClientRequest/utils/normalizeClientRequestArgs.test.ts index 3080c7aa..3eb551fd 100644 --- a/src/interceptors/ClientRequest/utils/normalizeClientRequestArgs.test.ts +++ b/src/interceptors/ClientRequest/utils/normalizeClientRequestArgs.test.ts @@ -186,7 +186,9 @@ it('handles [URL, RequestOptions, callback] input', () => { expect(url.href).toEqual('https://mswjs.io/resource') // Options must be preserved. - expect(options).toEqual({ + // `urlToHttpOptions` from `node:url` generates additional + // ClientRequest options, some of which are not legally allowed. + expect(options).toMatchObject({ agent: false, _defaultAgent: httpsGlobalAgent, protocol: url.protocol, @@ -194,8 +196,6 @@ it('handles [URL, RequestOptions, callback] input', () => { headers: { 'Content-Type': 'text/plain', }, - - host: url.host, hostname: url.hostname, path: url.pathname, }) @@ -214,7 +214,7 @@ it('handles [URL, RequestOptions] where options have custom "hostname"', () => { ]) expect(url.href).toBe('http://host-from-options.com/path-from-url') expect(options).toMatchObject({ - host: 'host-from-options.com', + hostname: 'host-from-options.com', path: '/path-from-url', }) }) @@ -230,8 +230,8 @@ it('handles [URL, RequestOptions] where options contain "host" and "path" and "p ]) // Must remove the query string since it's not specified in "options.path" expect(url.href).toBe('http://host-from-options.com:1234/path-from-options') - expect(options).toMatchObject({ - host: 'host-from-options.com:1234', + expect(options).toMatchObject({ + hostname: 'host-from-options.com', path: '/path-from-options', port: 1234, }) @@ -245,8 +245,8 @@ it('handles [URL, RequestOptions] where options contain "path" with query string }, ]) expect(url.href).toBe('http://example.com/path-from-options?foo=bar&baz=xyz') - expect(options).toMatchObject({ - host: 'example.com', + expect(options).toMatchObject({ + hostname: 'example.com', path: '/path-from-options?foo=bar&baz=xyz', }) }) @@ -399,7 +399,6 @@ it('merges URL-based RequestOptions with the custom RequestOptions', () => { // Other options must be inferred from the URL. expect(options.protocol).toEqual(url.protocol) - expect(options.host).toEqual(url.host) expect(options.hostname).toEqual(url.hostname) expect(options.path).toEqual(url.pathname) }) @@ -414,7 +413,6 @@ it('respects custom "options.path" over URL path', () => { expect(url.href).toBe('http://example.com/path-from-options') expect(options.protocol).toBe('http:') - expect(options.host).toBe('example.com') expect(options.hostname).toBe('example.com') expect(options.path).toBe('/path-from-options') }) @@ -430,20 +428,20 @@ it('respects custom "options.path" over URL path with query string', () => { // Must replace both the path and the query string. expect(url.href).toBe('http://example.com/path-from-options') expect(options.protocol).toBe('http:') - expect(options.host).toBe('example.com') expect(options.hostname).toBe('example.com') expect(options.path).toBe('/path-from-options') }) it('preserves URL query string', () => { const [url, options] = normalizeClientRequestArgs('http:', [ - new URL('http://example.com/resource?a=b&c=d'), + new URL('http://example.com:8080/resource?a=b&c=d'), ]) - expect(url.href).toBe('http://example.com/resource?a=b&c=d') + expect(url.href).toBe('http://example.com:8080/resource?a=b&c=d') expect(options.protocol).toBe('http:') - expect(options.host).toBe('example.com') + // expect(options.host).toBe('example.com:8080') expect(options.hostname).toBe('example.com') // Query string is a part of the options path. expect(options.path).toBe('/resource?a=b&c=d') + expect(options.port).toBe(8080) }) diff --git a/src/interceptors/ClientRequest/utils/normalizeClientRequestArgs.ts b/src/interceptors/ClientRequest/utils/normalizeClientRequestArgs.ts index b8d4c750..03c77453 100644 --- a/src/interceptors/ClientRequest/utils/normalizeClientRequestArgs.ts +++ b/src/interceptors/ClientRequest/utils/normalizeClientRequestArgs.ts @@ -1,3 +1,4 @@ +import { urlToHttpOptions } from 'node:url' import { Agent as HttpAgent, globalAgent as httpGlobalAgent, @@ -20,7 +21,6 @@ import { parse as parseUrl, } from 'node:url' import { Logger } from '@open-draft/logger' -import { getRequestOptionsByUrl } from '../../../utils/getRequestOptionsByUrl' import { ResolvedRequestOptions, getUrlByRequestOptions, @@ -47,12 +47,12 @@ function resolveRequestOptions( // without any `RequestOptions` or callback. if (typeof args[1] === 'undefined' || typeof args[1] === 'function') { logger.info('request options not provided, deriving from the url', url) - return getRequestOptionsByUrl(url) + return urlToHttpOptions(url) } if (args[1]) { logger.info('has custom RequestOptions!', args[1]) - const requestOptionsFromUrl = getRequestOptionsByUrl(url) + const requestOptionsFromUrl = urlToHttpOptions(url) logger.info('derived RequestOptions from the URL:', requestOptionsFromUrl) @@ -103,7 +103,7 @@ function resolveCallback( export type NormalizedClientRequestArgs = [ url: URL, options: ResolvedRequestOptions, - callback?: HttpRequestCallback, + callback?: HttpRequestCallback ] /** @@ -137,7 +137,7 @@ export function normalizeClientRequestArgs( url = new URL(args[0]) logger.info('created a url:', url) - const requestOptionsFromUrl = getRequestOptionsByUrl(url) + const requestOptionsFromUrl = urlToHttpOptions(url) logger.info('request options from url:', requestOptionsFromUrl) options = resolveRequestOptions(args, url) @@ -200,17 +200,17 @@ export function normalizeClientRequestArgs( return args[1] === undefined ? normalizeClientRequestArgs(defaultProtocol, [resolvedUrl]) : typeof args[1] === 'function' - ? normalizeClientRequestArgs(defaultProtocol, [resolvedUrl, args[1]]) - : normalizeClientRequestArgs(defaultProtocol, [ - resolvedUrl, - args[1], - args[2], - ]) + ? normalizeClientRequestArgs(defaultProtocol, [resolvedUrl, args[1]]) + : normalizeClientRequestArgs(defaultProtocol, [ + resolvedUrl, + args[1], + args[2], + ]) } // Handle a given "RequestOptions" object as-is // and derive the URL instance from it. else if (isObject(args[0])) { - options = { ... args[0] as any } + options = { ...(args[0] as any) } logger.info('first argument is RequestOptions:', options) // When handling a "RequestOptions" object without an explicit "protocol", diff --git a/src/utils/getRequestOptionsByUrl.ts b/src/utils/getRequestOptionsByUrl.ts deleted file mode 100644 index 33eeeaca..00000000 --- a/src/utils/getRequestOptionsByUrl.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { RequestOptions } from 'http' - -/** - * Converts a URL instance into the RequestOptions object expected by - * the `ClientRequest` class. - * @see https://github.com/nodejs/node/blob/908292cf1f551c614a733d858528ffb13fb3a524/lib/internal/url.js#L1257 - */ -export function getRequestOptionsByUrl(url: URL): RequestOptions { - const options: RequestOptions = { - method: 'GET', - protocol: url.protocol, - hostname: - typeof url.hostname === 'string' && url.hostname.startsWith('[') - ? url.hostname.slice(1, -1) - : url.hostname, - host: url.host, - path: `${url.pathname}${url.search || ''}`, - } - - if (!!url.port) { - options.port = Number(url.port) - } - - if (url.username || url.password) { - options.auth = `${url.username}:${url.password}` - } - - return options -} diff --git a/test/helpers.ts b/test/helpers.ts index 957bced8..7edaf2ed 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -1,8 +1,8 @@ -import https from 'https' -import http, { ClientRequest, IncomingMessage, RequestOptions } from 'http' +import { urlToHttpOptions } from 'node:url' +import https from 'node:https' +import http, { ClientRequest, IncomingMessage, RequestOptions } from 'node:http' import nodeFetch, { Response, RequestInfo, RequestInit } from 'node-fetch' import { Page } from '@playwright/test' -import { getRequestOptionsByUrl } from '../src/utils/getRequestOptionsByUrl' import { getIncomingMessageBody } from '../src/interceptors/ClientRequest/utils/getIncomingMessageBody' import { SerializedRequest } from '../src/RemoteHttpInterceptor' import { RequestHandler } from 'express' @@ -24,7 +24,7 @@ export function httpGet( const parsedUrl = new URL(url) const resolvedOptions = Object.assign( {}, - getRequestOptionsByUrl(parsedUrl), + urlToHttpOptions(parsedUrl), options ) @@ -46,7 +46,7 @@ export function httpsGet( const parsedUrl = new URL(url) const resolvedOptions = Object.assign( {}, - getRequestOptionsByUrl(parsedUrl), + urlToHttpOptions(parsedUrl), options ) @@ -69,7 +69,7 @@ export function httpRequest( const parsedUrl = new URL(url) const resolvedOptions = Object.assign( {}, - getRequestOptionsByUrl(parsedUrl), + urlToHttpOptions(parsedUrl), options ) @@ -96,7 +96,7 @@ export function httpsRequest( const parsedUrl = new URL(url) const resolvedOptions = Object.assign( {}, - getRequestOptionsByUrl(parsedUrl), + urlToHttpOptions(parsedUrl), options )