fix: More strictly, determine when new URL() should be used

src/request.ts

@@ -270,6 +270,69 @@ Object.defineProperty(requestPrototype, 'blob', {
 })
 Object.setPrototypeOf(requestPrototype, Request.prototype)
 
+const isPathDelimiter = (charCode: number): boolean =>
+  charCode === 0x2f || charCode === 0x3f || charCode === 0x23
+
+// `/.`, `/..` (including `%2e` variants, which are handled by `%` detection) are normalized by `new URL()`.
+const hasDotSegment = (url: string, dotIndex: number): boolean => {
+  const prev = dotIndex === 0 ? 0x2f : url.charCodeAt(dotIndex - 1)
+  if (prev !== 0x2f) {
+    return false
+  }
+
+  const nextIndex = dotIndex + 1
+  if (nextIndex === url.length) {
+    return true
+  }
+
+  const next = url.charCodeAt(nextIndex)
+  if (isPathDelimiter(next)) {
+    return true
+  }
+  if (next !== 0x2e) {
+    return false
+  }
+
+  const nextNextIndex = dotIndex + 2
+  if (nextNextIndex === url.length) {
+    return true
+  }
+  return isPathDelimiter(url.charCodeAt(nextNextIndex))
+}
+
+const allowedRequestUrlChar = new Uint8Array(128)
+for (let c = 0x30; c <= 0x39; c++) {
+  allowedRequestUrlChar[c] = 1
+}
+for (let c = 0x41; c <= 0x5a; c++) {
+  allowedRequestUrlChar[c] = 1
+}
+for (let c = 0x61; c <= 0x7a; c++) {
+  allowedRequestUrlChar[c] = 1
+}
+;(() => {
+  const chars = '-./:?#[]@!$&\'()*+,;=~_'
+  for (let i = 0; i < chars.length; i++) {
+    allowedRequestUrlChar[chars.charCodeAt(i)] = 1
+  }
+})()
+
+const safeHostChar = new Uint8Array(128)
+// 0-9
+for (let c = 0x30; c <= 0x39; c++) {
+  safeHostChar[c] = 1
+}
+// a-z
+for (let c = 0x61; c <= 0x7a; c++) {
+  safeHostChar[c] = 1
+}
+;(() => {
+  const chars = '.-_'
+  for (let i = 0; i < chars.length; i++) {
+    safeHostChar[chars.charCodeAt(i)] = 1
+  }
+})()
+
 export const newRequest = (
   incoming: IncomingMessage | Http2ServerRequest,
   defaultHostname?: string
@@ -316,26 +379,55 @@ export const newRequest = (
     scheme = incoming.socket && (incoming.socket as TLSSocket).encrypted ? 'https' : 'http'
   }
 
-  // Fast path: avoid new URL() allocation for common requests.
-  // Fall back to new URL() only when path normalization is needed (e.g. `..` sequences).
-  if (incomingUrl.indexOf('..') === -1) {
-    // Validate host header doesn't contain URL-breaking characters
-    for (let i = 0; i < host.length; i++) {
-      const c = host.charCodeAt(i)
-      if (c < 0x21 || c === 0x2f || c === 0x23 || c === 0x3f || c === 0x40 || c === 0x5c) {
-        // reject control chars, space, / # ? @ \
-        throw new RequestError('Invalid host header')
-      }
+  req[urlKey] = `${scheme}://${host}${incomingUrl}`
+  let needsHostValidationByURL = false
+  for (let i = 0, len = host.length; i < len; i++) {
+    const c = host.charCodeAt(i)
+    if (c > 0x7f || safeHostChar[c] === 0) {
+      needsHostValidationByURL = true
+      break
     }
-    req[urlKey] = `${scheme}://${host}${incomingUrl}`
-  } else {
-    const url = new URL(`${scheme}://${host}${incomingUrl}`)
-    // check by length for performance.
+  }
+
+  if (needsHostValidationByURL) {
+    let urlObj: URL
+    try {
+      urlObj = new URL(req[urlKey])
+    } catch (e) {
+      throw new RequestError('Invalid URL', { cause: e })
+    }
+
     // if suspicious, check by host. host header sometimes contains port.
-    if (url.hostname.length !== host.length && url.hostname !== host.replace(/:\d+$/, '')) {
+    if (
+      urlObj.hostname.length !== host.length &&
+      urlObj.hostname !== (host.includes(':') ? host.replace(/:\d+$/, '') : host).toLowerCase()
+    ) {
       throw new RequestError('Invalid host header')
     }
-    req[urlKey] = url.href
+    req[urlKey] = urlObj.href
+  } else if (incomingUrl.length === 0) {
+    req[urlKey] += '/'
+  } else {
+    if (incomingUrl.charCodeAt(0) !== 0x2f) {
+      // '/'
+      throw new RequestError('Invalid URL')
+    }
+
+    for (let i = 1, len = incomingUrl.length; i < len; i++) {
+      const c = incomingUrl.charCodeAt(i)
+      if (
+        c > 0x7f ||
+        allowedRequestUrlChar[c] === 0 ||
+        (c === 0x2e && hasDotSegment(incomingUrl, i))
+      ) {
+        try {
+          req[urlKey] = new URL(req[urlKey]).href
+        } catch (e) {
+          throw new RequestError('Invalid URL', { cause: e })
+        }
+        break
+      }
+    }
   }
 
   return req

test/request.test.ts

@@ -154,6 +154,17 @@ describe('Request', () => {
       }).toThrow(RequestError)
     })
 
+    it('Should throw error if host header port is invalid', async () => {
+      expect(() => {
+        newRequest({
+          headers: {
+            host: 'localhost:65536',
+          },
+          url: '/foo.txt',
+        } as IncomingMessage)
+      }).toThrow(RequestError)
+    })
+
     it('Should be created request body from `req.rawBody` if it exists', async () => {
       const rawBody = Buffer.from('foo')
       const socket = new Socket()
@@ -177,6 +188,87 @@ describe('Request', () => {
       expect(text).toBe('foo')
     })
 
+    describe('IPv6 host', () => {
+      it('Should throw error for unmatched closing bracket in host', async () => {
+        expect(() => {
+          newRequest({
+            headers: {
+              host: 'host]',
+            },
+            url: '/foo.txt',
+          } as IncomingMessage)
+        }).toThrow(RequestError)
+      })
+
+      it('Should throw error for unmatched opening bracket in host', async () => {
+        expect(() => {
+          newRequest({
+            headers: {
+              host: '[host',
+            },
+            url: '/foo.txt',
+          } as IncomingMessage)
+        }).toThrow(RequestError)
+      })
+    })
+
+    describe('URL normalization', () => {
+      test.each([
+        ['[::1]', '/foo.txt'],
+        ['[::1]:8080', '/foo.txt'],
+        ['localhost', '/'],
+        ['localhost', '/foo/bar/baz'],
+        ['localhost', '/foo_bar'],
+        ['localhost', '/foo//bar'],
+        ['localhost', '/static/%2e%2e/foo.txt'],
+        ['localhost', '/static\\..\\foo.txt'],
+        ['localhost', '/..'],
+        ['localhost', '/foo/.'],
+        ['localhost', '/foo/bar/..'],
+        ['localhost', '/a/b/../../c'],
+        ['localhost', '/a/../../../b'],
+        ['localhost', '/a/b/c/../../../'],
+        ['localhost', '/./foo.txt'],
+        ['localhost', '/foo/../bar.txt'],
+        ['localhost', '/a/./b/../c?q=%2E%2E#hash'],
+        ['localhost', '/foo/%2E/bar/../baz'],
+        ['localhost', '/hello%20world'],
+        ['localhost', '/foo%23bar'],
+        ['localhost', '/foo"bar'],
+        ['localhost', '/%2e%2E/foo'],
+        ['localhost', '/caf%C3%A9'],
+        ['localhost', '/foo%2fbar/..//baz'],
+        ['localhost', '/foo?q=../bar'],
+        ['localhost', '/path?q=hello%20world'],
+        ['localhost', '/file.txt'],
+        ['localhost', ''],
+        ['LOCALHOST', '/foo.txt'],
+        ['LOCALHOST:80', '/foo.txt'],
+        ['LOCALHOST:443', '/foo.txt'],
+        ['LOCALHOST:8080', '/foo.txt'],
+        ['Localhost:3000', '/foo.txt'],
+      ])('Should normalize %s to %s', async (host, url) => {
+        const req = newRequest({
+          headers: {
+            host,
+          },
+          url,
+        } as IncomingMessage)
+        expect(req.url).toBe(new URL(url, `http://${host}`).toString())
+      })
+
+      it('Should throw error for non-origin-form request-target', async () => {
+        expect(() => {
+          newRequest({
+            headers: {
+              host: 'localhost',
+            },
+            url: '*',
+          } as IncomingMessage)
+        }).toThrow(RequestError)
+      })
+    })
+
     describe('absolute-form for request-target', () => {
       it('should be created from valid absolute URL', async () => {
         const req = newRequest({