fix: support IPv6 during ClientRequest options parsing

src/utils/getUrlByRequestOptions.test.ts

@@ -105,10 +105,15 @@ it('resolves hostname to localhost if none provided', () => {
   expect(getUrlByRequestOptions({}).hostname).toBe('localhost')
 })
 
-it('supports "hostname" instead of "host" and "port"', () => {
+it('resolves host to localhost if none provided', () => {
+  expect(getUrlByRequestOptions({}).host).toBe('localhost')
+})
+
+it('supports "hostname" and "port"', () => {
   const options: RequestOptions = {
     protocol: 'https:',
-    hostname: '127.0.0.1:1234',
+    hostname: '127.0.0.1',
+    port: 1234,
     path: '/resource',
   }
 
@@ -117,14 +122,37 @@ it('supports "hostname" instead of "host" and "port"', () => {
   )
 })
 
-it('handles IPv6 hostnames', () => {
+it('use "hostname" if both "hostname" and "host" are specified', () => {
+  const options: RequestOptions = {
+    protocol: 'https:',
+    host: 'host',
+    hostname: 'hostname',
+    path: '/resource',
+  }
+
+  expect(getUrlByRequestOptions(options).href).toBe(
+    'https://hostname/resource'
+  )
+})
+
+it('parses "host" in IPv6', () => {
   expect(
     getUrlByRequestOptions({
       host: '::1',
       path: '/resource',
     }).href
   ).toBe('http://[::1]/resource')
 
+  expect(
+    getUrlByRequestOptions({
+      host: '[::1]',
+      path: '/resource',
+    }).href
+  ).toBe('http://[::1]/resource')
+
+})
+
+it('parses "host" and "port" in IPv6', () => {
   expect(
     getUrlByRequestOptions({
       host: '::1',

src/utils/getUrlByRequestOptions.ts

@@ -15,7 +15,7 @@ export type ResolvedRequestOptions = RequestOptions & RequestSelf
 
 export const DEFAULT_PATH = '/'
 const DEFAULT_PROTOCOL = 'http:'
-const DEFAULT_HOST = 'localhost'
+const DEFAULT_HOSTNAME = 'localhost'
 const SSL_PORT = 443
 
 function getAgent(
@@ -50,15 +50,6 @@ function getPortByRequestOptions(
     return Number(options.port)
   }
 
-  // Extract the port from the hostname.
-  if (options.hostname != null) {
-    const [, extractedPort] = options.hostname.match(/:(\d+)$/) || []
-
-    if (extractedPort != null) {
-      return Number(extractedPort)
-    }
-  }
-
   // Otherwise, try to resolve port from the agent.
   const agent = getAgent(options)
 
@@ -75,17 +66,6 @@ function getPortByRequestOptions(
   return undefined
 }
 
-function getHostByRequestOptions(options: ResolvedRequestOptions): string {
-  const { hostname, host } = options
-
-  // If the hostname is specified, resolve the host from the "host:port" string.
-  if (hostname != null) {
-    return hostname.replace(/:\d+$/, '')
-  }
-
-  return host || DEFAULT_HOST
-}
-
 interface RequestAuth {
   username: string
   password: string
@@ -109,22 +89,20 @@ function isRawIPv6Address(host: string): boolean {
   return host.includes(':') && !host.startsWith('[') && !host.endsWith(']')
 }
 
-function getHostname(host: string, port?: number): string {
-  const portString = typeof port !== 'undefined' ? `:${port}` : ''
+function getHostname(options: ResolvedRequestOptions): string | undefined {
+  let host = options.hostname || options.host
 
-  /**
-   * @note As of Node >= 17, hosts (including "localhost") can resolve to IPv6
-   * addresses, so construct valid URL by surrounding the IPv6 host with brackets.
-   */
-  if (isRawIPv6Address(host)) {
-    return `[${host}]${portString}`
-  }
+  if (host) {
+    if (isRawIPv6Address(host)) {
+       host = `[${host}]`
+    }
 
-  if (typeof port === 'undefined') {
-    return host
+    // Check the presence of the port, and if it's present,
+    // remove it from the host, returning a hostname.
+    return new URL(`http://${host}`).hostname
   }
 
-  return `${host}${portString}`
+  return DEFAULT_HOSTNAME
 }
 
 /**
@@ -146,13 +124,10 @@ export function getUrlByRequestOptions(options: ResolvedRequestOptions): URL {
   const protocol = getProtocolByRequestOptions(options)
   logger.info('protocol', protocol)
 
-  const host = getHostByRequestOptions(options)
-  logger.info('host', host)
-
   const port = getPortByRequestOptions(options)
   logger.info('port', port)
 
-  const hostname = getHostname(host, port)
+  const hostname = getHostname(options)
   logger.info('hostname', hostname)
 
   const path = options.path || DEFAULT_PATH
@@ -166,7 +141,8 @@ export function getUrlByRequestOptions(options: ResolvedRequestOptions): URL {
     : ''
   logger.info('auth string:', authString)
 
-  const url = new URL(`${protocol}//${hostname}${path}`)
+  const portString = typeof port !== 'undefined' ? `:${port}` : ''
+  const url = new URL(`${protocol}//${hostname}${portString}${path}`)
   url.username = credentials?.username || ''
   url.password = credentials?.password || ''
 

test/modules/http/compliance/http-request-ipv6.test.ts

@@ -0,0 +1,29 @@
+import { it, expect, beforeAll, afterAll } from 'vitest'
+import { httpGet } from '../../../helpers'
+import { ClientRequestInterceptor } from '../../../../src/interceptors/ClientRequest'
+import { DeferredPromise } from '@open-draft/deferred-promise'
+
+const interceptor = new ClientRequestInterceptor()
+
+beforeAll(() => {
+  interceptor.apply()
+})
+
+afterAll(() => {
+  interceptor.dispose()
+})
+
+it('supports requests with IPv6 request url', async () => {
+  const url = 'http://[2607:f0d0:1002:51::4]:8080/'
+  const listenerUrlPromise = new DeferredPromise<string>()
+
+  interceptor.on('request', ({ request }) => {
+    listenerUrlPromise.resolve(request.url)
+    request.respondWith(new Response('test'))
+  });
+
+  const { resBody } = await httpGet(url)
+  const requestUrl = await listenerUrlPromise
+  expect(resBody).toBe('test')
+  expect(requestUrl).toBe(url)
+})