[Router] Optionnaly disable the trailing slash redirection behavior #33342
Comments
|
Closing in favor of #32996 |
|
@Jean-ita do you have an opinion on this ? |
did you give this a try? Maybe that's something that should be documented better and done? |
|
Well, that also disable some other features |
|
@nicolas-grekas no I didn't, as @stof said, that also disable other features & I don't think that's a good alternative. |
|
Just to add another point in favor of disabling the trailing slash redirection behavior: The automatic redirect can break urls if they use query parameter arrays - for example: |
|
The documentation explains this as "nowadays it’s common to treat both URLs as the same URL and redirect between them". Yes, it's common, but not universal. The URLs are exposed to the end user. This is something where the framework shouldn't get in the way. If I want to have an application that doesn't follow this, Symfony is not providing a proper way to do this without losing some other features. I think adding a setting that would be checked in addition to whether RedirectableUrlMatcherInterface is implemented here https://github.com/symfony/routing/blob/7485aa5c8999dff509d1b433339a6b131dcdf845/Matcher/UrlMatcher.php#L135 would be a good way to resolve this for people that get struck by this behavior and have no other choice but use an UrlMatcher that doesn't implement RedirectableUrlMatcherInterface and lose some features. |
|
In my opinion url with event_controllers:
resource: ../../src/Module/Event/Controller/Api
type: annotation
name_prefix: event_
prefix: /api/v1/eventand my controller looks like this namespace App\Module\Event\Controller\Api;
(...)
/**
* Class EventController
*
* @package App\Module\Event\Controller\Api
*
* @Rest\Route("", name="event_")
*/
class EventController extends ApiController
{
/**
* @return View
*
* @Rest\Get("", name="index")
*/
public function index(): Viewand in my swagger (i using nelmio/api-doc-bundle:^3.4) i have |
|
Hello all. This feature will be useful. //ordering of methods is important, if notFoundSlug will be first then it not working
/**
* @Route(path="/{slug}{slash}", name="hack_trailing_slash_200_ok", requirements={"slash"="\/"})
*/
public function foundBySlug()
{
return new Response("ok");
}
/**
* @Route(path="/{slug}", name="hack_trailing_slash_404_redirect")
*/
public function notFoundSlug()
{
throw $this->createNotFoundException();
}But if i remove |
|
@ivanbogomoloff you can match a route with a forced trailing |
Not working. /**
* @Route(path="/{slug}{_</(?!/)>}", name="slug_slash")
*/
public function foundBySlug()
{
return new Response("ok");
}When i try I found another solution, but it scary too /**
* @Route(path="/{slug}{slash}", name="slug_slash", requirements={"slash" = ".+"})
*/
public function foundBySlug(Request $request)
{
$url = $request->getPathInfo();
if(!preg_match('/\/$/i', $url)) {
// /anything will give 404
throw $this->createNotFoundException();
}
// /anything/ give ok
return new Response("ok");
} |
Yes, that's because you'd also need to define (after the one with a slash)
It's how catch-all controllers are supposed to work yes. |
|
Thank you for this issue. |
|
Could I get an answer? If I do not hear anything I will assume this issue is resolved or abandoned. Please get back to me <3 |
|
Just a quick reminder to make a comment on this. If I don't hear anything I'll close this. |
|
Could I get an answer? If I do not hear anything I will assume this issue is resolved or abandoned. Please get back to me <3 |
|
Friendly ping? Should this still be open? I will close if I don't hear anything. |
|
@carsonbot As far as I'm aware this was not implemented yet. |
|
Thank you for this issue. |
|
@carsonbot I could send fix to routing component, but i guess it might be incomptatable with "symfony way", but it works.
Example: class TrailingController extends AbstractController
{
/**
* @Route("/anything/", name="a2", defaults={"_trailing_slash": false})
*/
public function anythingWitSlash()
{
// vendor/symfony/routing/Matcher/Dumper/CompiledUrlMatcherTrait.php
return new Response($this->generateUrl('a1'));
}
/**
* @Route("/anything", name="a1", defaults={"_trailing_slash": false})
*/
public function anythingWithoutSlash()
{
return new Response($this->generateUrl('a2'));
}
}Then in //
foreach ($this->staticRoutes[$trimmedPathinfo] ?? [] as [$ret, $requiredHost, $requiredMethods, $requiredSchemes, $hasTrailingSlash, , $condition]) {
//...a lot of code
if ('/' !== $pathinfo && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) {
if ($supportsRedirections && (!$requiredMethods || isset($requiredMethods['GET']))) {
// Fix here at line ~106
if(isset($ret['_trailing_slash']) && $ret['_trailing_slash'] === false) {
continue;
}
return $allow = $allowSchemes = [];
}
continue;
}
//...a lot of codeIn my case it works fine with other many routes. |
|
Firstly, many thanks to @nicolas-grekas for your input into this problem, especially the suggestion to use
to allow both the trailing-slash and non-trailing-slash URL variants to exist, which I found in #32996. However the router, when generating a URL for the trailing-slash URL, requires you to pass the If you feel this belongs in a separate issue to hhis, please let me know. I think it's a relevent detail to the whole discussion around automatic redirection with/without trailings slashes. As a little background, my use-case is that we have a requirement to add a "subcategory" layer to our URL hierarchy, and therefore change our URLs from
to
So we also need to retain I agree with what @perk11 said earlier in this issue:
My feeling is that having the option to disable the automatic-redirection behaviour would save needing the "hacky" dummy route parameter. |
|
Or an option to aliasing instead of redirect. In an API requests, the |
|
Thank you for this issue. |
|
Friendly reminder that this issue exists. If I don't hear anything I'll close this. |
|
Symfony 4.4 user here, have this very problem, totally agree with @perk11 that while this redirection is common, it's clearly not universal. |
|
Oh, and by the way the « Route Matching Logs » of the profiler uses the full path to match, so its matching is wrong in such cases (it matches a route expecting a trailing slash where, in fact, it matched a non-trailing-slash-ending route earlier). |
|
Thank you for this issue. |
|
Hello? This issue is about to be closed if nobody replies. |
|
I still agree with what @perk11 said earlier in this issue:
An option to disable this particular "magic" behaviour would be welcome. |
|
is there still no option to turn off this behavior? |
|
Thank you for this issue. |
|
@carsonbot As far as I'm aware this was not implemented yet. |
|
Thank you for this issue. |
|
Could I get a reply or should I close this? |
|
It can be close in my opinion |
Uh oh!
There was an error while loading. Please reload this page.
Hi,
Since SF
3.44.1, a new behavior regarding trailing slashes has been implemented. An other version of this behavior has been around forever. This behavior, largely documented, can be a little bit confusing and lead to some strange redirect in large applications.As there is no real standard about this kind of redirect, I think this could be a better option to add a configuration key, to optionally disable all the automatic redirect to trailing slash or no stuff.
Following the comments on #32996 and #33362 this can be a valid use case to avoid a SEO risk.
Multiple options can be imagined to disable this behavior, with some pros and cons:
Global behavior:
General considerations:
Use
UrlMatcherinstead ofRedirectableUrlMatcher:Add a configuration key:
Local behavior:
General considerations:
Use and document the syntax described here (regex with negative lookahead to forbid final slash if unwanted):
Option on route declaration
What do you think ? I'll update the list if I think to more pros / cons
Thanks,
--
Edited on 2019-08-30 with more precise description
The text was updated successfully, but these errors were encountered: