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

Multiple slashes at start of request path breaks router #6868

Open
fvsch opened this issue Dec 16, 2024 · 1 comment
Open

Multiple slashes at start of request path breaks router #6868

fvsch opened this issue Dec 16, 2024 · 1 comment

Comments

@fvsch
Copy link

fvsch commented Dec 16, 2024

Description

A HTTP request for GET //some-path or GET ///some-path on a Kirby 4.5 site (and possibly with 5.0, haven't tested yet) will break the Kirby router.

  1. GET /some-path will show the page at content/some-path (or match another defined route).
  2. GET //some-path will show the home page.
  3. GET ///some-path will show the home page and print this warning twice in the HTML output:

Deprecated: Automatic conversion of false to array is deprecated in (…)/vendor/getkirby/cms/src/Http/Uri.php on line 101

Expected behavior

I expected one of three possible behaviors (not sure what I prefer personally):

  1. Show a 404 (or 400?) for //some-path and ///some-path.
  2. Show the same content on all three URLs.
  3. Redirect ///some-path and //some-path to /some-path.

Testing on other sites (some Google properties, Wikipedia, etc.), I'm seeing a mix of the first solution (404s) and the second one (show the same content).

To reproduce

Compare:

(Third one doesn't show PHP warnings, probably because the server configuration is better than on my server. ^^)

Your setup

Kirby Version
4.5.0

Your system (please complete the following information)
(Not relevant here.)

@fvsch
Copy link
Author

fvsch commented Dec 16, 2024

The bug happens here:

kirby/src/Http/Uri.php

Lines 90 to 99 in 94cc37e

if (is_string($props) === true) {
// make sure the URL parser works properly when there's a
// colon in the string but the string is a relative URL
if (Url::isAbsolute($props) === false) {
$props = 'https://getkirby.com/' . $props;
$props = parse_url($props);
unset($props['scheme'], $props['host']);
} else {
$props = parse_url($props);
}

  1. When $props is '//some-path':

    • It's considered to be absolute and parsed with $props = parse_url($props);
    • The value of $props becomes array(1) { ["host"]=> string(4) "some-path" }, such that the URL's pathname is lost.
  2. When $props is '///some-path':

    • It's considered to be absolute and parsed with $props = parse_url($props);
    • The value of $props becomes false, leading to the deprecation warnings mentioned above.

I don't know if the implementation of Kirby\Http\Uri can or should be hardened, or if the fix should be at the call site (which I haven't been able to identify).

By the way, I found this issue because I was facing this problem in a small web server I’m writing, and wanted to check how it works on my site (which uses Kirby).

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

No branches or pull requests

1 participant