[TASK] Add proper exception handling to RouterInterface logic 19/58519/3
authorBenni Mack <benni@typo3.org>
Mon, 1 Oct 2018 10:42:06 +0000 (12:42 +0200)
committerSusanne Moog <susanne.moog@typo3.org>
Mon, 1 Oct 2018 13:39:27 +0000 (15:39 +0200)
Two new Exceptions are now thrown when routing does not
work, one being thrown when a URL is generated but could
not be generated, and one when a URL could not be resolved.

This is much cleaner than the distinction of a nullable return type,
so the new interface is adapted as well.

As a drive-by fix, the Backend routing exception now inherits from
this new exception.

Resolves: #86500
Releases: master
Change-Id: Ifaf7b61422dfd49df18399c3bcbdf735bc522cba
Reviewed-on: https://review.typo3.org/58519
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Susanne Moog <susanne.moog@typo3.org>
Tested-by: Susanne Moog <susanne.moog@typo3.org>
typo3/sysext/backend/Classes/Routing/Exception/RouteNotFoundException.php
typo3/sysext/backend/Classes/Utility/BackendUtility.php
typo3/sysext/core/Classes/Routing/InvalidRouteArgumentsException.php [new file with mode: 0644]
typo3/sysext/core/Classes/Routing/PageRouter.php
typo3/sysext/core/Classes/Routing/RouteNotFoundException.php [new file with mode: 0644]
typo3/sysext/core/Classes/Routing/RouterInterface.php
typo3/sysext/core/Classes/Routing/SiteMatcher.php
typo3/sysext/frontend/Classes/Middleware/PageResolver.php
typo3/sysext/frontend/Classes/Typolink/PageLinkBuilder.php
typo3/sysext/viewpage/Classes/Controller/ViewModuleController.php

index 79df4ea..09c53d0 100644 (file)
@@ -17,6 +17,6 @@ namespace TYPO3\CMS\Backend\Routing\Exception;
 /**
  * Exception thrown when a route does not exist
  */
-class RouteNotFoundException extends \TYPO3\CMS\Core\Exception
+class RouteNotFoundException extends \TYPO3\CMS\Core\Routing\RouteNotFoundException
 {
 }
index 54108e9..f63382f 100644 (file)
@@ -41,6 +41,7 @@ use TYPO3\CMS\Core\Resource\Exception\ResourceDoesNotExistException;
 use TYPO3\CMS\Core\Resource\File;
 use TYPO3\CMS\Core\Resource\ProcessedFile;
 use TYPO3\CMS\Core\Resource\ResourceFactory;
+use TYPO3\CMS\Core\Routing\InvalidRouteArgumentsException;
 use TYPO3\CMS\Core\Routing\RouterInterface;
 use TYPO3\CMS\Core\Routing\SiteMatcher;
 use TYPO3\CMS\Core\Site\Entity\PseudoSite;
@@ -2689,7 +2690,7 @@ class BackendUtility
                     $anchorSection,
                     RouterInterface::ABSOLUTE_URL
                 );
-            } catch (SiteNotFoundException $e) {
+            } catch (SiteNotFoundException | InvalidRouteArgumentsException $e) {
                 $previewUrl = self::createPreviewUrl($pageUid, $rootLine, $anchorSection, $additionalGetVars, $viewScript);
             }
         }
diff --git a/typo3/sysext/core/Classes/Routing/InvalidRouteArgumentsException.php b/typo3/sysext/core/Classes/Routing/InvalidRouteArgumentsException.php
new file mode 100644 (file)
index 0000000..d5250dd
--- /dev/null
@@ -0,0 +1,24 @@
+<?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!
+ */
+
+/**
+ * Exception thrown when a route does not exist or does not match the Route Arguments
+ */
+class InvalidRouteArgumentsException extends \TYPO3\CMS\Core\Exception
+{
+}
index b3dabed..92dfd6e 100644 (file)
@@ -108,8 +108,9 @@ class PageRouter implements RouterInterface
      * @param ServerRequestInterface $request
      * @param RouteResultInterface|SiteRouteResult|null $previousResult
      * @return SiteRouteResult
+     * @throws RouteNotFoundException
      */
-    public function matchRequest(ServerRequestInterface $request, RouteResultInterface $previousResult = null): ?RouteResultInterface
+    public function matchRequest(ServerRequestInterface $request, RouteResultInterface $previousResult = null): RouteResultInterface
     {
         $urlPath = $previousResult->getTail();
         $slugCandidates = $this->getCandidateSlugsFromRoutePath($urlPath ?: '/');
@@ -117,7 +118,7 @@ class PageRouter implements RouterInterface
         $pageCandidates = $this->getPagesFromDatabaseForCandidates($slugCandidates, $language->getLanguageId());
         // Stop if there are no candidates
         if (empty($pageCandidates)) {
-            return null;
+            throw new RouteNotFoundException('No page candidates found for path "' . $urlPath . '"', 1538389999);
         }
 
         $decoratedParameters = [];
@@ -157,9 +158,9 @@ class PageRouter implements RouterInterface
             $matchedRoute->setOption('_decoratedParameters', $decoratedParameters);
             return $this->buildPageArguments($matchedRoute, $result, $request->getQueryParams());
         } catch (ResourceNotFoundException $e) {
-            // return nothing
+            // Do nothing
         }
-        return null;
+        throw new RouteNotFoundException('No route found for path "' . $urlPath . '"', 1538389998);
     }
 
     /**
@@ -265,15 +266,17 @@ class PageRouter implements RouterInterface
             }
         }
 
+        if (!$uri instanceof UriInterface) {
+            throw new InvalidRouteArgumentsException('Uri could not be built for page "' . $pageId . '"', 1538390230);
+        }
+
         if ($pageRouteResult && $pageRouteResult->areDirty()) {
             // for generating URLs this should(!) never happen
             // if it does happen, generator logic has flaws
-            throw new \OverflowException('Route arguments are dirty', 1537613247);
+            throw new InvalidRouteArgumentsException('Route arguments are dirty', 1537613247);
         }
 
-        if ($matchedRoute && $pageRouteResult && $uri instanceof UriInterface
-            && !empty($pageRouteResult->getDynamicArguments())
-        ) {
+        if ($matchedRoute && $pageRouteResult && !empty($pageRouteResult->getDynamicArguments())) {
             $cacheHash = $this->generateCacheHash($pageId, $pageRouteResult);
 
             if (!empty($cacheHash)) {
@@ -285,7 +288,6 @@ class PageRouter implements RouterInterface
         if ($fragment) {
             $uri = $uri->withFragment($fragment);
         }
-        // @todo Throw exception in case $uri is null
         return $uri;
     }
 
diff --git a/typo3/sysext/core/Classes/Routing/RouteNotFoundException.php b/typo3/sysext/core/Classes/Routing/RouteNotFoundException.php
new file mode 100644 (file)
index 0000000..30f3515
--- /dev/null
@@ -0,0 +1,24 @@
+<?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!
+ */
+
+/**
+ * Exception thrown when a route does not exist
+ */
+class RouteNotFoundException extends \TYPO3\CMS\Core\Exception
+{
+}
index 73b07ee..8a0e9a4 100644 (file)
@@ -38,9 +38,10 @@ interface RouterInterface
     /**
      * @param ServerRequestInterface $request
      * @param RouteResultInterface|null $previousResult
-     * @return RouteResultInterface|null if no route has matched, an empty RouteResult is returned.
+     * @return RouteResultInterface
+     * @throws RouteNotFoundException
      */
-    public function matchRequest(ServerRequestInterface $request, RouteResultInterface $previousResult = null): ?RouteResultInterface;
+    public function matchRequest(ServerRequestInterface $request, RouteResultInterface $previousResult = null): RouteResultInterface;
 
     /**
      * Builds a URI based on the $route and the given parameters.
@@ -50,6 +51,7 @@ interface RouterInterface
      * @param string $fragment the section/fragment www.example.com/page/#fragment, WITHOUT the hash
      * @param string $type see the constants above.
      * @return UriInterface
+     * @throws InvalidRouteArgumentsException
      */
     public function generateUri($route, array $parameters = [], string $fragment = '', string $type = self::ABSOLUTE_URL): UriInterface;
 }
index a3d692b..a5b5baf 100644 (file)
@@ -43,6 +43,8 @@ use TYPO3\CMS\Frontend\Page\PageRepository;
  *
  * 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.
+ *
+ * @internal Please note that the site matcher will be probably cease to exist and adapted to the SiteFinder concept when Pseudo-Site handling will be removed.
  */
 class SiteMatcher implements SingletonInterface
 {
index 6a7ccb5..6e8787e 100644 (file)
@@ -27,6 +27,7 @@ use TYPO3\CMS\Core\Database\ConnectionPool;
 use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction;
 use TYPO3\CMS\Core\Database\Query\Restriction\FrontendWorkspaceRestriction;
 use TYPO3\CMS\Core\Routing\PageArguments;
+use TYPO3\CMS\Core\Routing\RouteNotFoundException;
 use TYPO3\CMS\Core\Routing\SiteRouteResult;
 use TYPO3\CMS\Core\Site\Entity\Site;
 use TYPO3\CMS\Core\Site\Entity\SiteInterface;
@@ -98,9 +99,17 @@ class PageResolver implements MiddlewareInterface
                 );
             } else {
                 // Check for the route
-                $pageArguments = $site->getRouter()->matchRequest($request, $previousResult);
+                try {
+                    $pageArguments = $site->getRouter()->matchRequest($request, $previousResult);
+                } catch (RouteNotFoundException $e) {
+                    return GeneralUtility::makeInstance(ErrorController::class)->pageNotFoundAction(
+                        $request,
+                        'The requested page does not exist',
+                        ['code' => PageAccessFailureReasons::PAGE_NOT_FOUND]
+                    );
+                }
             }
-            if ($pageArguments === null || !$pageArguments->getPageId()) {
+            if (!$pageArguments->getPageId()) {
                 return GeneralUtility::makeInstance(ErrorController::class)->pageNotFoundAction(
                     $request,
                     'The requested page does not exist',
index 36607a5..b380587 100644 (file)
@@ -24,6 +24,7 @@ use TYPO3\CMS\Core\Database\ConnectionPool;
 use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction;
 use TYPO3\CMS\Core\Exception\Page\RootLineException;
 use TYPO3\CMS\Core\Exception\SiteNotFoundException;
+use TYPO3\CMS\Core\Routing\InvalidRouteArgumentsException;
 use TYPO3\CMS\Core\Routing\RouterInterface;
 use TYPO3\CMS\Core\Routing\SiteMatcher;
 use TYPO3\CMS\Core\Site\Entity\Site;
@@ -362,12 +363,16 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
 
         $targetPageId = (int)($page['l10n_parent'] > 0 ? $page['l10n_parent'] : $page['uid']);
         $queryParameters['_language'] = $siteLanguageOfTargetPage;
-        $uri = $siteOfTargetPage->getRouter()->generateUri(
-            $targetPageId,
-            $queryParameters,
-            $fragment,
-            $useAbsoluteUrl ? RouterInterface::ABSOLUTE_URL : RouterInterface::ABSOLUTE_PATH
-        );
+        try {
+            $uri = $siteOfTargetPage->getRouter()->generateUri(
+                $targetPageId,
+                $queryParameters,
+                $fragment,
+                $useAbsoluteUrl ? RouterInterface::ABSOLUTE_URL : RouterInterface::ABSOLUTE_PATH
+            );
+        } catch (InvalidRouteArgumentsException $e) {
+            throw new UnableToLinkException('The target page could not be linked. Error: ' . $e->getMessage(), 1535472406);
+        }
         // Override scheme, but only if the site does not define a scheme yet AND the site defines a domain/host
         if ($useAbsoluteUrl && !$uri->getScheme() && $uri->getHost()) {
             $scheme = $conf['forceAbsoluteUrl.']['scheme'] ?? 'https';
index 42e261b..0c6fa32 100644 (file)
@@ -218,7 +218,6 @@ class ViewModuleController
                 $additionalGetVars .= '&MP=' . $mountPointInformation['MPvar'];
             }
             $additionalGetVars .= $this->getTypeParameterIfSet($finalPageIdToShow);
-            /** @todo */
             if ($site instanceof Site) {
                 $additionalQueryParams = [];
                 parse_str($additionalGetVars, $additionalQueryParams);