getkirby-v2 / toolkit

This is the deprecated toolkit for Kirby v2.
http://getkirby.com
81 stars 50 forks source link

url::build removes trailing slash #259

Closed iksi closed 7 years ago

iksi commented 7 years ago

Using the build function in toolkit/lib/url.php gives an unexpected output by removing the trailing slash: url::build(['host' => 'www.getkirby.com'], 'https://www.getkirby.com/panel/'); gives the following output: https://www.getkirby.com/panel whereas I expect it to be https://www.getkirby.com/panel/

Ran into this on a server where https is being forced before the url can be rewritten to contain a trailing slash (which the build function strips) when logging into the panel. The issue doesn’t occur when the function idn_to_ascii is not available and the url gets returned as is by the function unIdn() in toolkit/lib/url.php.

Maybe related: https://github.com/getkirby/toolkit/issues/249

lukasbestle commented 7 years ago

Fixed on the develop branch with one added character. ;) This change might have broken something else though in case another method has expected the previously wrong return value. We will see.

shimikano commented 6 years ago

This change breaks the routing of paths ending in a trailing slash, which used to work fine in Kirby < 2.5.7.

E.g., a route such as

[
  'pattern' => 'foo',
  'action' => function() { return go('bar'); },
]

used to match the path foo/ since the trailing slash got trimmed away before the path is fed into the router.

For my site, this lead to lots of 404 errors since legacy links still contain the trailing slash that was present in WordPress links.

I'd consider this as a major regression and suggest considering restoring the routing behavior prior to 2.5.7.

Also cf. these forum posts:

bastianallgeier commented 6 years ago

It is indeed a major regression and I'm really sorry about it. I just committed a fix on the develop branch. It will be included in today's release.