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

Strange behavior with request path extensions #94

Open
FlorianKoerner opened this issue Jul 26, 2024 · 0 comments
Open

Strange behavior with request path extensions #94

FlorianKoerner opened this issue Jul 26, 2024 · 0 comments

Comments

@FlorianKoerner
Copy link

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);
    }
}
@FlorianKoerner FlorianKoerner changed the title Strange behavior with request extensions Strange behavior with request path extensions Jul 26, 2024
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