ProxyAgent: "Cannot read properties of undefined (reading 'origin')" in Agent#closeClientIfUnused for HTTP targets (regression in 8.7.0)

[tonynelson19]

Bug Description

When using ProxyAgent to make plain-HTTP requests through a proxy (without proxyTunnel), undici crashes with:

TypeError: Cannot read properties of undefined (reading 'origin')
    at closeClientIfUnused (.../undici/lib/dispatcher/agent.js:111)

The crash occurs in Agent's closeClientIfUnused, which runs on the disconnect / connectionError teardown path. It reads client[kUrl].origin (and dispatcher[kUrl].origin) for every dispatcher stored in kClients:

// lib/dispatcher/agent.js
let hasOrigin = false;
for (const client of this[kClients].values()) {
  if (client[kUrl].origin === dispatcher[kUrl].origin) { // <-- crashes here
    hasOrigin = true;
    break;
  }
}

if (!hasOrigin) {
  this[kOrigins].delete(dispatcher[kUrl].origin); // <-- ...or here
}

ProxyAgent's internal factory stores an Http1ProxyWrapper (and Socks5ProxyAgent) in the inner Agent's kClients map. These extend DispatcherBase and never set the internal kUrl symbol, so client[kUrl] is undefined and reading .origin throws.

The unsafe kUrl access was introduced in v8.2.0, and a proxy-agent.js change in v8.7.0 made the Http1ProxyWrapper path the default for HTTP-over-HTTP-proxy requests, which is what now triggers it.

Reproduction

Standalone reproduction script:

import { once } from 'node:events';
import { createServer } from 'node:http';
import { createRequire } from 'node:module';
import { test } from 'node:test';

import { ProxyAgent, request } from 'undici';

const require = createRequire(import.meta.url);
const { kClients, kUrl } = require('undici/lib/core/symbols.js');

test(
  'ProxyAgent (non-tunnel, http target) crashes Agent#closeClientIfUnused',
  { timeout: 60000 },
  async (t) => {
    t.plan(3);

    // A minimal forward proxy (non-CONNECT) that returns a valid response.
    const proxy = createServer((req, res) => res.end('ok'));
    proxy.on('connection', (socket) => socket.on('error', () => {}));
    proxy.listen(0, '127.0.0.1');
    await once(proxy, 'listening');
    const proxyPort = proxy.address().port;

    const target = createServer((req, res) => res.end('ok'));
    target.listen(0, '127.0.0.1');
    await once(target, 'listening');
    const targetPort = target.address().port;

    // No proxyTunnel option -> http: targets are forwarded via Http1ProxyWrapper.
    const dispatcher = new ProxyAgent({ uri: `http://127.0.0.1:${proxyPort}` });

    t.after(async () => {
      await dispatcher.close().catch(() => {});
      proxy.close();
      target.close();
    });

    // Perform a real request so a genuine wrapper is registered in kClients.
    const res = await request(`http://127.0.0.1:${targetPort}/`, {
      dispatcher,
    });
    await res.body.text();

    // Locate the inner Agent's registered wrapper.
    let wrapper;
    for (const sym of Object.getOwnPropertySymbols(dispatcher)) {
      const value = dispatcher[sym];
      if (value && value[kClients] instanceof Map && value[kClients].size > 0) {
        wrapper = [...value[kClients].values()][0];
        break;
      }
    }

    t.assert.equal(wrapper.constructor.name, 'Http1ProxyWrapper');
    t.assert.equal(
      wrapper[kUrl],
      undefined,
      'Http1ProxyWrapper is expected to lack the kUrl symbol'
    );

    // Emitting the exact event undici emits on connection teardown runs
    // Agent#closeClientIfUnused, which reads `client[kUrl].origin`. On a buggy
    // undici this throws "Cannot read properties of undefined (reading 'origin')".
    t.assert.throws(() => {
      wrapper.emit('connectionError', 'http://x', [wrapper], new Error('boom'));
    }, /Cannot read properties of undefined \(reading 'origin'\)/);
  }
);

Steps to reproduce (if not using the script above):

1. Save the script above as test/repro-proxy-origin.mjs.
2. Run node --test test/repro-proxy-origin.mjs.
3. The test passes, asserting that the connectionError teardown throws Cannot read properties of undefined (reading 'origin') from Agent#closeClientIfUnused (i.e. the bug is present).

Expected Behavior

closeClientIfUnused should not throw for dispatchers that lack kUrl. An http: request through a ProxyAgent should complete and tear down cleanly.

Possible fixes:

• Guard the access, e.g. dispatcher[kUrl]?.origin / client[kUrl]?.origin, and early-return when dispatcher[kUrl] is undefined
• Have Http1ProxyWrapper / Socks5ProxyAgent expose a kUrl
• Track origins by the kClients map key (as in v8.1.0) so the bookkeeping no longer depends on kUrl.

Actual Behavior

The inner Agent stores a kUrl-less Http1ProxyWrapper in kClients. On connection teardown (disconnect / connectionError), closeClientIfUnused reads client[kUrl].origin and throws:

TypeError: Cannot read properties of undefined (reading 'origin')
    at closeClientIfUnused (.../undici/lib/dispatcher/agent.js:111)

Logs & Screenshots

Running the reproduction script on undici 8.7.0 / Node v22.23.1:

registered client: Http1ProxyWrapper
client has kUrl:   false

TypeError: Cannot read properties of undefined (reading 'origin')
    at closeClientIfUnused (.../node_modules/undici/lib/dispatcher/agent.js:118:50)

(The reported line is 111 or 118 depending on which kUrl access runs first for a given kClients contents; both are the same root cause.)

Environment

• Node.js version: v22.23.1
• undici version: 8.7.0

Additional context

Two independent changes combine to produce this crash.

1. The unsafe kUrl access was introduced in v8.2.0. In v8.1.0, kClients stored wrapper objects { count, dispatcher, origin } and the cleanup loop compared a plain entry.origin property (always present). In v8.2.0 the map was refactored to store bare dispatcher instances, and the loop was rewritten to reach through the internal kUrl symbol:

- this[kClients].set(key, { count: 0, dispatcher, origin })
+ this[kClients].set(key, dispatcher)

- if (entry.origin === origin) {
+ if (client[kUrl].origin === dispatcher[kUrl].origin) {

Bisection of lib/dispatcher/agent.js across release tags:

Version	closeClientIfUnused	client[kUrl].origin loop	Crash possible?
v7.16.0	yes (added with maxOrigins)	no	No
v8.0.0	yes	no	No
v8.1.0	yes (adds hasOrigin, uses entry.origin)	no	No
v8.2.0	yes	yes (introduced)	Yes
v8.3.0 – v8.7.0	yes	yes	Yes

2. v8.7.0 started exercising that path for HTTP targets.
lib/dispatcher/agent.js is byte-identical between 8.6.0 and 8.7.0, but lib/dispatcher/proxy-agent.js changed so that HTTP-over-HTTP-proxy requests are now forwarded via Http1ProxyWrapper instead of being tunneled:

• The proxyTunnel default changed from true to undefined.
• The gate for using Http1ProxyWrapper was rewritten via a new shouldProxyTunnel(requestProtocol, proxyTunnel):

function shouldProxyTunnel(requestProtocol, proxyTunnel) {
  return proxyTunnel === true || requestProtocol !== 'http:';
}
// ...
if (!shouldProxyTunnel(protocol, this[kTunnelProxy])) {
  return new Http1ProxyWrapper(/* ... */); // kUrl-less wrapper
}

For a default new ProxyAgent({ uri }) (so proxyTunnel is unset) and an http: target, shouldProxyTunnel('http:', undefined) === false, so the wrapper is used. In 8.6.0 the same setup had proxyTunnel = true, so the wrapper was never created and requests went through a real Pool/Client (which sets kUrl).

Passing proxyTunnel: true to ProxyAgent restores the pre-8.7.0 routing (all requests tunneled via CONNECT through real Client/Pool instances that set kUrl), which avoids the crash:

new ProxyAgent({ uri, proxyTunnel: true });

Notes on the reproduction.

• The request itself uses only the public API (ProxyAgent + request), which registers the real, kUrl-less Http1ProxyWrapper in the inner Agent's kClients map.
• Http1ProxyWrapper delegates dispatch to a private #client and does not re-emit its inner client's disconnect / connectionError, so organically timing the teardown is flaky. The wrapper.emit('connectionError', ...) line reproduces the exact event undici's own machinery fires on teardown, invoking closeClientIfUnused synchronously and deterministically.