[TASK] Separate site resolving from page slug resolving 62/58062/11
authorBenni Mack <benni@typo3.org>
Tue, 28 Aug 2018 16:41:59 +0000 (18:41 +0200)
committerSusanne Moog <susanne.moog@typo3.org>
Wed, 29 Aug 2018 13:41:59 +0000 (15:41 +0200)
Currently, the site resolver checks for a valid site, and if so,
checks for valid page slugs. The latter is now moved to the
"PageResolver" middleware where the actual root line within
TSFE is resolved, and is now decoupled.

On top, a new "RouteResult" Array Access object is added
which is attached to the PSR-7 request as attribute "routing",
which will later be used for extensions
to add their own custom mappings.

The RouteResult object is now returned from the SiteMatcher
and PageRouter in order to encapsulate symfony/routing API
even more and to work with objects instead of array with magic
properties like "next" - which was actually renamed to "tail".

Fixes with this change:
- Proper and defined 307 redirect from a site (like www.domain.com)
to the default language (www.domain.com/en/)
- Proper and defined 307 redirect when a site/language is defined
without a trailing slash but accessed with a trailing slash and vice versa
- Proper and defined 307 redirect when a page slug is defined
without a trailing slash but accessed with a trailing slash and vice versa

Tests for SiteResolving have been streamlined and tests for
PageResolver have been added.

Please note:
The hook for checkAlternativeIdMethods() is now only called for
Frontend Requests without site handling.

Resolves: #86013
Releases: master
Change-Id: I9309d4092d007d555b37d575539ecf42303628de
Reviewed-on: https://review.typo3.org/58062
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Susanne Moog <susanne.moog@typo3.org>
Tested-by: Susanne Moog <susanne.moog@typo3.org>
typo3/sysext/backend/Classes/Utility/BackendUtility.php
typo3/sysext/core/Classes/Routing/PageRouter.php
typo3/sysext/core/Classes/Routing/RouteResult.php [new file with mode: 0644]
typo3/sysext/core/Classes/Routing/SiteMatcher.php
typo3/sysext/core/Documentation/Changelog/master/Feature-85947-PageBasedURLHandling.rst
typo3/sysext/frontend/Classes/Middleware/PageResolver.php
typo3/sysext/frontend/Classes/Middleware/SiteResolver.php
typo3/sysext/frontend/Tests/Functional/SiteHandling/SiteRequestTest.php
typo3/sysext/frontend/Tests/Unit/Middleware/PageResolverTest.php [new file with mode: 0644]
typo3/sysext/frontend/Tests/Unit/Middleware/SiteResolverTest.php

index 8c991ee..161c997 100644 (file)
@@ -2848,9 +2848,8 @@ class BackendUtility
                 // available sys_domain record.
                 $siteMatcher = GeneralUtility::makeInstance(SiteMatcher::class);
                 $result = $siteMatcher->matchRequest(new ServerRequest($domain));
-                if (isset($result['site']) && $result['site'] instanceof PseudoSite) {
-                    /** @var PseudoSite $site */
-                    $site = $result['site'];
+                $site = $result->getSite();
+                if ($site instanceof PseudoSite) {
                     $domain = $site->getBase();
                     $domain = ltrim($domain, '/');
                 }
index 440420c..825af84 100644 (file)
@@ -47,23 +47,26 @@ use TYPO3\CMS\Core\Utility\GeneralUtility;
  *
  * And create route candidates for that.
  *
- * PageRouter does not restrict the HTTP method or is bound to any domain constraints,
+ * Please note: PageRouter does not restrict the HTTP method or is bound to any domain constraints,
  * as the SiteMatcher has done that already.
  *
+ * The concept of the PageRouter is to *resolve*, and not build URIs. On top, it is a facade to hide the
+ * dependency to symfony and to not expose its logic.
+
  * @internal This API is not public yet and might change in the future, until TYPO3 v9 or TYPO3 v10.
  */
 class PageRouter
 {
     /**
      * @param ServerRequestInterface $request
-     * @param string $routePath
+     * @param string $routePathTail
      * @param SiteInterface $site
      * @param SiteLanguage $language
-     * @return array|null
+     * @return RouteResult
      */
-    public function matchRoute(ServerRequestInterface $request, string $routePath, SiteInterface $site, SiteLanguage $language): ?array
+    public function matchRoute(ServerRequestInterface $request, string $routePathTail, SiteInterface $site, SiteLanguage $language): RouteResult
     {
-        $slugCandidates = $this->getCandidateSlugsFromRoutePath($routePath);
+        $slugCandidates = $this->getCandidateSlugsFromRoutePath($routePathTail);
         if (empty($slugCandidates)) {
             return null;
         }
@@ -77,9 +80,9 @@ class PageRouter
         foreach ($pageCandidates ?? [] as $page) {
             $path = $page['slug'];
             $route = new Route(
-                $path . '{next}',
-                ['page' => $page, 'next' => ''],
-                ['next' => '.*'],
+                $path . '{tail}',
+                ['page' => $page, 'tail' => ''],
+                ['tail' => '.*'],
                 ['utf8' => true]
             );
             $collection->add('page_' . $page['uid'], $route);
@@ -88,10 +91,12 @@ class PageRouter
         $context = new RequestContext('/', $request->getMethod(), $request->getUri()->getHost());
         $matcher = new UrlMatcher($collection, $context);
         try {
-            return $matcher->match('/' . $routePath);
+            $result = $matcher->match('/' . $routePathTail);
+            return new RouteResult($request->getUri(), $site, $language, $result['tail'], $result);
         } catch (ResourceNotFoundException $e) {
-            return null;
+            // do nothing
         }
+        return new RouteResult($request->getUri(), $site, $language);
     }
 
     /**
diff --git a/typo3/sysext/core/Classes/Routing/RouteResult.php b/typo3/sysext/core/Classes/Routing/RouteResult.php
new file mode 100644 (file)
index 0000000..69037ec
--- /dev/null
@@ -0,0 +1,152 @@
+<?php
+declare(strict_types = 1);
+
+namespace TYPO3\CMS\Core\Routing;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+use Psr\Http\Message\UriInterface;
+use TYPO3\CMS\Core\Site\Entity\SiteInterface;
+use TYPO3\CMS\Core\Site\Entity\SiteLanguage;
+
+/**
+ * Class, usually available within request attribute "routing"
+ * containing all the findings of the Routers.
+ *
+ * @internal this API might change until 9 LTS.
+ */
+class RouteResult implements \ArrayAccess
+{
+    /**
+     * @var array
+     */
+    protected $validProperties = ['uri', 'site', 'language', 'tail'];
+
+    /**
+     * Incoming URI which was processed.
+     * @var UriInterface
+     */
+    protected $uri;
+
+    /**
+     * @var SiteInterface
+     */
+    protected $site;
+
+    /**
+     * @var SiteLanguage
+     */
+    protected $language;
+
+    /**
+     * data bag with additional attributes
+     * @var array
+     */
+    protected $data;
+
+    /**
+     * The leftover string of the path from the uri
+     * @var string
+     */
+    protected $tail;
+
+    public function __construct(UriInterface $uri, SiteInterface $site, SiteLanguage $language = null, string $tail = '', array $data = [])
+    {
+        $this->uri = $uri;
+        $this->site = $site;
+        $this->language = $language;
+        $this->tail = $tail;
+        $this->data = $data;
+    }
+
+    public function getUri(): UriInterface
+    {
+        return $this->uri;
+    }
+
+    public function getSite(): SiteInterface
+    {
+        return $this->site;
+    }
+
+    public function getLanguage(): ?SiteLanguage
+    {
+        return $this->language;
+    }
+
+    public function getTail(): string
+    {
+        return $this->tail;
+    }
+
+    public function offsetExists($offset): bool
+    {
+        return in_array($offset, $this->validProperties, true) || isset($this->data[$offset]);
+    }
+
+    /**
+     * @param mixed $offset
+     * @return mixed|UriInterface|string|SiteInterface|SiteLanguage
+     */
+    public function offsetGet($offset)
+    {
+        switch ($offset) {
+            case 'uri':
+                return $this->uri;
+            case 'site':
+                return $this->site;
+            case 'language':
+                return $this->language;
+            case 'tail':
+                return $this->tail;
+            default:
+                return $this->data[$offset];
+        }
+    }
+
+    public function offsetSet($offset, $value)
+    {
+        switch ($offset) {
+            case 'uri':
+                throw new \InvalidArgumentException('You can never replace the URI in a route result', 1535462423);
+            case 'site':
+                throw new \InvalidArgumentException('You can never replace the Site object in a route result', 1535462454);
+            case 'language':
+                throw new \InvalidArgumentException('You can never replace the Language object in a route result', 1535462452);
+            case 'tail':
+                $this->tail = $value;
+                break;
+            default:
+                $this->data[$offset] = $value;
+        }
+    }
+
+    public function offsetUnset($offset)
+    {
+        switch ($offset) {
+            case 'uri':
+                throw new \InvalidArgumentException('You can never replace the URI in a route result', 1535462429);
+            case 'site':
+                throw new \InvalidArgumentException('You can never replace the Site object in a route result', 1535462458);
+            case 'language':
+                $this->language = null;
+                break;
+            case 'tail':
+                $this->tail = '';
+                break;
+            default:
+                unset($this->data[$offset]);
+        }
+    }
+}
index b255b56..79a743a 100644 (file)
@@ -41,7 +41,10 @@ use TYPO3\CMS\Frontend\Page\PageRepository;
  * Symfony Routing to find the proper route with its defaults / attributes.
  *
  * On top, this is also commonly used throughout TYPO3 to fetch a site by a given pageId.
- * ->matchPageId()
+ * ->matchPageId().
+ *
+ * The concept of the SiteMatcher is to *resolve*, and not build URIs. On top, it is a facade to hide the
+ * dependency to symfony and to not expose its logic.
  */
 class SiteMatcher implements SingletonInterface
 {
@@ -77,9 +80,9 @@ class SiteMatcher implements SingletonInterface
      * a sys_domain record, and match against them.
      *
      * @param ServerRequestInterface $request
-     * @return array
+     * @return RouteResult
      */
-    public function matchRequest(ServerRequestInterface $request): array
+    public function matchRequest(ServerRequestInterface $request): RouteResult
     {
         $site = null;
         $language = null;
@@ -124,7 +127,8 @@ class SiteMatcher implements SingletonInterface
             );
             $matcher = new UrlMatcher($collection, $context);
             try {
-                return $matcher->match($request->getUri()->getPath());
+                $result = $matcher->match($request->getUri()->getPath());
+                return new RouteResult($request->getUri(), $result['site'], $result['language'], $result['tail']);
             } catch (NoConfigurationException | ResourceNotFoundException $e) {
                 // No site found
             }
@@ -139,19 +143,24 @@ class SiteMatcher implements SingletonInterface
             while (count($host)) {
                 $context->setHost(implode('.', $host));
                 try {
-                    return $matcher->match($request->getUri()->getPath());
+                    $result = $matcher->match($request->getUri()->getPath());
+                    return new RouteResult($request->getUri(), $result['site'], $result['language'], $result['tail']);
                 } catch (NoConfigurationException | ResourceNotFoundException $e) {
                     array_shift($host);
                 }
             }
         } else {
             try {
-                return $matcher->match($request->getUri()->getPath());
+                $result = $matcher->match($request->getUri()->getPath());
+                return new RouteResult($request->getUri(), $result['site'], $result['language'], $result['tail']);
             } catch (NoConfigurationException | ResourceNotFoundException $e) {
                 // No domain record found
             }
         }
-        return ['site' => $site, 'language' => $language];
+        if ($site == null) {
+            $site = $this->pseudoSiteFinder->getSiteByPageId(0);
+        }
+        return new RouteResult($request->getUri(), $site, $language);
     }
 
     /**
@@ -174,20 +183,31 @@ class SiteMatcher implements SingletonInterface
     /**
      * Returns a Symfony RouteCollection containing all routes to all sites.
      *
-     * {next} is not evaluated yet, but set as suffix and will change in the future.
-     *
      * @return RouteCollection
      */
     protected function getRouteCollectionForAllSites(): RouteCollection
     {
         $groupedRoutes = [];
         foreach ($this->finder->getAllSites() as $site) {
+            // Add the site as entrypoint
+            $urlParts = parse_url($site->getBase());
+            $route = new Route(
+                ($urlParts['path'] ?? '/') . '{tail}',
+                ['site' => $site, 'language' => null, 'tail' => ''],
+                array_filter(['tail' => '.*', 'port' => (string)($urlParts['port'] ?? '')]),
+                ['utf8' => true],
+                $urlParts['host'] ?? '',
+                !empty($urlParts['scheme']) ? [$urlParts['scheme']] : null
+            );
+            $identifier = 'site_' . $site->getIdentifier();
+            $groupedRoutes[($urlParts['scheme'] ?? '-') . ($urlParts['host'] ?? '-')][$urlParts['path'] ?? '/'][$identifier] = $route;
+            // Add all languages
             foreach ($site->getAllLanguages() as $siteLanguage) {
                 $urlParts = parse_url($siteLanguage->getBase());
                 $route = new Route(
-                    ($urlParts['path'] ?? '/') . '{next}',
-                    ['site' => $site, 'language' => $siteLanguage, 'next' => ''],
-                    array_filter(['next' => '.*', 'port' => (string)($urlParts['port'] ?? '')]),
+                    ($urlParts['path'] ?? '/') . '{tail}',
+                    ['site' => $site, 'language' => $siteLanguage, 'tail' => ''],
+                    array_filter(['tail' => '.*', 'port' => (string)($urlParts['port'] ?? '')]),
                     ['utf8' => true],
                     $urlParts['host'] ?? '',
                     !empty($urlParts['scheme']) ? [$urlParts['scheme']] : null
@@ -222,9 +242,9 @@ class SiteMatcher implements SingletonInterface
                 }
                 $urlParts = parse_url($domainName);
                 $route = new Route(
-                    ($urlParts['path'] ?? '/') . '{next}',
-                    ['site' => $site, 'language' => null, 'next' => ''],
-                    array_filter(['next' => '.*', 'port' => (string)($urlParts['port'] ?? '')]),
+                    ($urlParts['path'] ?? '/') . '{tail}',
+                    ['site' => $site, 'language' => null, 'tail' => ''],
+                    array_filter(['tail' => '.*', 'port' => (string)($urlParts['port'] ?? '')]),
                     ['utf8' => true],
                     $urlParts['host'] ?? '',
                     !empty($urlParts['scheme']) ? [$urlParts['scheme']] : null
@@ -237,7 +257,7 @@ class SiteMatcher implements SingletonInterface
     }
 
     /**
-     * As the {next} parameter is greedy, it needs to be ensured that the one with the
+     * As the {tail} parameter is greedy, it needs to be ensured that the one with the
      * most specific part matches first.
      *
      * @param array $groupedRoutes
index b0b56fb..08b3105 100644 (file)
@@ -28,7 +28,7 @@ be properly resolved which is a requirement to resolve the page path part
 of the URL.
 
 Note #2: If a page has the path segment "/team/about-us", but there is no
-other page with a path segment "/team/about-us/", then an automatic 301
+other page with a path segment "/team/about-us/", then an automatic 307
 HTTP redirect to the proper URI is triggered.
 
 Impact
index 5bf5ee1..e0df66b 100644 (file)
@@ -23,10 +23,17 @@ use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
 use TYPO3\CMS\Core\Context\Context;
 use TYPO3\CMS\Core\Context\UserAspect;
 use TYPO3\CMS\Core\Context\WorkspaceAspect;
+use TYPO3\CMS\Core\Http\RedirectResponse;
+use TYPO3\CMS\Core\Routing\PageRouter;
+use TYPO3\CMS\Core\Routing\RouteResult;
+use TYPO3\CMS\Core\Site\Entity\Site;
 use TYPO3\CMS\Core\Site\Entity\SiteInterface;
+use TYPO3\CMS\Core\Site\Entity\SiteLanguage;
 use TYPO3\CMS\Core\Type\Bitmask\Permission;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Frontend\Controller\ErrorController;
 use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
+use TYPO3\CMS\Frontend\Page\PageAccessFailureReasons;
 
 /**
  * Process the ID, type and other parameters.
@@ -38,6 +45,16 @@ use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
 class PageResolver implements MiddlewareInterface
 {
     /**
+     * @var TypoScriptFrontendController
+     */
+    protected $controller;
+
+    public function __construct(TypoScriptFrontendController $controller = null)
+    {
+        $this->controller = $controller ?? $GLOBALS['TSFE'];
+    }
+
+    /**
      * Resolve the page ID
      *
      * @param ServerRequestInterface $request
@@ -48,29 +65,87 @@ class PageResolver implements MiddlewareInterface
     {
         // First, resolve the root page of the site, the Page ID of the current domain
         if (($site = $request->getAttribute('site', null)) instanceof SiteInterface) {
-            $GLOBALS['TSFE']->domainStartPage = $site->getRootPageId();
+            $this->controller->domainStartPage = $site->getRootPageId();
+        }
+        $language = $request->getAttribute('language', null);
+
+        $hasSiteConfiguration = $language instanceof SiteLanguage && $site instanceof Site;
+
+        // Resolve the page ID based on TYPO3's native routing functionality
+        if ($hasSiteConfiguration) {
+            /** @var RouteResult $previousResult */
+            $previousResult = $request->getAttribute('routing', new RouteResult($request->getUri(), $site, $language));
+            if (!empty($previousResult->getTail())) {
+                // Check for the route
+                $routeResult = $this->getPageRouter()->matchRoute($request, $previousResult->getTail(), $site, $language);
+                $request = $request->withAttribute('routing', $routeResult);
+                if (is_array($routeResult['page'])) {
+                    $page = $routeResult['page'];
+                    $this->controller->id = (int)($page['l10n_parent'] > 0 ? $page['l10n_parent'] : $page['uid']);
+                    $tail = $routeResult->getTail();
+                    $requestedUri = $request->getUri();
+                    // the request was called with "/my-page" but it's actually called "/my-page/", let's do a redirect
+                    if ($tail === '' && substr($requestedUri->getPath(), -1) !== substr($page['slug'], -1)) {
+                        $uri = $requestedUri->withPath($requestedUri->getPath() . '/');
+                        return new RedirectResponse($uri, 307);
+                    }
+                    if ($tail === '/') {
+                        $uri = $requestedUri->withPath(rtrim($requestedUri->getPath(), '/'));
+                        return new RedirectResponse($uri, 307);
+                    }
+                    if (!empty($tail)) {
+                        // @todo: kick in the resolvers for the RouteEnhancers at this point
+                        return GeneralUtility::makeInstance(ErrorController::class)->pageNotFoundAction(
+                            $request,
+                            'The requested page does not exist',
+                            ['code' => PageAccessFailureReasons::PAGE_NOT_FOUND]
+                        );
+                    }
+                } else {
+                    return GeneralUtility::makeInstance(ErrorController::class)->pageNotFoundAction(
+                        $request,
+                        'The requested page does not exist',
+                        ['code' => PageAccessFailureReasons::PAGE_NOT_FOUND]
+                    );
+                }
+                // At this point, we later get further route modifiers
+                // for bw-compat we update $GLOBALS[TYPO3_REQUEST] to be used later in TSFE.
+                $GLOBALS['TYPO3_REQUEST'] = $request;
+            }
+        } else {
+            // old-school page resolving for realurl, cooluri etc.
+            $this->controller->siteScript = $request->getAttribute('normalizedParams')->getSiteScript();
+            $this->checkAlternativeIdMethods($this->controller);
         }
 
-        $GLOBALS['TSFE']->siteScript = $request->getAttribute('normalizedParams')->getSiteScript();
-        $this->checkAlternativeIdMethods($GLOBALS['TSFE']);
-        $GLOBALS['TSFE']->determineId();
+        $this->controller->determineId();
 
         // No access? Then remove user & Re-evaluate the page-id
-        if ($GLOBALS['TSFE']->isBackendUserLoggedIn() && !$GLOBALS['BE_USER']->doesUserHaveAccess($GLOBALS['TSFE']->page, Permission::PAGE_SHOW)) {
+        if ($this->controller->isBackendUserLoggedIn() && !$GLOBALS['BE_USER']->doesUserHaveAccess($this->controller->page, Permission::PAGE_SHOW)) {
             unset($GLOBALS['BE_USER']);
             // Register an empty backend user as aspect
             $this->setBackendUserAspect(GeneralUtility::makeInstance(Context::class), null);
-            $this->checkAlternativeIdMethods($GLOBALS['TSFE']);
-            $GLOBALS['TSFE']->determineId();
+            if (!$hasSiteConfiguration) {
+                $this->checkAlternativeIdMethods($this->controller);
+            }
+            $this->controller->determineId();
         }
 
         // Evaluate the cache hash parameter
-        $GLOBALS['TSFE']->makeCacheHash();
+        $this->controller->makeCacheHash();
 
         return $handler->handle($request);
     }
 
     /**
+     * @return PageRouter
+     */
+    protected function getPageRouter(): PageRouter
+    {
+        return GeneralUtility::makeInstance(PageRouter::class);
+    }
+
+    /**
      * Provides ways to bypass the '?id=[xxx]&type=[xx]' format, using either PATH_INFO or Server Rewrites
      *
      * Two options:
index 5de6c3b..3679208 100644 (file)
@@ -21,9 +21,8 @@ use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
 use TYPO3\CMS\Core\Http\RedirectResponse;
-use TYPO3\CMS\Core\Routing\PageRouter;
+use TYPO3\CMS\Core\Http\Uri;
 use TYPO3\CMS\Core\Routing\SiteMatcher;
-use TYPO3\CMS\Core\Site\Entity\PseudoSite;
 use TYPO3\CMS\Core\Site\Entity\Site;
 use TYPO3\CMS\Core\Site\Entity\SiteLanguage;
 use TYPO3\CMS\Core\Site\SiteFinder;
@@ -65,78 +64,49 @@ class SiteResolver implements MiddlewareInterface
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
     {
         $routeResult = $this->matcher->matchRequest($request);
-        $site = $routeResult['site'] ?? null;
-        $language = $routeResult['language'] ?? null;
-        $routePath = $routeResult['next'] ?? '';
+        $site = $routeResult->getSite();
+        $language = $routeResult->getLanguage();
 
-        // language is found, and hidden but also not visible to the BE user, this needs to fail
-        if ($language instanceof SiteLanguage && !$this->isLanguageEnabled($language, $GLOBALS['BE_USER'] ?? null)) {
-            $request = $request->withAttribute('site', $site);
-            return GeneralUtility::makeInstance(ErrorController::class)->pageNotFoundAction(
-                $request,
-                'Page is not available in the requested language.',
-                ['code' => PageAccessFailureReasons::LANGUAGE_NOT_AVAILABLE]
-            );
-        }
-
-        // Add language+site information to the PSR-7 request object.
-        if ($language instanceof SiteLanguage && $site instanceof Site) {
-            $request = $request->withAttribute('site', $site);
-            $request = $request->withAttribute('language', $language);
-            $queryParams = $request->getQueryParams();
-            // necessary to calculate the proper hash base
-            $queryParams['L'] = $language->getLanguageId();
-            $request = $request->withQueryParams($queryParams);
-            $_GET['L'] = $queryParams['L'];
+        $request = $request->withAttribute('site', $site);
+        $request = $request->withAttribute('routing', $routeResult);
 
-            $routePath = ltrim($routePath, '/');
-            if (!empty($routePath)) {
-                // Check for the route
-                $routeResult = $this->getPageRouter()
-                    ->matchRoute($request, $routePath, $site, $language);
-                if (is_array($routeResult)) {
-                    $page = $routeResult['page'];
-                    $pageId = (int)($page['l10n_parent'] > 0 ? $page['l10n_parent'] : $page['uid']);
-                    // @todo: we could move the middleware earlier which will make A LOT OF things easier
-                    $GLOBALS['TSFE']->id = $pageId;
-                    $_GET['id'] = $pageId;
-                    $queryParams = $request->getQueryParams();
-                    $queryParams['id'] = $pageId;
-                    $request = $request->withQueryParams($queryParams);
-                    if (!empty($routeResult['next'] ?? '')) {
-                        if ($routeResult['next'] === '/') {
-                            // a URL was called via "/mysite/" but the page is actually called "/mysite"
-                            // let's do a redirect
-                            $uri = $request->getUri();
-                            $path = rtrim($uri->getPath(), '/');
-                            $uri = $uri->withPath($path);
-                            return new RedirectResponse($uri, 301);
-                        }
-                        // @todo: kick in the resolvers for the RouteEnhancers at this point
-                        return GeneralUtility::makeInstance(ErrorController::class)->pageNotFoundAction(
-                            $request,
-                            'The requested page does not exist',
-                            ['code' => PageAccessFailureReasons::PAGE_NOT_FOUND]
-                        );
-                    }
-                } else {
-                    return GeneralUtility::makeInstance(ErrorController::class)->pageNotFoundAction(
-                        $request,
-                        'The requested page does not exist',
-                        ['code' => PageAccessFailureReasons::PAGE_NOT_FOUND]
-                    );
-                }
+        // Usually called when "https://www.example.com" was entered, but all sites have "https://www.example.com/lang-key/"
+        // So a redirect to the first possible language is done.
+        if ($site instanceof Site && !($language instanceof SiteLanguage)) {
+            $language = $site->getDefaultLanguage();
+            $uri = new Uri($language->getBase());
+            return new RedirectResponse($uri, 307);
+        }
+        // language is found, and hidden but also not visible to the BE user, this needs to fail
+        if ($language instanceof SiteLanguage) {
+            if (!$this->isLanguageEnabled($language, $GLOBALS['BE_USER'] ?? null)) {
+                return GeneralUtility::makeInstance(ErrorController::class)->pageNotFoundAction(
+                    $request,
+                    'Page is not available in the requested language.',
+                    ['code' => PageAccessFailureReasons::LANGUAGE_NOT_AVAILABLE]
+                );
+            }
+            $requestedUri = $request->getUri();
+            $tail = $routeResult->getTail();
+            // a URL was called via "/fr-FR/" but the page is actually called "/fr-FR", let's do a redirect
+            if ($tail === '/') {
+                $uri = $requestedUri->withPath(rtrim($requestedUri->getPath(), '/'));
+                return new RedirectResponse($uri, 307);
+            }
+            // Request was "/fr-FR" but the site is actually called "/fr-FR/", let's do a redirect
+            if ($tail === '' && (string)(new Uri($language->getBase()))->getPath() !== (string)$requestedUri->getPath()) {
+                $uri = $requestedUri->withPath($requestedUri->getPath() . '/');
+                return new RedirectResponse($uri, 307);
             }
-        } elseif ($site instanceof PseudoSite) {
-            $request = $request->withAttribute('site', $site);
+            $request = $request->withAttribute('language', $language);
         }
+
         // At this point, we later get further route modifiers
         // for bw-compat we update $GLOBALS[TYPO3_REQUEST] to be used later in TSFE.
         $GLOBALS['TYPO3_REQUEST'] = $request;
 
         return $handler->handle($request);
     }
-
     /**
      * Checks if the language is allowed in Frontend, if not, check if there is valid BE user
      *
@@ -152,12 +122,4 @@ class SiteResolver implements MiddlewareInterface
         }
         return false;
     }
-
-    /**
-     * @return PageRouter
-     */
-    protected function getPageRouter(): PageRouter
-    {
-        return GeneralUtility::makeInstance(PageRouter::class);
-    }
 }
index 0c0c54d..1a21b9d 100644 (file)
@@ -136,7 +136,6 @@ class SiteRequestTest extends AbstractTestCase
         ];
 
         $languagePaths = [
-            '',
             'en-en/',
             'fr-fr/',
             'fr-ca/',
@@ -211,7 +210,8 @@ class SiteRequestTest extends AbstractTestCase
     public function pageIsRenderedWithDomainsDataProvider(): array
     {
         $domainPaths = [
-            'https://website.local/',
+            // @todo: This turns into a redirect to the default language (".us") making this function obsolete
+            // 'https://website.local/',
             'https://website.us/',
             'https://website.fr/',
             'https://website.ca/',
diff --git a/typo3/sysext/frontend/Tests/Unit/Middleware/PageResolverTest.php b/typo3/sysext/frontend/Tests/Unit/Middleware/PageResolverTest.php
new file mode 100644 (file)
index 0000000..7cf6ac1
--- /dev/null
@@ -0,0 +1,190 @@
+<?php
+declare(strict_types = 1);
+
+namespace TYPO3\CMS\Frontend\Tests\Unit\Middleware;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Server\RequestHandlerInterface;
+use TYPO3\CMS\Core\Http\JsonResponse;
+use TYPO3\CMS\Core\Http\NullResponse;
+use TYPO3\CMS\Core\Http\ServerRequest;
+use TYPO3\CMS\Core\Routing\PageRouter;
+use TYPO3\CMS\Core\Routing\RouteResult;
+use TYPO3\CMS\Core\Site\Entity\Site;
+use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
+use TYPO3\CMS\Frontend\Middleware\PageResolver;
+use TYPO3\TestingFramework\Core\AccessibleObjectInterface;
+use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
+
+class PageResolverTest extends UnitTestCase
+{
+    /**
+     * @var bool Reset singletons created by subject
+     */
+    protected $resetSingletonInstances = true;
+
+    /**
+     * @var TypoScriptFrontendController|AccessibleObjectInterface
+     */
+    protected $controller;
+
+    /**
+     * @var RequestHandlerInterface
+     */
+    protected $responseOutputHandler;
+
+    /**
+     * @var PageResolver|AccessibleObjectInterface
+     */
+    protected $subject;
+
+    protected function setUp(): void
+    {
+        $this->controller = $this->getAccessibleMock(TypoScriptFrontendController::class, ['getSiteScript', 'makeCacheHash', 'determineId', 'isBackendUserLoggedIn'], [], '', false);
+
+        // A request handler which expects a site with some more details are found.
+        $this->responseOutputHandler = new class implements RequestHandlerInterface {
+            public function handle(ServerRequestInterface $request): ResponseInterface
+            {
+                /** @var RouteResult $routeResult */
+                $routeResult = $request->getAttribute('routing', false);
+                if ($routeResult) {
+                    return new JsonResponse(
+                        [
+                            'site' => $routeResult->getSite()->getIdentifier(),
+                            'language-id' => $routeResult->getLanguage()->getLanguageId(),
+                            'tail' => $routeResult->getTail(),
+                            'page' => $routeResult['page']
+                        ]
+                    );
+                }
+                return new NullResponse();
+            }
+        };
+    }
+
+    /**
+     * @test
+     */
+    public function properSiteConfigurationLoadsPageRouter()
+    {
+        $incomingUrl = 'https://king.com/lotus-flower/en/mr-magpie/bloom';
+        $pageRecord = ['uid' => 13, 'l10n_parent' => 0, 'slug' => '/mr-magpie/bloom'];
+        $site = new Site('lotus-flower', 13, [
+            'base' => '/lotus-flower/',
+            'languages' => [
+                0 => [
+                    'languageId' => 0,
+                    'locale' => 'en_US.UTF-8',
+                    'base' => '/en/'
+                ],
+            ]
+        ]);
+        $language = $site->getDefaultLanguage();
+
+        $request = new ServerRequest($incomingUrl, 'GET');
+        $request = $request->withAttribute('site', $site);
+        $request = $request->withAttribute('language', $language);
+        $request = $request->withAttribute('routing', new RouteResult($request->getUri(), $site, $language, 'mr-magpie/bloom'));
+
+        $expectedRouteResult = new RouteResult($request->getUri(), $site, $language, '', ['page' => $pageRecord]);
+        $pageRouterMock = $this->getMockBuilder(PageRouter::class)->disableOriginalConstructor()->setMethods(['matchRoute'])->getMock();
+        $pageRouterMock->expects($this->once())->method('matchRoute')->willReturn($expectedRouteResult);
+
+        $subject = $this->getAccessibleMock(PageResolver::class, ['getPageRouter'], [$this->controller], '', true);
+        $subject->expects($this->any())->method('getPageRouter')->willReturn($pageRouterMock);
+        $response = $subject->process($request, $this->responseOutputHandler);
+        $result = $response->getBody()->getContents();
+        $result = json_decode($result, true);
+        $this->assertEquals('lotus-flower', $result['site']);
+        $this->assertEquals($pageRecord, $result['page']);
+    }
+
+    /**
+     * Ensures that a request with a trailing slash will be redirect to one without a trailing slash because the
+     * page slug does have a trailing slash.
+     * @test
+     */
+    public function properSiteConfigurationLoadsPageRouterWithRedirect()
+    {
+        $incomingUrl = 'https://king.com/lotus-flower/en/mr-magpie/bloom/';
+        $pageRecord = ['uid' => 13, 'l10n_parent' => 0, 'slug' => '/mr-magpie/bloom'];
+        $site = new Site('lotus-flower', 13, [
+            'base' => '/lotus-flower/',
+            'languages' => [
+                0 => [
+                    'languageId' => 0,
+                    'locale' => 'en_US.UTF-8',
+                    'base' => '/en/'
+                ],
+            ]
+        ]);
+        $language = $site->getDefaultLanguage();
+
+        $request = new ServerRequest($incomingUrl, 'GET');
+        $request = $request->withAttribute('site', $site);
+        $request = $request->withAttribute('language', $language);
+        $request = $request->withAttribute('routing', new RouteResult($request->getUri(), $site, $language, 'mr-magpie/bloom/'));
+
+        $expectedRouteResult = new RouteResult($request->getUri(), $site, $language, '/', ['page' => $pageRecord]);
+        $pageRouterMock = $this->getMockBuilder(PageRouter::class)->disableOriginalConstructor()->setMethods(['matchRoute'])->getMock();
+        $pageRouterMock->expects($this->once())->method('matchRoute')->willReturn($expectedRouteResult);
+
+        $subject = $this->getAccessibleMock(PageResolver::class, ['getPageRouter'], [$this->controller], '', true);
+        $subject->expects($this->any())->method('getPageRouter')->willReturn($pageRouterMock);
+        $response = $subject->process($request, $this->responseOutputHandler);
+        $this->assertEquals(307, $response->getStatusCode());
+        $this->assertEquals('https://king.com/lotus-flower/en/mr-magpie/bloom', $response->getHeader('Location')[0]);
+    }
+
+    /**
+     * Ensures that a request without a trailing slash will be redirect to one with a trailing slash because the
+     * page slug does not have a trailing slash.
+     * @test
+     */
+    public function properSiteConfigurationLoadsPageRouterWithRedirectWithoutTrailingSlash()
+    {
+        $incomingUrl = 'https://king.com/lotus-flower/en/mr-magpie/bloom';
+        $pageRecord = ['uid' => 13, 'l10n_parent' => 0, 'slug' => '/mr-magpie/bloom/'];
+        $site = new Site('lotus-flower', 13, [
+            'base' => '/lotus-flower/',
+            'languages' => [
+                0 => [
+                    'languageId' => 0,
+                    'locale' => 'en_US.UTF-8',
+                    'base' => '/en/'
+                ],
+            ]
+        ]);
+        $language = $site->getDefaultLanguage();
+
+        $request = new ServerRequest($incomingUrl, 'GET');
+        $request = $request->withAttribute('site', $site);
+        $request = $request->withAttribute('language', $language);
+        $request = $request->withAttribute('routing', new RouteResult($request->getUri(), $site, $language, 'mr-magpie/bloom/'));
+
+        $expectedRouteResult = new RouteResult($request->getUri(), $site, $language, '', ['page' => $pageRecord]);
+        $pageRouterMock = $this->getMockBuilder(PageRouter::class)->disableOriginalConstructor()->setMethods(['matchRoute'])->getMock();
+        $pageRouterMock->expects($this->once())->method('matchRoute')->willReturn($expectedRouteResult);
+
+        $subject = $this->getAccessibleMock(PageResolver::class, ['getPageRouter'], [$this->controller], '', true);
+        $subject->expects($this->any())->method('getPageRouter')->willReturn($pageRouterMock);
+        $response = $subject->process($request, $this->responseOutputHandler);
+        $this->assertEquals(307, $response->getStatusCode());
+        $this->assertEquals('https://king.com/lotus-flower/en/mr-magpie/bloom/', $response->getHeader('Location')[0]);
+    }
+}
index ea2c5dd..490515c 100644 (file)
@@ -16,9 +16,6 @@ namespace TYPO3\CMS\Frontend\Tests\Unit\Middleware;
  * The TYPO3 project - inspiring people to share!
  */
 
-use PHPUnit\Framework\MockObject\MockObject;
-use Prophecy\Argument;
-use Prophecy\Prophecy\ObjectProphecy;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\RequestHandlerInterface;
@@ -26,7 +23,6 @@ use TYPO3\CMS\Core\Cache\CacheManager;
 use TYPO3\CMS\Core\Http\JsonResponse;
 use TYPO3\CMS\Core\Http\NullResponse;
 use TYPO3\CMS\Core\Http\ServerRequest;
-use TYPO3\CMS\Core\Routing\PageRouter;
 use TYPO3\CMS\Core\Routing\SiteMatcher;
 use TYPO3\CMS\Core\Site\Entity\Site;
 use TYPO3\CMS\Core\Site\Entity\SiteLanguage;
@@ -55,16 +51,10 @@ class SiteResolverTest extends UnitTestCase
     protected $siteFoundRequestHandler;
 
     /**
-     * @var PageRouter|ObjectProphecy
-     */
-    protected $pageRouterProphecy;
-
-    /**
      * Set up
      */
     protected function setUp(): void
     {
-        $GLOBALS['TSFE'] = new \stdClass();
         $this->siteFinder = $this->getAccessibleMock(SiteFinder::class, ['dummy'], [], '', false);
 
         // A request handler which expects a site to be found.
@@ -89,9 +79,6 @@ class SiteResolverTest extends UnitTestCase
             }
         };
 
-        $this->pageRouterProphecy = $this->prophesize(PageRouter::class);
-        $this->pageRouterProphecy->matchRoute(Argument::cetera())->willReturn(['page' => ['uid' => 13, 'l10n_parent' => 0]]);
-
         $cacheManagerProphecy = $this->prophesize(CacheManager::class);
         GeneralUtility::setSingletonInstance(CacheManager::class, $cacheManagerProphecy->reveal());
     }
@@ -121,9 +108,7 @@ class SiteResolverTest extends UnitTestCase
             ])
         ]);
 
-        $subject = $this->createSiteResolverMock(
-            new SiteMatcher($this->siteFinder)
-        );
+        $subject = new SiteResolver(new SiteMatcher($this->siteFinder));
 
         $request = new ServerRequest($incomingUrl, 'GET');
         $response = $subject->process($request, $this->siteFoundRequestHandler);
@@ -176,9 +161,7 @@ class SiteResolverTest extends UnitTestCase
             ]),
         ]);
 
-        $subject = $this->createSiteResolverMock(
-            new SiteMatcher($this->siteFinder)
-        );
+        $subject = new SiteResolver(new SiteMatcher($this->siteFinder));
 
         $request = new ServerRequest($incomingUrl, 'GET');
         $response = $subject->process($request, $this->siteFoundRequestHandler);
@@ -268,9 +251,7 @@ class SiteResolverTest extends UnitTestCase
             ]),
         ]);
 
-        $subject = $this->createSiteResolverMock(
-            new SiteMatcher($this->siteFinder)
-        );
+        $subject = new SiteResolver(new SiteMatcher($this->siteFinder));
 
         $request = new ServerRequest($incomingUrl, 'GET');
         $response = $subject->process($request, $this->siteFoundRequestHandler);
@@ -303,13 +284,6 @@ class SiteResolverTest extends UnitTestCase
                 2,
                 '/mysubsite/'
             ],
-            'matches second site because third site language prefix did not match' => [
-                'https://www.random-result.com/mysubsite/micro-site/oh-yes-you-do/',
-                'sub-site',
-                14,
-                2,
-                '/mysubsite/'
-            ],
             'matches third site' => [
                 'https://www.random-result.com/mysubsite/micro-site/ru/oh-yes-you-do/',
                 'subsub-site',
@@ -317,24 +291,20 @@ class SiteResolverTest extends UnitTestCase
                 13,
                 '/mysubsite/micro-site/ru/'
             ],
-            /**
-             * This case does not work, as no language prefix is defined.
-            'matches a subsite in first site' => [
-                'https://www.random-result.com/products/pampers/',
+            'matches a subpage in first site' => [
+                'https://www.random-result.com/en/products/pampers/',
                 'outside-site',
                 13,
                 0,
-                '/'
+                '/en/'
             ],
-             * This case does not work as we now resolve the rest of the URL
-            'matches a subsite with translation in first site' => [
+            'matches a subpage with translation in first site' => [
                 'https://www.random-result.com/fr/products/pampers/',
                 'outside-site',
                 13,
                 1,
                 '/fr/'
             ],
-             */
         ];
     }
 
@@ -391,9 +361,7 @@ class SiteResolverTest extends UnitTestCase
             ]),
         ]);
 
-        $subject = $this->createSiteResolverMock(
-            new SiteMatcher($this->siteFinder)
-        );
+        $subject = new SiteResolver(new SiteMatcher($this->siteFinder));
 
         $request = new ServerRequest($incomingUrl, 'GET');
         $response = $subject->process($request, $this->siteFoundRequestHandler);
@@ -410,6 +378,72 @@ class SiteResolverTest extends UnitTestCase
         }
     }
 
+    public function doRedirectOnMissingOrSuperfluousRequestUrlDataProvider()
+    {
+        return [
+            'redirect to first language' => [
+                'https://twenty.one/',
+                'https://twenty.one/en/',
+            ],
+            'redirect to first language adding a slash' => [
+                'https://twenty.one/en',
+                'https://twenty.one/en/',
+            ],
+            'redirect to second language removing a slash' => [
+                'https://twenty.one/fr/',
+                'https://twenty.one/fr',
+            ],
+            'redirect to subsite by adding a slash' => [
+                'https://twenty.one/mysubsite',
+                'https://twenty.one/mysubsite/',
+            ],
+        ];
+    }
+
+    /**
+     * @param string $incomingUrl
+     * @param string $expectedRedirectUrl
+     * @dataProvider doRedirectOnMissingOrSuperfluousRequestUrlDataProvider
+     * @test
+     */
+    public function doRedirectOnMissingOrSuperfluousRequestUrl(string $incomingUrl, string $expectedRedirectUrl)
+    {
+        $this->siteFinder->_set('sites', [
+            'outside-site' => new Site('outside-site', 13, [
+                'base' => 'https://twenty.one/',
+                'languages' => [
+                    0 => [
+                        'languageId' => 0,
+                        'locale' => 'en_US.UTF-8',
+                        'base' => '/en/'
+                    ],
+                    1 => [
+                        'languageId' => 1,
+                        'locale' => 'fr_CA.UTF-8',
+                        'base' => '/fr'
+                    ]
+                ]
+            ]),
+            'sub-site' => new Site('sub-site', 14, [
+                'base' => 'https://twenty.one/mysubsite/',
+                'languages' => [
+                    2 => [
+                        'languageId' => 2,
+                        'locale' => 'it_IT.UTF-8',
+                        'base' => '/'
+                    ]
+                ]
+            ]),
+        ]);
+
+        $subject = new SiteResolver(new SiteMatcher($this->siteFinder));
+
+        $request = new ServerRequest($incomingUrl, 'GET');
+        $response = $subject->process($request, $this->siteFoundRequestHandler);
+        $this->assertEquals(307, $response->getStatusCode());
+        $this->assertEquals($expectedRedirectUrl, $response->getHeader('Location')[0] ?? '');
+    }
+
     /**
      * @return array
      */
@@ -459,30 +493,11 @@ class SiteResolverTest extends UnitTestCase
             ]),
         ]);
 
-        $subject = $this->createSiteResolverMock(
-            new SiteMatcher($this->siteFinder)
-        );
+        $subject = new SiteResolver(new SiteMatcher($this->siteFinder));
 
         // Request to default page
         $request = new ServerRequest($url, 'GET');
         $response = $subject->process($request, $this->siteFoundRequestHandler);
         static::assertEquals($expectedStatusCode, $response->getStatusCode());
     }
-
-    /**
-     * @param SiteMatcher|null $matcher
-     * @return MockObject|SiteResolver
-     */
-    private function createSiteResolverMock(SiteMatcher $matcher = null): MockObject
-    {
-        $mock = $this->getAccessibleMock(
-            SiteResolver::class,
-            ['getPageRouter'],
-            [$matcher]
-        );
-        $mock->expects(static::any())
-            ->method('getPageRouter')
-            ->willReturn($this->pageRouterProphecy->reveal());
-        return $mock;
-    }
 }