[BUG] proxy behavior change introduced in #59 – appears to always use HttpProxyAgent

[wlisac]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

Overview

I’ve noticed a behavior change in npm/agent v2.2.0 that shipped as part of npm/cli v10.2.0.

npm install appears to always use HttpProxyAgent instead of selecting between HttpProxyAgent and HttpsProxyAgent depending on the requested URL's protocol.

Observations

I think I've isolated the behavior change to #59.

Prior to #59, it looks like the library would select an HttpAgent or HttpsAgent based on the url.protocol of the request.

agent/lib/index.js

Lines 20 to 30 in 22ac4e5

	const secure = url.protocol === 'https:'
	const proxy = getProxy(url, { proxy: _proxy, noProxy })
	const options = { ...normalizeOptions(_options), proxy }
	return cacheAgent({
	key: cacheOptions({ ...options, secure }),
	cache: agentCache,
	secure,
	proxies: [HttpAgent, HttpsAgent],
	}, options)
	}

agent/lib/util.js

Lines 67 to 75 in 22ac4e5

	const cacheAgent = ({ key, cache, secure, proxies }, ...args) => {
	if (cache.has(key)) {
	return cache.get(key)
	}
	const Ctor = (secure ? proxies[1] : proxies[0]) ?? proxies[0]
	const agent = new Ctor(...args)
	cache.set(key, agent)
	return agent
	}

After #59, the library selects an HttpProxyAgent or HttpsProxyAgent based on an options.secureEndpoint property, but this appears to (always?) be undefined and not based on the url.protocol of the request.

agent/lib/index.js

Lines 26 to 36 in e24eb6c

	const cacheKey = cacheOptions({
	...normalizedOptions,
	secureEndpoint: url.protocol === 'https:',
	})
	if (agentCache.has(cacheKey)) {
	return agentCache.get(cacheKey)
	}
	const newAgent = new Agent(normalizedOptions)
	agentCache.set(cacheKey, newAgent)

agent/lib/agents.js

Lines 26 to 32 in e24eb6c

	if (proxy) {
	this.#proxy = {
	proxy: urlify(proxy),
	noProxy,
	Agent: getProxyAgent(proxy),
	}
	}

agent/lib/proxy.js

Lines 26 to 37 in e24eb6c

	const getProxyAgent = (url) => {
	url = urlify(url)
	const protocol = url.protocol.slice(0, -1)
	if (SOCKS_PROTOCOLS.has(protocol)) {
	return SocksProxyAgent
	}
	if (protocol === 'https' || protocol === 'http') {
	return [HttpProxyAgent, HttpsProxyAgent]
	}
	throw new InvalidProxyProtocolError(url)

agent/lib/agents.js

Lines 64 to 72 in e24eb6c

	let { Agent: ProxyAgent } = this.#proxy
	if (Array.isArray(ProxyAgent)) {
	ProxyAgent = options.secureEndpoint ? ProxyAgent[1] : ProxyAgent[0]
	}
	const proxyAgent = new ProxyAgent(proxy, this.#options)
	proxyCache.set(cacheKey, proxyAgent)
	return proxyAgent

Expected Behavior

• I would expect npm install to use HttpProxyAgent when the requested url has an http protocol.
• I would expect npm install to use HttpsProxyAgent when the requested url has an https protocol.

Steps To Reproduce

1. Use the latest node and npm, for example: node v21.3.0 (npm v10.2.4)
2. Configure .npmrc to use a proxy, for example:

https-proxy=<some proxy>
http-proxy=<some proxy>

3. Ensure an empty cache and install a package like uuid

$ npm cache clean --force
$ npm install -g uuid

4. Observe HttpProxyAgent is used to fetch an https url: 'https://registry.npmjs.org/uuid'
5. Observe the request's path is modified when using an HttpProxyAgent to setRequestProps for an https url. For example, the request.path is modified from '/uuid' to 'http://registry.npmjs.org:443/uuid'.

agent/lib/agents.js

Lines 161 to 173 in e24eb6c

	addRequest (request, options) {
	const proxy = this.#getProxy(options)
	// it would be better to call proxy.addRequest here but this causes the
	// http-proxy-agent to call its super.addRequest which causes the request
	// to be added to the agent twice. since we only support 3 agents
	// currently (see the required agents in proxy.js) we have manually
	// checked that the only public methods we need to call are called in the
	// next block. this could change in the future and presumably we would get
	// failing tests until we have properly called the necessary methods on
	// each of our proxy agents
	if (proxy?.setRequestProps) {
	proxy.setRequestProps(request, options)
	}

6. Note that the installation succeeds in the example above, but other registries appear to be less forgiving of the path change and this can cause installation issues in some cases.

Environment

• npm: 10.2.4
• Node: 21.3.0
• OS: macOS 14.1.1
• platform: macOS

[lukekarrys]

fixed by #92