fix(fetch): preserve path for credentialed URLs (#4892)

Author: mcollina

lib/web/fetch/index.js

@@ -2132,9 +2132,12 @@ async function httpNetworkFetch (
     /** @type {import('../../..').Agent} */
     const agent = fetchParams.controller.dispatcher
 
+    const path = url.pathname + url.search
+    const hasTrailingQuestionMark = url.search.length === 0 && url.href[url.href.length - url.hash.length - 1] === '?'
+
     return new Promise((resolve, reject) => agent.dispatch(
       {
-        path: url.href.slice(url.href.indexOf(url.host) + url.host.length, url.hash.length ? -url.hash.length : undefined),
+        path: hasTrailingQuestionMark ? `${path}?` : path,
         origin: url.origin,
         method: request.method,
         body: agent.isMockActive ? request.body && (request.body.source || request.body.stream) : body,

test/fetch/issue-4897.js

@@ -0,0 +1,36 @@
+'use strict'
+
+const { test } = require('node:test')
+const { fetch } = require('../..')
+
+function createAssertingDispatcher (t, expectedPath) {
+  return {
+    dispatch (opts, handler) {
+      t.assert.strictEqual(opts.path, expectedPath)
+      handler.onError(new Error('stop'))
+      return true
+    }
+  }
+}
+
+async function assertPath (t, url, expectedPath) {
+  const dispatcher = createAssertingDispatcher(t, expectedPath)
+
+  await t.assert.rejects(fetch(url, { dispatcher }), (err) => {
+    t.assert.strictEqual(err.cause?.message, 'stop')
+    return true
+  })
+}
+
+// https://github.com/nodejs/undici/issues/4897
+test('fetch path extraction does not match hostnames inside scheme', async (t) => {
+  const hosts = ['h', 't', 'p', 'ht', 'tp', 'tt']
+
+  for (const scheme of ['http', 'https']) {
+    for (const host of hosts) {
+      await t.test(`${scheme}://${host}/test?a=b#frag`, async (t) => {
+        await assertPath(t, `${scheme}://${host}/test?a=b#frag`, '/test?a=b')
+      })
+    }
+  }
+})

test/websocket/issue-4889.js

@@ -1,15 +1,37 @@
+'use strict'
+
 const { test } = require('node:test')
-const http = require('node:http')
-const crypto = require('node:crypto')
+const { once } = require('node:events')
+const { createServer } = require('node:http')
+const { WebSocketServer } = require('ws')
+
 const { WebSocket } = require('../..')
-const { createDeferredPromise } = require('../../lib/util/promise')
+const { closeServerAsPromise } = require('../utils/node-http')
+
+// https://github.com/nodejs/undici/issues/4889
+test('websocket auth retry preserves path when URL contains credentials', async (t) => {
+  const expectedAuth = `Basic ${Buffer.from('user:pass').toString('base64')}`
+
+  let attempts = 0
+
+  const server = createServer()
+  const wss = new WebSocketServer({ noServer: true })
+
+  t.after(async () => {
+    for (const client of wss.clients) {
+      client.terminate()
+    }
+
+    await new Promise((resolve) => wss.close(resolve))
+    await closeServerAsPromise(server)()
+  })
 
-test('WebSocket basic auth', (t) => {
-  const server = http.createServer()
+  server.on('upgrade', (req, socket, head) => {
+    attempts++
+    t.assert.strictEqual(req.url, '/path')
 
-  server.on('upgrade', (req, socket) => {
-    const auth = req.headers.authorization
-    if (!auth || auth !== `Basic ${Buffer.from('user:pass').toString('base64')}`) {
+    if (attempts === 1) {
+      t.assert.strictEqual(req.headers.authorization, undefined)
       socket.write(
         'HTTP/1.1 401 Unauthorized\r\n' +
         'WWW-Authenticate: Basic realm="test"\r\n' +
@@ -20,43 +42,26 @@ test('WebSocket basic auth', (t) => {
       return
     }
 
-    const key = req.headers['sec-websocket-key']
-    const accept = crypto
-      .createHash('sha1')
-      .update(key + '258EAFA5-E914-47DA-95CA-C5AB0DC85B11')
-      .digest('base64')
-
-    socket.write(
-      'HTTP/1.1 101 Switching Protocols\r\n' +
-      'Upgrade: websocket\r\n' +
-      'Connection: Upgrade\r\n' +
-      'Sec-WebSocket-Accept: ' + accept + '\r\n' +
-      '\r\n'
-    )
-
-    socket.on('data', () => socket.destroy())
-  }).listen(0)
-
-  const { port } = server.address()
-  const url = `ws://user:pass@127.0.0.1:${port}/path`
-
-  const ws = new WebSocket(url)
+    if (attempts === 2) {
+      t.assert.strictEqual(req.headers.authorization, expectedAuth)
+      wss.handleUpgrade(req, socket, head, (websocket) => {
+        wss.emit('connection', websocket, req)
+      })
+      return
+    }
 
-  t.after(() => {
-    ws.close()
-    server.close()
+    t.assert.fail(`unexpected upgrade attempt #${attempts}`)
   })
 
-  const promise = createDeferredPromise()
+  server.listen(0, '127.0.0.1')
+  await once(server, 'listening')
 
-  ws.addEventListener('open', () => {
-    promise.resolve()
-    ws.send('h')
-  })
+  const ws = new WebSocket(`ws://user:pass@127.0.0.1:${server.address().port}/path`)
 
-  ws.addEventListener('error', (e) => {
-    promise.reject(e)
+  await new Promise((resolve, reject) => {
+    ws.addEventListener('open', resolve, { once: true })
+    ws.addEventListener('error', ({ error }) => reject(error), { once: true })
   })
 
-  return promise.promise
+  t.assert.strictEqual(attempts, 2)
 })