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

fix: uppercase origin header #287

Merged
merged 1 commit into from
Jun 19, 2022
Merged

Conversation

n3oney
Copy link
Contributor

@n3oney n3oney commented Jun 18, 2022

While HTTP headers should be case-insensitive, unfortunately some servers don't conform to that behavior. Unfortunately, the http crate always stores headers in lowercase without any way to prevent that behavior, so if we run into a server that does not conform to spec, we're doomed.
This is a workaround to that issue - it changes the origin header into Origin.

@daniel-abramov
Copy link
Member

Hm, alright, let's merge it! This is a second header for which we implement such a workaround, perhaps we would need to find a generic solution for it (similar to how hyper handled it once) since it's not clear how many other headers will be ignored by the server for the same reason (the server that performs a case-sensitive check of a single header is likely to do the same for all other headers as well).

@daniel-abramov daniel-abramov merged commit fbac502 into snapview:master Jun 19, 2022
@n3oney
Copy link
Contributor Author

n3oney commented Jun 20, 2022

Hm, alright, let's merge it! This is a second header for which we implement such a workaround, perhaps we would need to find a generic solution for it (similar to how hyper handled it once) since it's not clear how many other headers will be ignored by the server for the same reason (the server that performs a case-sensitive check of a single header is likely to do the same for all other headers as well).

I definitely agree. I think it'd be much better to implement some kind of custom hashmap to handle headers. I know that isahc has a title_case_headers configuration option which as the name suggests turns headers into title case, however that's still very much a workaround, and a full-fledged solution would be appreciated.

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

Successfully merging this pull request may close these issues.

2 participants