Merge pull request #1925 from wallabag/fix-redirect-without-referer

Redirect to homepage if referer is null

src/Wallabag/CoreBundle/Controller/EntryController.php

@@ -344,7 +344,9 @@ class EntryController extends Controller
             $message
         );
 
-        return $this->redirect($request->headers->get('referer'));
+        $redirectUrl = $this->get('wallabag_core.helper.redirect')->to($request->headers->get('referer'));
+
+        return $this->redirect($redirectUrl);
     }
 
     /**
@@ -374,7 +376,9 @@ class EntryController extends Controller
             $message
         );
 
-        return $this->redirect($request->headers->get('referer'));
+        $redirectUrl = $this->get('wallabag_core.helper.redirect')->to($request->headers->get('referer'));
+
+        return $this->redirect($redirectUrl);
     }
 
     /**
@@ -408,7 +412,11 @@ class EntryController extends Controller
         );
 
         // don't redirect user to the deleted entry
-        return $this->redirect($url !== $request->headers->get('referer') ? $request->headers->get('referer') : $this->generateUrl('homepage'));
+        $to = ($url !== $request->headers->get('referer') ? $request->headers->get('referer') : null);
+
+        $redirectUrl = $this->get('wallabag_core.helper.redirect')->to($to);
+
+        return $this->redirect($redirectUrl);
     }
 
     /**

src/Wallabag/CoreBundle/Controller/TagController.php

@@ -65,7 +65,9 @@ class TagController extends Controller
         }
         $em->flush();
 
-        return $this->redirect($request->headers->get('referer'));
+        $redirectUrl = $this->get('wallabag_core.helper.redirect')->to($request->headers->get('referer'));
+
+        return $this->redirect($redirectUrl);
     }
 
     /**

src/Wallabag/CoreBundle/Helper/Redirect.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace Wallabag\CoreBundle\Helper;
+
+use Symfony\Component\Routing\Router;
+
+/**
+ * Manage redirections to avoid redirecting to empty routes.
+ */
+class Redirect
+{
+    private $router;
+
+    public function __construct(Router $router)
+    {
+        $this->router = $router;
+    }
+
+    /**
+     * @param string $url      URL to redirect
+     * @param string $fallback Fallback URL if $url is null
+     *
+     * @return string
+     */
+    public function to($url, $fallback = '')
+    {
+        if (null !== $url) {
+            return $url;
+        }
+
+        if ('' === $fallback) {
+            return $this->router->generate('homepage');
+        }
+
+        return $fallback;
+    }
+}

src/Wallabag/CoreBundle/Resources/config/services.yml

@@ -114,3 +114,8 @@ services:
         class: Wallabag\CoreBundle\Operator\Doctrine\Matches
         tags:
             - { name: rulerz.operator, executor: rulerz.executor.doctrine, operator: matches, inline: true }
+
+    wallabag_core.helper.redirect:
+        class: Wallabag\CoreBundle\Helper\Redirect
+        arguments:
+            - "@router"

src/Wallabag/CoreBundle/Tests/Helper/RedirectTest.php

@@ -0,0 +1,55 @@
+<?php
+
+namespace Wallabag\CoreBundle\Tests\Helper;
+
+use Wallabag\CoreBundle\Helper\Redirect;
+
+class RedirectTest extends \PHPUnit_Framework_TestCase
+{
+    /** @var \PHPUnit_Framework_MockObject_MockObject */
+    private $routerMock;
+
+    /** @var Redirect */
+    private $redirect;
+
+    public function setUp()
+    {
+        $this->routerMock = $this->getRouterMock();
+        $this->redirect = new Redirect($this->routerMock);
+    }
+
+    public function testRedirectToNullWithFallback()
+    {
+        $redirectUrl = $this->redirect->to(null, 'fallback');
+
+        $this->assertEquals('fallback', $redirectUrl);
+    }
+
+    public function testRedirectToNullWithoutFallback()
+    {
+        $redirectUrl = $this->redirect->to(null);
+
+        $this->assertEquals($this->routerMock->generate('homepage'), $redirectUrl);
+    }
+
+    public function testRedirectToValidUrl()
+    {
+        $redirectUrl = $this->redirect->to('/unread/list');
+
+        $this->assertEquals('/unread/list', $redirectUrl);
+    }
+
+    private function getRouterMock()
+    {
+        $mock = $this->getMockBuilder('Symfony\Component\Routing\Router')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $mock->expects($this->any())
+            ->method('generate')
+            ->with('homepage')
+            ->willReturn('homepage');
+
+        return $mock;
+    }
+}