Add Option to use NotFoundHttpException for 404 HTTP errors (#91)

* Add Option to use NotFoundHttpException for 404 HTTP errors * Add Option to use NotFoundHttpException for 404 HTTP errors (Test) * Add Option to use NotFoundHttpException for 404 HTTP errors (Add an E_USER_DEPRECATED if legacy handles the 404 error)
ezsystems · May 4, 2017 · 3515e9b · 3515e9b
1 parent c5ab5ab
commit 3515e9b
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 5 deletions.
diff --git a/bundle/DependencyInjection/Configuration.php b/bundle/DependencyInjection/Configuration.php
@@ -46,6 +46,9 @@ function ($v) {
     private function addSiteAccessSettings(NodeBuilder $nodeBuilder)
     {
         $nodeBuilder
+            ->booleanNode('not_found_http_conversion')
+                ->info('Whether to use 404 conversion or not. If true, will let symfony handle 404 errors.')
+            ->end()
             ->arrayNode('templating')
                 ->children()
                     ->scalarNode('view_layout')

diff --git a/bundle/DependencyInjection/EzPublishLegacyExtension.php b/bundle/DependencyInjection/EzPublishLegacyExtension.php
@@ -69,6 +69,10 @@ function (array $scopeSettings, $currentScope, ContextualizerInterface $contextu
                     $contextualizer->setContextualParameter('module_default_layout', $currentScope, $scopeSettings['templating']['module_layout']);
                 }
 
+                if (isset($scopeSettings['not_found_http_conversion'])) {
+                    $contextualizer->setContextualParameter('not_found_http_conversion', $currentScope, $scopeSettings['not_found_http_conversion']);
+                }
+
                 if (isset($scopeSettings['legacy_mode'])) {
                     $container = $contextualizer->getContainer();
                     $container->setParameter("ezsettings.$currentScope.legacy_mode", $scopeSettings['legacy_mode']);

diff --git a/bundle/LegacyResponse/LegacyResponseManager.php b/bundle/LegacyResponse/LegacyResponseManager.php
@@ -14,6 +14,7 @@
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Symfony\Component\Security\Core\Exception\AccessDeniedException;
 use Symfony\Component\Templating\EngineInterface;
 use DateTime;
@@ -46,6 +47,13 @@ class LegacyResponseManager
      */
     private $legacyMode;
 
+    /**
+     * Flag indicating if we're convert 404 errors into a NotFoundHttpException.
+     *
+     * @var bool
+     */
+    private $notFoundHttpConversion;
+
     /**
      * @var RequestStack
      */
@@ -55,6 +63,7 @@ public function __construct(EngineInterface $templateEngine, ConfigResolverInter
     {
         $this->templateEngine = $templateEngine;
         $this->legacyLayout = $configResolver->getParameter('module_default_layout', 'ezpublish_legacy');
+        $this->notFoundHttpConversion = $configResolver->getParameter('not_found_http_conversion', 'ezpublish_legacy');
         $this->legacyMode = $configResolver->getParameter('legacy_mode');
         $this->requestStack = $requestStack;
     }
@@ -64,6 +73,7 @@ public function __construct(EngineInterface $templateEngine, ConfigResolverInter
      *
      * @param \ezpKernelResult $result
      *
+     * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
      * @throws \Symfony\Component\Security\Core\Exception\AccessDeniedException
      * @return \eZ\Bundle\EzPublishLegacyBundle\LegacyResponse
      */
@@ -87,11 +97,26 @@ public function generateResponseFromModuleResult(ezpKernelResult $result)
 
         // Handling error codes sent by the legacy stack
         if (isset($moduleResult['errorCode'])) {
-            // If having an "Unauthorized" or "Forbidden" error code in non-legacy mode,
-            // we send an AccessDeniedException to be able to trigger redirection to login in Symfony stack.
-            if (!$this->legacyMode && ($moduleResult['errorCode'] == 401 || $moduleResult['errorCode'] == 403)) {
-                $errorMessage = isset($moduleResult['errorMessage']) ? $moduleResult['errorMessage'] : 'Access denied';
-                throw new AccessDeniedException($errorMessage);
+            if (!$this->legacyMode) {
+                // If having an "Unauthorized" or "Forbidden" error code in non-legacy mode,
+                // we send an AccessDeniedException to be able to trigger redirection to login in Symfony stack.
+                if ($moduleResult['errorCode'] == 401 || $moduleResult['errorCode'] == 403) {
+                    $errorMessage = isset($moduleResult['errorMessage']) ? $moduleResult['errorMessage'] : 'Access denied';
+                    throw new AccessDeniedException($errorMessage);
+                }
+                // If having an "Not found" error code in non-legacy mode and conversation is true,
+                // we send an NotFoundHttpException to be able to trigger error page in Symfony stack.
+                if ($moduleResult['errorCode'] == 404) {
+                    if ($this->notFoundHttpConversion) {
+                        $errorMessage = isset($moduleResult['errorMessage']) ? $moduleResult['errorMessage'] : 'Not found';
+                        throw new NotFoundHttpException($errorMessage);
+                    }
+                    @trigger_error(
+                        "Legacy 404 error handling is deprecated, and will be removed in legacy-bridge 2.0.\n" .
+                        'Use the not_found_http_conversion setting to use the new behavior and disable this notice.',
+                        E_USER_DEPRECATED
+                    );
+                }
             }
 
             $response->setStatusCode(

diff --git a/bundle/Resources/config/default_settings.yml b/bundle/Resources/config/default_settings.yml
@@ -7,4 +7,8 @@ parameters:
     # Pagelayout to use while rendering a content view in legacy
     ezpublish_legacy.default.view_default_layout: EzPublishLegacyBundle::legacy_view_default_pagelayout.html.twig
 
+    # Whether to use 404 conversion or not. If true, will let symfony handle 404 errors.
+    ezpublish_legacy.default.not_found_http_conversion: false
+
+    # Whether to use legacy mode or not. If true, will let the legacy kernel handle url aliases.
     ezsettings.default.legacy_mode: false
diff --git a/bundle/Tests/LegacyResponse/LegacyResponseManagerTest.php b/bundle/Tests/LegacyResponse/LegacyResponseManagerTest.php
@@ -66,6 +66,51 @@ public function generateResponseAccessDeniedProvider()
         );
     }
 
+    /**
+     * @param bool $legacyMode whether legacy mode is active or not
+     * @param bool $notFoundHttpConversion whether not found http conversion is active or not
+     * @param bool $expectException whether exception is expected
+     * @dataProvider generateResponseNotFoundProvider
+     */
+    public function testGenerateResponseNotFound($legacyMode, $notFoundHttpConversion, $expectException)
+    {
+        $this->configResolver
+            ->expects($this->any())
+            ->method('getParameter')
+            ->will(
+                $this->returnValueMap(
+                    array(
+                        array('legacy_mode', null, null, $legacyMode),
+                        array('not_found_http_conversion', 'ezpublish_legacy', null, $notFoundHttpConversion),
+                    )
+                )
+            );
+        if ($expectException) {
+            $this->setExpectedException('Symfony\Component\HttpKernel\Exception\NotFoundHttpException', 'Not found');
+        }
+        $manager = new LegacyResponseManager($this->templateEngine, $this->configResolver, new RequestStack());
+        $content = 'foobar';
+        $moduleResult = array(
+            'content' => $content,
+            'errorCode' => 404,
+            'errorMessage' => 'Not found',
+        );
+        $kernelResult = new ezpKernelResult($content, array('module_result' => $moduleResult));
+        $response = $manager->generateResponseFromModuleResult($kernelResult);
+        if (!$expectException) {
+            $this->assertSame($moduleResult['errorCode'], $response->getStatusCode());
+        }
+    }
+
+    public function generateResponseNotFoundProvider()
+    {
+        return array(
+            array(true, true, false),
+            array(false, true, true),
+            array(false, false, false),
+        );
+    }
+
     public function testLegacyResultHasLayout()
     {
         $requestStack = new RequestStack();