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

Header parameters are not being handled as case-insensitive #75

Open
brocoli opened this issue Feb 4, 2019 · 1 comment
Open

Header parameters are not being handled as case-insensitive #75

brocoli opened this issue Feb 4, 2019 · 1 comment

Comments

@brocoli
Copy link

brocoli commented Feb 4, 2019

Headers field names are case-insensitive as per RFC7230, but they're being treated as case-sensitive in requests.

The root of the problem seems to be that in oas3/parameterParsers/index.ts, many parameters go through the toStructuredParser function, that does const value = rawParamValues[location.name]. rawParamValues has the header keys already in lower-case, while location.name is still in Train-Case.

I've got a PR to fix this in the works, though I'm not sure if the way I solved this was the best approach possible. I copied the one used for query parameters -- to call a different function in oas3/Operation.ts in parseParameters for them and, do a little preprocessing -- but it feels a little redundant to do this for each location (i.e. in: query / in: header), when we already have custom parsers that vary by style / format.

@brocoli brocoli changed the title Parameters with in: header are not being handled as case-insensitive Header parameters are not being handled as case-insensitive Feb 4, 2019
brocoli pushed a commit to brocoli/exegesis that referenced this issue Feb 4, 2019
Header name fields were not being handled as case-insensitive,
which would cause unwanted authentication and validation errors.
Refer RFC7230.

Fixes exegesis-js#75
@jwalton
Copy link
Contributor

jwalton commented Apr 26, 2019

Your fix here certainly looks reasonable to me. Open a PR and I'll merge it. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants