Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

case sensitivity of headers should not matter #262

Open
bcoe opened this issue Mar 24, 2020 · 4 comments
Open

case sensitivity of headers should not matter #262

bcoe opened this issue Mar 24, 2020 · 4 comments
Assignees
Labels
needs more info This issue needs more information from the customer to proceed. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@bcoe
Copy link
Contributor

bcoe commented Mar 24, 2020

Currently if one were to set a "content-type" header and a "Content-Type" header, the two are not equivalent.

I've seen libraries like express approach this differently, such that headers are case-insensitive.

@bcoe bcoe added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Mar 24, 2020
@kungfutse
Copy link

Headers are case-insensitive according to RFC2616 so this is a bug, not a feature request, in my opinion.

@JustinBeckwith
Copy link
Contributor

I'm not sure what the actual bug is here :) Can someone give me an example? Right now the request.Headers field is just a POJO.

For example:

const options: GaxiosOptions = { 
  headers: {
    'content-type': 'application/json',
    'Content-Type': 'text/xml',
  },
},

We could look for this situation and throw if we find duplicate keys once lower cased?

@JustinBeckwith JustinBeckwith added the needs more info This issue needs more information from the customer to proceed. label May 2, 2020
@bcoe
Copy link
Contributor Author

bcoe commented May 6, 2020

@JustinBeckwith a warning might be enough, but I think a cleaning step that lowercases headers would be good, that way upstream libraries that build on top of gaxios, e.g., google-auth-library need not check every combination of a given header.

@danielbankhead
Copy link
Member

We should the native Headers class to resolve this; we'll look into this in the major after next 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs more info This issue needs more information from the customer to proceed. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

4 participants