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 CORS behaviour if there is no origin header #439

Merged
merged 1 commit into from
Apr 20, 2020
Merged

Fix CORS behaviour if there is no origin header #439

merged 1 commit into from
Apr 20, 2020

Conversation

rkusa
Copy link
Contributor

@rkusa rkusa commented Apr 18, 2020

The CORS origin header handling fell back to an empty header value. This value is considered valid for Origin::Any (wildcard origin). However, if the CORS middleware is restricted to a whitelist of origins, the is_valid_origin check correctly fails for an empty origin header
value. Though, since we fell back to an empty value when there is no origin header at all, we fail the validation and consequentially respond with a 403 (forbidden) to such requests.

The CORS specifications stats the following for simple cross-origin requests, actual requests and preflight requests:

If the Origin header is not present terminate this set of steps. The request is outside the scope of this specification.

This PR therefore updates the CORS middleware to ignore requests that have no origin header set.

The CORS origin header handling fell back to an empty header value. This
value is considered valid for `Origin::Any` (wildcard origin). However,
if the CORS middleware is restricted to a whitelist of origins, the
`is_valid_origin` check correctly fails for an empty origin header
value. Though, since we fell back to an empty value when there is no
origin header at all, we fail the validation and consequentially respond
with a 403 (forbidden) to such requests.

The CORS specifications stats the following for simple cross-origin
requests, actual requests and preflight requests:

> If the Origin header is not present terminate this set of steps. The
> request is outside the scope of this specification.

This commit therefore updates the CORS middleware to ignore requests
that have no origin header set.
@@ -395,7 +398,7 @@ mod test {
#[test]
fn not_set_origin_header() {
let mut app = app();
app.middleware(Cors::new());
app.middleware(Cors::new().allow_origin(ALLOW_ORIGIN));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test succeeded before because the default setting is Origin::Any which is successful for empty origin header values (as described in the PR message). Adding a specific header was necessary to make the test fail (before this PR).

Copy link
Member

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great; thanks heaps!

@yoshuawuyts yoshuawuyts merged commit ac22bb3 into http-rs:master Apr 20, 2020
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