/http4s

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

Structured parsing of the Origin header (issue #2007) #2082

Merged
merged 2 commits into from Sep 13, 2018

Conversation

Reviewers

@rossabaker rossabaker

@aeons aeons

@ChristopherDavenport ChristopherDavenport

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@davegurnell @rossabaker @aeons @ChristopherDavenport
@davegurnell
Contributor

davegurnell commented Sep 13, 2018

This PR improves the parsing of the Origin header. It's joint work with @hosamaly at Syntactic Sugar, London.

Instead of being interpreted as a string, the header is now interpreted as a list of items of type Origin.Host. These are similar to Uris except that Origin only permits a scheme, host, and port.

We commonly think of the Origin as containing a single host. According to the Mozilla Developer page, however, the header may be an empty string (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin). And according to the RFC, it may contain multiple hosts (https://tools.ietf.org/html/rfc6454#section-7).

There's one last complication. The RFC technically states that Origin may be the literal string null, which translates to an "opaque origin" ()
@davegurnell
Structured parsing of Origin headers. 
Work done in conjunction with Hosam Aly (@hosamaly) at Syntactic Sugar, London.

Instead of being interpreted as a string, the Origin header is now interpreted
as a "null" header or a non-empty list of "hosts" as specified in the RFC:

https://tools.ietf.org/html/rfc6454#section-7

Note that we don't use `Uri` to model the hosts because the RFC states that
only a scheme, host, and port are acceptable.

The following MDN article is the first hit on Google.
It states that the empty string is a valid Origin header.
While this is technically incorrect,
we do permissively parse the empty string as a null header:

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin

This PR also updates some tests in CORSSpec
that erroneously added a trailing slash to headers in their test data.
Loading status checks…
cbc44f2
@aeons

aeons requested changes Sep 13, 2018
View changes

This looks good. Run it though scalafmt and the build will succeed.

@davegurnell
Run through Scalafmt
Loading status checks…
e38f9db
@davegurnell

This comment has been minimized.

Sign in to view
Contributor

davegurnell commented Sep 13, 2018

Ah! Thanks for the tip. Rerunning...
@aeons

aeons approved these changes Sep 13, 2018
View changes

@rossabaker

rossabaker approved these changes Sep 13, 2018
View changes

Looks great. Thanks!

@ChristopherDavenport

ChristopherDavenport approved these changes Sep 13, 2018
View changes

@aeons aeons merged commit a37e813 into http4s:master Sep 13, 2018
2 checks passed

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment