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

Incorrect routing on duplicate slashes #2184

Closed
hundblue opened this issue Sep 13, 2018 · 3 comments
Closed

Incorrect routing on duplicate slashes #2184

hundblue opened this issue Sep 13, 2018 · 3 comments
Assignees

Comments

@hundblue
Copy link

URL pathnames prefixed by duplicate slashes ('//', '///', ...) are incorrectly interpreted as absolute URLs. At present, the URL https://learn.getgrav.org//__FAKE_HOSTNAME__/content/routing
is resolved to
https://learn.getgrav.org/content/routing
This is caused by the $bits = parse_url($uri); asignment in Uri.php.

@Perlkonig
Copy link
Contributor

The word "incorrectly" implies that there's a "correct" way to interpret those slashes. But if you look at the relevant RFCs, I can find no prescribed normalization of such slashes. Did I miss something?

So URI paths containing such are simply invalid, and the result of running such a URL through various tools is undefined. parse_url is a built-in PHP function. Grav can't do anything about it, short of rolling its own.

@mahagr
Copy link
Member

mahagr commented Sep 13, 2018

URL starting with // is shorthand for the current protocol which is either http:// or https://.

That said the behaviour you observed in the learn site is wrong, it should end up to 404 page.

@mahagr
Copy link
Member

mahagr commented Mar 31, 2021

Confirmed this to be a bug in Grav\Common\Uri class. The issue isn't present on other libraries.

@mahagr mahagr added bug and removed evaluating labels Mar 31, 2021
@mahagr mahagr added the fixed label Mar 31, 2021
mahagr added a commit that referenced this issue Mar 31, 2021
@mahagr mahagr closed this as completed Apr 13, 2021
ViliusS added a commit to ViliusS/grav that referenced this issue Apr 24, 2024
…etgrav#2184]"

Previous change broke running Grav under Load Balancer with custom_base_url
set. Also, it's not a good idea to add 3rd party domain into host value,
which could be accidently reused some time in the future.

This (not counting CHANGELOG) reverts commit 0af3385.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants