fix: don't put opts.headers into the agent itself

[shunf4]

This PR came about as I was looking into an error message complained by npm install using a http proxy. The header content-encoding: gzip was sent in a fetch to https://registry.npmmirror.com/tar/-/tar-6.2.1.tgz, causing a 400 Bad Request response, but it's not the case when no http proxy was set; the install simply succeeded.

A minimal reproduction:

$ node -v
v20.10.0

$ npm -v
10.2.3

# Though I think the latest code is affected also.

$ npm init
$ npm install [email protected] --registry=https://registry.npmmirror.com
$ rm -rf node_modules/tar
$ npm cache clean --force
$ HTTP_PROXY="${MY_HTTP_PROXY}" HTTPS_PROXY="${MY_HTTP_PROXY}" http_proxy="${MY_HTTP_PROXY}" https_proxy="${MY_HTTP_PROXY}" npm install

npm ERR! code E400
npm ERR! 400 Bad Request - GET https://registry.npmmirror.com/tar/-/tar-6.2.1.tgz

$ curl -L --http1.1 -H 'content-encoding: gzip' -H 'content-type: application/json' -i 'https://registry.npmmirror.com/tar/-/tar-6.2.1.tgz'

HTTP/1.1 400 Bad Request
Server: Tengine
Content-Type: text/html; charset=utf-8
Content-Length: 24
Connection: keep-alive
Strict-Transport-Security: max-age=5184000
Date: *
Vary: Origin
Via: *
Ali-Swift-Global-Savetime: *
X-Cache: MISS TCP_MISS dirn:-2:-2
X-Swift-Error: orig response 4XX error
X-Swift-SaveTime: *
X-Swift-CacheTime: 0
Timing-Allow-Origin: *
EagleId: *

<h2>400 Bad Request</h2>

Digging into the code I found that the request to https://registry.npmmirror.com/*/-/*-<version>.tgz was never meant to be using gzip: true, nor had the content-encoding: gzip header ever been passed in opts.header field. It's the agent, that (unnecessarily and unwantedly?) contained the content-encoding: gzip header passed from the first request (which was the one meant to be using gzipped request body), and reused by caching mechanism for following requests. However, agent.headers will be picked up if and only if an http proxy is configured:

agent/lib/agents.js

Line 175 in 21c1987

proxy.setRequestProps(request, options)

The most obvious fix seems to be removing headers field in agent's normalizeOptions; an agent is not expected to remember the headers, in usual sense. I don't know the fix is good enough to be approved; it may bring unexpected implications or break some existing usages, since my situation is only a rather marginal case.

References

None

[wraithgar]

I think you stumbled upon a very sneaky but very real bug here. Great work.

[wraithgar]

Looking through how we use the options elsewhere this seems like it should work. The headers aren't part of the cache key, and the two times we call this are both used for making a new Agent.

[wraithgar]

We probably want to add a test (maybe to test/agent.js) that shows subsequent requests w/ differing headers.