Set lowercase headers for NPM audit requests

[maartenba]

PR fixing https://npm.community/t/npm-audit-making-non-rfc-compliant-requests-to-server-resulting-in-400-bad-request-pr-with-fix/1742

NPM audit making non-RFC-compliant requests to server resulting in 400 Bad Request (+ PR with fix)

What I Wanted to Do

Run the npm audit command against a custom registry, and get back the expected result from this command.

What Happened Instead

The server returned a 400 bad Request status code.

So instead of blaming NPM, I dove into the server code, and after a long search found out that NPM's audit command sends the content-type HTTP header twice.

Reproduction Steps

Not applicable as they are very specific. But do read on, please.

Details

Here's the verbose log from running npm audit against my repo:

0 info it worked if it ends with ok
1 verbose cli [ 'C:\\Program Files\\nodejs\\node.exe',
1 verbose cli   'C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js',
1 verbose cli   'audit',
1 verbose cli   '--registry',
1 verbose cli   'http://localhost:1234/npm' ]
2 info using npm@6.2.0
3 info using node@v10.9.0
4 verbose npm-session 044f543ee484e70f
5 timing audit compress Completed in 3ms
6 info audit Submitting payload of 264 bytes
7 http fetch POST 400 http://localhost:1234/npm/-/npm/v1/security/audits 78ms
8 verbose stack Error: 400 Bad Request - POST http://localhost:1234/npm/-/npm/v1/security/audits
8 verbose stack     at res.buffer.catch.then.body (C:\Program Files\nodejs\node_modules\npm\node_modules\npm-registry-fetch\check-response.js:94:15)
8 verbose stack     at process._tickCallback (internal/process/next_tick.js:68:7)
9 verbose statusCode 400
10 verbose cwd C:\Users\maart\Desktop\foo
11 verbose Windows_NT 10.0.17134
12 verbose argv "C:\\Program Files\\nodejs\\node.exe" "C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js" "audit" "--registry" "http://localhost:1234/npm"
13 verbose node v10.9.0
14 verbose npm  v6.2.0
15 error code E400
16 error 400 Bad Request - POST http://localhost:1234/npm/-/npm/v1/security/audits
17 verbose exit [ 1, true ]

Having some HTTP inspector in between, this is the request that was made:

POST http://localhost:1234/npm/-/npm/v1/security/audits
connection: keep-alive
user-agent: npm/6.2.0 node/v10.9.0 win32 x64
npm-in-ci: false
npm-scope: 
npm-session: db3e53c56f1e3571
referer: undefined
content-encoding: gzip
content-type: application/json
content-type: application/octet-stream
accept: */*
content-length: 352
accept-encoding: gzip,deflate
Host: localhost:1234
Fiddler-Encoding: base64

H4sIAAAAAAAACmWPX0+DMBTFv0ufsfLPTUn2YNZs0U1lmhAyo6aDMktoC21XZITvbtkeXOLbvb9zcs89PeCYERCBQgjgAEOkooLb3YMudC2RpDlQSRSIerATQistcW31zxB6MACDA3JSE54TntF/rv7i4NnvAMo12UuqO8vUN77x/CuJFg/HfZqJwi1RVz4LVZrwjYvpNA1Xh5gsfpZ+ut0a73qV3aLXZJnco1xsuu4lQW3crIMJQkzPNztZP80bU8frx43bzmZgGMY8pXFVgej9Y2zDhCHnmRGNc6zx+Cav2dffqxPon7pzkZMLbDwX3p2EusK6EJJZ2FIe+DboF6BsoeJJAQAA

The issue in this request is that any webserver that does request filtering will throw a 400 Bad request as a response. Why? The issue is in having the content-type header sent twice:

content-type: application/json
content-type: application/octet-stream

Having looked in the NPM source code, the npm audit command sets its header here:
https://github.com/npm/cli/blob/latest/lib/install/audit.js#L90

Have a good look, as there is nothing wrong here. Except, take note that the header is CamelCase Content-Type.

Diving into making the request, we can see this block:
https://github.com/npm/npm-registry-fetch/blob/latest/index.js#L39

The check that happens there checks lowercase content-type, and when that does not exist, adds content-type: application/octet-stream.

After some debugging, I found this to be the issue of seeing duplicate headers.

How to fix? There are two potential fixes, and I created a PR for one.

• Fix 1 - Make sure headers are added lowercase in https://github.com/npm/cli/blob/latest/lib/install/audit.js#L90 - PR: Set lowercase headers for NPM audit requests #62
• Fix 2 - Always make header keys lowercase in this function: https://github.com/npm/npm-registry-fetch/blob/latest/index.js#L156

Platform Info

$ npm --versions
{ npm: '6.2.0',
  ares: '1.14.0',
  cldr: '33.1',
  http_parser: '2.8.0',
  icu: '62.1',
  modules: '64',
  napi: '3',
  nghttp2: '1.32.0',
  node: '10.9.0',
  openssl: '1.1.0i',
  tz: '2018e',
  unicode: '11.0',
  uv: '1.22.0',
  v8: '6.8.275.24-node.14',
  zlib: '1.2.11' }
$ node -p process.platform
win32

[zkat]

This seems like a good idea. To be fair, this code is changing completely soon, but I'll take this patch in the interim. Thank you!