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

Fixing issue #6817 (failed to match named routes). #6828

Open
wants to merge 2 commits into
base: 4.x
Choose a base branch
from

Conversation

FoxCie
Copy link

@FoxCie FoxCie commented Feb 19, 2025

No description provided.

@FoxCie FoxCie mentioned this pull request Feb 19, 2025
@@ -294,6 +294,26 @@ private function getSymfonyControllerFqcn(Request $request): ?string
// sub-url or sub-path - using the HTTP header x-forwarded-prefix)
$url = $this->urlGenerator->generate($routeName, $routeParams, UrlGeneratorInterface::RELATIVE_PATH);

// Now, forge the "absolute" path so that it can be matched to a route.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just asking: if this code is neede to take care of localized URLs ... couldn't this be fixed if we do something like:

+$routeParams = array_merge($routeParams, ['_locale', $request->attributes->get('_locale')])
$url = $this->urlGenerator->generate($routeName, $routeParams, UrlGeneratorInterface::RELATIVE_PATH);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not test it, but I guess it might have worked.

Nevertheless, I think that trying to match a relative path to a route is not a good idea, because the relative path will actually be considered as an absolute one by the matcher (e.g. if the path is ../path, it will be treated as /../path by the matcher (because that is what is returned by getPathInfo()). At least that's what I have seen from my dumps.

I searched a little in Symfony's code, and I found that they actually compute the path we want, and then is is modified either to a relative one or to a full url with the base path here https://github.com/symfony/symfony/blob/7.3/src/Symfony/Component/Routing/Generator/UrlGenerator.php#L255 . They do this :

        if (self::RELATIVE_PATH === $referenceType) {
            $url = self::getRelativePath($this->context->getPathInfo(), $url);
        } else {
            $url = $schemeAuthority.$this->context->getBaseUrl().$url;
        }

I am pretty sure that the initial value of $url is exactly the path we are looking for, so we have to compute it back. I did it by (trying to) reverse the part where they compute the relative path, and what has been merged reverses the else part instead (note : when using ABSOLUTE_PATH and not ABSOLUTE_URL, the $schemeAuthority should be empty, hence the solution is valid if $this->context->getBaseUrl() returns the same thing as $request->getBasePath(), which seems to be the case.

I feel like it would be nice to actually be able to retrieve the path without the prefix with another constant for instance (ABSOLUTE_LOCAL_PATH ?), but we would need to modify this in Symfony, which is out of our scope.

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