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-sensitive look up on allowed headers map #176

Closed
EmmEff opened this issue Apr 29, 2024 · 4 comments
Closed

Case-sensitive look up on allowed headers map #176

EmmEff opened this issue Apr 29, 2024 · 4 comments

Comments

@EmmEff
Copy link

EmmEff commented Apr 29, 2024

Allowed headers are ingested correctly in initialization and in the call cors.handlePreflght():

cors/cors.go

Line 368 in 3d336ea

if found && !c.allowedHeadersAll && !c.allowedHeaders.Subsumes(reqHeaders[0]) {

The headers in the request are handled AS IS.

Allowed headers are normalized during initialization:

cors/cors.go

Line 199 in 3d336ea

normalized := convert(options.AllowedHeaders, strings.ToLower)

In the function SortedSet.Subsumes(), a case-sensitive lookup is done on the map:

pos, found := set.m[name]

This issue surfaced in the v1.11.0 release.

@rs
Copy link
Owner

rs commented Apr 29, 2024

The spec says headers are sent as a ordered lowercase list. Is there a specific case where this change break something?

@EmmEff
Copy link
Author

EmmEff commented Apr 29, 2024

Well it broke my code :)

In my investigation, I was following the docs @ https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS which uses mixed case values in the CSV argument to Access-Control-Request-Headers in their examples.

I see the spec is conflicts with that (as you mention)

@jub0bs
Copy link
Contributor

jub0bs commented Apr 29, 2024

@EmmEff The middleware is working as expected. That v1.11.0 broke your code/tests is unfortunate, but your tests arguably shouldn't be coupled so strongly to the intricacies of rs/cors's implementation. I'd be interested to see the failing test in question.

I was following the docs @ https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS which uses mixed case values in the CSV argument to Access-Control-Request-Headers in their examples.

Indeed; for instance,

Access-Control-Request-Headers: X-PINGOTHER, Content-Type

should read

Access-Control-Request-Headers: content-type,x-pingother

Well spotted; I understand how that can trip people up. In that case, the right thing to do is open an issue on https://github.com/mdn/content; I'll do that.

@jub0bs
Copy link
Contributor

jub0bs commented May 2, 2024

@EmmEff Are you still experiencing problems or can we this issue be closed now?

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

3 participants