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

Redirect to url with trailing slash, WITHOUT the protocol and host. #945

Merged
merged 1 commit into from
Nov 29, 2024

Conversation

paulharris
Copy link
Contributor

Crow does not know the correct protocol when behind a reverse proxy. So instead, simply redirect to the same url and add a trailing slash.

Regarding #915
I've just realised that this fails when behind a reverse proxy.

Situation: running crowcpp as a HTTP behind a HTTPS reverse proxy
Crow wants to redirect.
It sets the location to the full URL, including protocol.
It assumes http:// is correct, as that is what it is running.
But, it needs https:// to correctly hit the reverse proxy...

Why do we need to specify the host? Is there a good reason?
I couldn't see any reasoning given in prior commits or issues.

967adf0#diff-83f8db2c08b899a7b155ca1ddbef781d51babff4553c79cd20fb3a216ffc7780R937
4cdde73#diff-cce8b59e628bdbbe6a329eea27b56b908c8a1d5926a4b44f697f39a301046c93L1402

07042b5#diff-83f8db2c08b899a7b155ca1ddbef781d51babff4553c79cd20fb3a216ffc7780R677

Crow does not know the correct protocol when behind a reverse proxy.
So instead, simply redirect to the same url and add a trailing slash.

Note, it is now valid for the "Location" header field to be relative.
From wikipedia:
An obsolete version of the HTTP 1.1 specifications (IETF RFC 2616)
required a complete absolute URI for redirection.
The IETF HTTP working group found that the most popular web browsers
tolerate the passing of a relative URL[3] and, consequently, the updated
HTTP 1.1 specifications (IETF RFC 7231) relaxed the original constraint,
allowing the use of relative URLs in Location headers.
@paulharris paulharris force-pushed the fix-redirect-trailing-slash branch from f180244 to 7aef5b3 Compare November 13, 2024 04:02
@paulharris
Copy link
Contributor Author

Note that I found that Location required the host in the past,
it is now valid to use relative paths.
I've put the quote into the commit, but not the link.
https://en.wikipedia.org/wiki/HTTP_location

So I think the host+protocol is not required anymore.

@gittiver
Copy link
Member

https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.2 states this, so solution is conforming to standard.

@gittiver gittiver merged commit 042776f into CrowCpp:master Nov 29, 2024
11 checks passed
@paulharris paulharris deleted the fix-redirect-trailing-slash branch December 2, 2024 01:59
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