From af497a641c2a46c99bbc67215e041a46c91695bc Mon Sep 17 00:00:00 2001
From: =?utf8?q?Nicolas=20L=C5=93uillet?= <nicolas@loeuillet.org>
Date: Fri, 15 Apr 2016 07:58:01 +0200
Subject: [PATCH] Redirect to homepage if referer is null Fix #1924

---
 .../CoreBundle/Controller/EntryController.php | 14 ++++--
 .../CoreBundle/Controller/TagController.php   |  4 +-
 src/Wallabag/CoreBundle/Helper/Redirect.php   | 36 ++++++++++++++
 .../CoreBundle/Resources/config/services.yml  |  5 ++
 .../CoreBundle/Tests/Helper/RedirectTest.php  | 49 +++++++++++++++++++
 5 files changed, 104 insertions(+), 4 deletions(-)
 create mode 100644 src/Wallabag/CoreBundle/Helper/Redirect.php
 create mode 100644 src/Wallabag/CoreBundle/Tests/Helper/RedirectTest.php

diff --git a/src/Wallabag/CoreBundle/Controller/EntryController.php b/src/Wallabag/CoreBundle/Controller/EntryController.php
index 17b72bd1..9443ae82 100644
--- a/src/Wallabag/CoreBundle/Controller/EntryController.php
+++ b/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') : $this->generateUrl('homepage'));
+
+        $redirectUrl = $this->get('wallabag_core.helper.redirect')->to($to);
+
+        return $this->redirect($redirectUrl);
     }
 
     /**
diff --git a/src/Wallabag/CoreBundle/Controller/TagController.php b/src/Wallabag/CoreBundle/Controller/TagController.php
index e8e9ecbe..16d14d79 100644
--- a/src/Wallabag/CoreBundle/Controller/TagController.php
+++ b/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);
     }
 
     /**
diff --git a/src/Wallabag/CoreBundle/Helper/Redirect.php b/src/Wallabag/CoreBundle/Helper/Redirect.php
new file mode 100644
index 00000000..0921c3f9
--- /dev/null
+++ b/src/Wallabag/CoreBundle/Helper/Redirect.php
@@ -0,0 +1,36 @@
+<?php
+
+namespace Wallabag\CoreBundle\Helper;
+
+use Symfony\Component\Routing\Router;
+
+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 = '')
+    {
+        $returnUrl = $url;
+
+        if (null === $url) {
+            if ('' !== $fallback) {
+                $returnUrl = $fallback;
+            } else {
+                $returnUrl = $this->router->generate('homepage');
+            }
+        }
+
+        return $returnUrl;
+    }
+}
diff --git a/src/Wallabag/CoreBundle/Resources/config/services.yml b/src/Wallabag/CoreBundle/Resources/config/services.yml
index 6dc1f1d7..f8835198 100644
--- a/src/Wallabag/CoreBundle/Resources/config/services.yml
+++ b/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"
diff --git a/src/Wallabag/CoreBundle/Tests/Helper/RedirectTest.php b/src/Wallabag/CoreBundle/Tests/Helper/RedirectTest.php
new file mode 100644
index 00000000..da19cf58
--- /dev/null
+++ b/src/Wallabag/CoreBundle/Tests/Helper/RedirectTest.php
@@ -0,0 +1,49 @@
+<?php
+
+namespace Wallabag\CoreBundle\Tests\Helper;
+
+use Wallabag\CoreBundle\Helper\Redirect;
+
+class RedirectTest extends \PHPUnit_Framework_TestCase
+{
+    /** @var \Symfony\Component\Routing\Router */
+    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()
+    {
+        return $this->getMockBuilder('Symfony\Component\Routing\Router')
+            ->setMethods(['generate'])
+            ->disableOriginalConstructor()
+            ->getMock();
+    }
+}
-- 
2.41.0