Skip to content

Commit

Permalink
fix(ClientRequest): use urlToHttpOptions from node:url
Browse files Browse the repository at this point in the history
  • Loading branch information
kettanaito committed Aug 13, 2024
1 parent 0c0a4f1 commit 22472b3
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 62 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -186,16 +186,16 @@ it('handles [URL, RequestOptions, callback] input', () => {
expect(url.href).toEqual('https://mswjs.io/resource')

// Options must be preserved.
expect(options).toEqual<RequestOptions>({
// `urlToHttpOptions` from `node:url` generates additional
// ClientRequest options, some of which are not legally allowed.
expect(options).toMatchObject<RequestOptions>({
agent: false,
_defaultAgent: httpsGlobalAgent,
protocol: url.protocol,
method: 'GET',
headers: {
'Content-Type': 'text/plain',
},

host: url.host,
hostname: url.hostname,
path: url.pathname,
})
Expand All @@ -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',
})
})
Expand All @@ -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<RequestOptions>({
hostname: 'host-from-options.com',
path: '/path-from-options',
port: 1234,
})
Expand All @@ -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<RequestOptions>({
hostname: 'example.com',
path: '/path-from-options?foo=bar&baz=xyz',
})
})
Expand Down Expand Up @@ -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)
})
Expand All @@ -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')
})
Expand All @@ -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)
})
24 changes: 12 additions & 12 deletions src/interceptors/ClientRequest/utils/normalizeClientRequestArgs.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { urlToHttpOptions } from 'node:url'
import {
Agent as HttpAgent,
globalAgent as httpGlobalAgent,
Expand All @@ -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,
Expand All @@ -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)

Expand Down Expand Up @@ -103,7 +103,7 @@ function resolveCallback(
export type NormalizedClientRequestArgs = [
url: URL,
options: ResolvedRequestOptions,
callback?: HttpRequestCallback,
callback?: HttpRequestCallback
]

/**
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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",
Expand Down
29 changes: 0 additions & 29 deletions src/utils/getRequestOptionsByUrl.ts

This file was deleted.

14 changes: 7 additions & 7 deletions test/helpers.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -24,7 +24,7 @@ export function httpGet(
const parsedUrl = new URL(url)
const resolvedOptions = Object.assign(
{},
getRequestOptionsByUrl(parsedUrl),
urlToHttpOptions(parsedUrl),
options
)

Expand All @@ -46,7 +46,7 @@ export function httpsGet(
const parsedUrl = new URL(url)
const resolvedOptions = Object.assign(
{},
getRequestOptionsByUrl(parsedUrl),
urlToHttpOptions(parsedUrl),
options
)

Expand All @@ -69,7 +69,7 @@ export function httpRequest(
const parsedUrl = new URL(url)
const resolvedOptions = Object.assign(
{},
getRequestOptionsByUrl(parsedUrl),
urlToHttpOptions(parsedUrl),
options
)

Expand All @@ -96,7 +96,7 @@ export function httpsRequest(
const parsedUrl = new URL(url)
const resolvedOptions = Object.assign(
{},
getRequestOptionsByUrl(parsedUrl),
urlToHttpOptions(parsedUrl),
options
)

Expand Down

0 comments on commit 22472b3

Please sign in to comment.