Strange behavior with request path extensions

[FlorianKoerner]

Q	A
Bug?	yes
New Feature?	no
Sulu Version	2.6.3
SuluRedirect Version	2.1.3

Actual Behavior

The redirects do not work as expected if the source or target has an extension.

Source: /foo.php
Target: /bar.php

Request: /foo.php
Response: 404 Error

Expected Response: /bar.php

Source: /foo
Target: /bar.php

Request: /foo.php
Response: /bar.php.php

Expected Response: /bar.php

Source: /foo
Target: https://example.com/bar/

Request: /foo.php
Response: https://example.com/bar/.php

Expected Response: https://example.com/bar/

The current behavior works great for internal redirects that follow Sulu's pattern. But for external redirects or internal redirects that are taken over from a legacy system, the current behavior quickly fails.

Possible Solution

The following workaround works for us. We have overwritten most of the RedirectRouteProvider and the WebsiteRedirectController. I am also happy to create a pull request once the target behavior has been agreed.

<?php

declare(strict_types=1);

namespace App\Vendor\Sulu\Bundle\RedirectBundle\Routing;

use Sulu\Bundle\RedirectBundle\Model\RedirectRouteRepositoryInterface;
use Sulu\Bundle\RedirectBundle\Routing\RedirectRouteProvider as Inner;
use Symfony\Cmf\Component\Routing\RouteProviderInterface;
use Symfony\Component\DependencyInjection\Attribute\AsDecorator;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouteCollection;

#[AsDecorator(decorates: 'sulu_redirect.routing.provider', onInvalid: ContainerInterface::IGNORE_ON_INVALID_REFERENCE)]
readonly class RedirectRouteProvider implements RouteProviderInterface
{
    public function __construct(
        private Inner $inner,
        private RedirectRouteRepositoryInterface $redirectRouteRepository,
    ) {
    }

    public function getRouteCollectionForRequest(Request $request): RouteCollection
    {
        // server encodes the url and symfony does not encode it
        // symfony decodes this data here https://github.com/symfony/symfony/blob/v5.2.3/src/Symfony/Component/Routing/Matcher/UrlMatcher.php#L88
        $pathInfo = \rawurldecode($request->getPathInfo());
        $host = $request->getHost();

        // Sulu strips the request extension from query. This causes problems if we explicitly want to forward a file
        // extension. That's why we first search for the request URL with the request extension and only call the Sulu
        // logic if we don't find anything here.
        $redirectRoute = $this->redirectRouteRepository->findEnabledBySource($pathInfo, $host);

        if (!$redirectRoute) {
            return $this->inner->getRouteCollectionForRequest($request);
        }

        $route = new Route(
            $pathInfo,
            [
                '_controller' => 'sulu_redirect.controller.redirect::redirect',
                'redirectRoute' => $redirectRoute,
            ],
        );

        $routeCollection = new RouteCollection();
        $routeCollection->add(\sprintf('sulu_redirect.%s', $redirectRoute->getId()), $route);

        return $routeCollection;
    }

    public function getRouteByName($name): Route
    {
        return $this->inner->getRouteByName($name);
    }

    public function getRoutesByNames($names = null): iterable
    {
        return $this->inner->getRoutesByNames($names);
    }
}

<?php

declare(strict_types=1);

namespace App\Vendor\Sulu\Bundle\RedirectBundle\Controller;

use Sulu\Bundle\RedirectBundle\Controller\WebsiteRedirectController as Inner;
use Sulu\Bundle\RedirectBundle\Model\RedirectRouteInterface;
use Symfony\Component\DependencyInjection\Attribute\AsDecorator;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Exception\HttpException;

#[AsDecorator(decorates: 'sulu_redirect.controller.redirect', onInvalid: ContainerInterface::IGNORE_ON_INVALID_REFERENCE)]
readonly class WebsiteRedirectController
{
    public function __construct(private Inner $inner)
    {
    }

    public function redirect(Request $request, RedirectRouteInterface $redirectRoute): RedirectResponse
    {
        if (410 === $redirectRoute->getStatusCode()) {
            throw new HttpException(410);
        }

        $sourcePathHasExtension = $this->urlHasFileExtension($redirectRoute->getSource());

        $targetHasHost = $this->urlHasHost($redirectRoute->getTarget());
        $targetHasFile = $this->urlHasFile($redirectRoute->getTarget());
        $targetHasFileExtension = $this->urlHasFileExtension($redirectRoute->getTarget());

        // Sulu forces the request extension on the forwarding target. However, this is not our desired behavior if the
        // forwarding target already has a file extension or the forwarding target has a host (therefore probably external)
        // or the forwarding target has already a file extension or is not a file but a directory.
        if (!$sourcePathHasExtension && !$targetHasHost && $targetHasFile && !$targetHasFileExtension) {
            return $this->inner->redirect($request, $redirectRoute);
        }

        $url = [
            $redirectRoute->getTarget(),
            \str_contains($redirectRoute->getTarget(), '?') ? '&' : '?',
            \http_build_query($request->query->all()),
        ];

        $url = \trim(\implode('', $url), '&? ');

        return new RedirectResponse($url, $redirectRoute->getStatusCode());
    }

    private function urlHasHost(string $url): bool
    {
        $host = \parse_url($url, \PHP_URL_HOST);

        return \is_string($host) && !\str_ends_with($host, '/');
    }

    private function urlHasFile(string $url): bool
    {
        $path = \parse_url($url, \PHP_URL_PATH);

        return \is_string($path) && !\str_ends_with($path, '/');
    }

    private function urlHasFileExtension(string $url): bool
    {
        $path = \parse_url($url, \PHP_URL_PATH);

        return \is_string($path) && \pathinfo($path, \PATHINFO_EXTENSION);
    }
}

[mcoiffier]

Thank you for your solution. I'm interested in the PR.
I will try to implement it on my own. However, I'm not sure where to place these override files.
I know I shouldn't modify the bundle directly since any changes would be lost during the next composer update.
Thank you for your help.

[FlorianKoerner]

You can place them wherever you want, as long as the namespace fits accordingly. For my example from above:

• src/Vendor/Sulu/Bundle/RedirectBundle/Routing/RedirectRouteProvider.php
• src/Vendor/Sulu/Bundle/RedirectBundle/Controller/WebsiteRedirectController.php

[mcoiffier]

Great, thanks!

Here’s what I had done initially:
I placed the provider code in /src/Provider/Redirect/RedirectRouteProvider.php with the namespace App\Provider\Redirect,
and the controller code in /src/Controller/Redirect/WebsiteRedirectController.php with the namespace App\Controller\Redirect.

However, I was encountering the following error:

Cannot autowire service "App\Provider\Redirect\RedirectRouteProvider": argument "$redirectRouteRepository" of method "__construct()" references interface "Sulu\Bundle\RedirectBundle\Model\RedirectRouteRepositoryInterface" but no such service exists. You should maybe alias this interface to the existing "sulu.repository.redirect_route" service.

I thought the issue was due to the incorrect placement of my code, but your message prompted me to investigate further.

I ended up resolving it by aliasing the service and modifying the constructor of the provider as follows:

public function __construct(
    private Inner $inner,
    #[Autowire(service: 'sulu.repository.redirect_route')]
    private RedirectRouteRepositoryInterface $redirectRouteRepository,
) {
}

Now the error is gone, and I can start testing the redirections. Thanks again for your quick response!