type-is icon indicating copy to clipboard operation
type-is copied to clipboard

Should support upper-case headers

Open aaronatbissell opened this issue 1 year ago • 9 comments

According to RFC 9110, I believe headers are meant to be case-insensitive.

Field names are case-insensitive ...

Running the test suite in this project with a header of Content-Type yields errors. Would be nice to case-insensitively process those headers.

I understand most clients will lower-case headers on the way out, but we have been using this in machine-to-machine APIs that don't always get lower-cased on the way out.

aaronatbissell avatar Jun 12 '24 11:06 aaronatbissell

It is already case insensitive as Node.js HTTP lower cases them already. Try out the example at https://github.com/jshttp/type-is?tab=readme-ov-file#api and send upper case headers to see.

dougwilson avatar Jun 12 '24 11:06 dougwilson

That's true if you are using Node.js HTTP. We are using this in a project that does not use Node.js HTTP and has some clients that happen to send headers with upper-case characters.

aaronatbissell avatar Jun 12 '24 11:06 aaronatbissell

This project only works with Node.js HTTP. If you are using something else, you should be using something designed to work with whatever you are using instead.

dougwilson avatar Jun 12 '24 11:06 dougwilson

From the docs:

The request argument is expected to be a Node.js HTTP request.

dougwilson avatar Jun 12 '24 12:06 dougwilson

I'm sure you know this already, but express uses this package. It's unfortunate that now express doesn't work because of a case sensitivity issue with a header.

aaronatbissell avatar Jun 12 '24 12:06 aaronatbissell

Express uses Node.js HTTP. The req.headers object that enters into this module only contains lower case header names, even when the client sends them in capitals. This is because Node.js HTTP (and thus Express.js) will always lower case the headers when they enter the server.

Node.js does this because if you have a plan object like req.headers and want the header Content-Type, it is not ezpected to try and look up every possible upper and lower variant in the object to find it, thus why Node.js will always lower case the header names when it parses the request from the client.

Perhaps you can provide a full Express.js app and show us a cURL request or similar with upper case headers that does not work.

https://nodejs.org/api/http.html#messageheaders

The request/response headers object.

Key-value pairs of header names and values. Header names are lower-cased.

dougwilson avatar Jun 12 '24 12:06 dougwilson

Here's an issue from another project where they were seeing the exact same problem. I'm confident it's because their Content-Type header is capitalized. I'll be the first to admit that this isn't technically going over HTTP because it's invoking a lambda function directly instead of using something like API Gateway, but it's still possible to invoke express with a request that technically satisfies the shape of the request object, but has upper-case headers.

aaronatbissell avatar Jun 12 '24 12:06 aaronatbissell

it's still possible to invoke express with a request that technically satisfies the shape of the request object, but has upper-case headers.

Then it wouldn't be the correct shape, as req.headers must be in lower case keys. I linked to the docs above that specify it. I suggest of this is the case, you can close this issue and let them know the bug is with invoking Express.js manually outside of using Node.js HTTP and not actually adhering to their API contract of req.headers needs to be lower case keys.

The reason Node.js sets all the keys to lower case is because in Javascript a key lookup is case sensitive and in order to accommodate case insensitive this module would have to look up every possible combination. And what is req.headers were to then contain both a content-type key and a Content-Type key? This module.would now have to.procide some kind of way to reconcile that. All of that, thoigh, is taken care of in the Node.js HTTP layer ans this module uses the API contract it provides in order to not have to duplicate all that behavior (and slow down every request by now needing to double-check it all if it cannot trust it). The bug is clearly in that lamfa thing bypassing the Node.js HTTP processing and not adhering to it's API contract.

dougwilson avatar Jun 12 '24 12:06 dougwilson

I've requested an enhancement to the other library to mimic the behavior of Node.js HTTP

aaronatbissell avatar Jun 12 '24 13:06 aaronatbissell