diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index f41a7cbc3..9eee7c86f 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -45,11 +45,6 @@ parameters: count: 1 path: tests/Wallabag/CoreBundle/Controller/FeedControllerTest.php - - - message: "#^Call to method generate\\(\\) on an unknown class PHPUnit_Framework_MockObject_MockObject\\.$#" - count: 2 - path: tests/Wallabag/CoreBundle/Helper/RedirectTest.php - - message: "#^Property Tests\\\\Wallabag\\\\CoreBundle\\\\Helper\\\\RedirectTest\\:\\:\\$routerMock has unknown class PHPUnit_Framework_MockObject_MockObject as its type\\.$#" count: 1 diff --git a/src/Wallabag/CoreBundle/Controller/ConfigController.php b/src/Wallabag/CoreBundle/Controller/ConfigController.php index 196554231..4709a4d2e 100644 --- a/src/Wallabag/CoreBundle/Controller/ConfigController.php +++ b/src/Wallabag/CoreBundle/Controller/ConfigController.php @@ -32,6 +32,7 @@ use Wallabag\CoreBundle\Form\Type\IgnoreOriginUserRuleType; use Wallabag\CoreBundle\Form\Type\TaggingRuleImportType; use Wallabag\CoreBundle\Form\Type\TaggingRuleType; use Wallabag\CoreBundle\Form\Type\UserInformationType; +use Wallabag\CoreBundle\Helper\Redirect; use Wallabag\CoreBundle\Repository\ConfigRepository; use Wallabag\CoreBundle\Repository\EntryRepository; use Wallabag\CoreBundle\Repository\IgnoreOriginUserRuleRepository; @@ -48,8 +49,9 @@ class ConfigController extends AbstractController private TagRepository $tagRepository; private AnnotationRepository $annotationRepository; private ConfigRepository $configRepository; + private Redirect $redirectHelper; - public function __construct(EntityManagerInterface $entityManager, UserManagerInterface $userManager, EntryRepository $entryRepository, TagRepository $tagRepository, AnnotationRepository $annotationRepository, ConfigRepository $configRepository) + public function __construct(EntityManagerInterface $entityManager, UserManagerInterface $userManager, EntryRepository $entryRepository, TagRepository $tagRepository, AnnotationRepository $annotationRepository, ConfigRepository $configRepository, Redirect $redirectHelper) { $this->entityManager = $entityManager; $this->userManager = $userManager; @@ -57,6 +59,7 @@ class ConfigController extends AbstractController $this->tagRepository = $tagRepository; $this->annotationRepository = $annotationRepository; $this->configRepository = $configRepository; + $this->redirectHelper = $redirectHelper; } /** @@ -646,7 +649,9 @@ class ConfigController extends AbstractController $this->entityManager->persist($user); $this->entityManager->flush(); - return $this->redirect($request->getSession()->get('prevUrl')); + $redirectUrl = $this->redirectHelper->to($request->query->get('redirect')); + + return $this->redirect($redirectUrl); } /** diff --git a/src/Wallabag/CoreBundle/Controller/EntryController.php b/src/Wallabag/CoreBundle/Controller/EntryController.php index ca9ff56f3..9322860a6 100644 --- a/src/Wallabag/CoreBundle/Controller/EntryController.php +++ b/src/Wallabag/CoreBundle/Controller/EntryController.php @@ -15,7 +15,6 @@ use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Annotation\Route; -use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Contracts\Translation\TranslatorInterface; use Wallabag\CoreBundle\Entity\Entry; use Wallabag\CoreBundle\Entity\Tag; @@ -128,7 +127,7 @@ class EntryController extends AbstractController $this->entityManager->flush(); } - $redirectUrl = $this->redirectHelper->to($request->getSession()->get('prevUrl')); + $redirectUrl = $this->redirectHelper->to($request->query->get('redirect')); return $this->redirect($redirectUrl); } @@ -390,7 +389,6 @@ class EntryController extends AbstractController public function viewAction(Entry $entry) { $this->checkUserAction($entry); - $this->get('session')->set('prevUrl', $this->generateUrl('view', ['id' => $entry->getId()])); return $this->render( '@WallabagCore/Entry/entry.html.twig', @@ -452,7 +450,7 @@ class EntryController extends AbstractController $message ); - $redirectUrl = $this->redirectHelper->to($request->getSession()->get('prevUrl')); + $redirectUrl = $this->redirectHelper->to($request->query->get('redirect')); return $this->redirect($redirectUrl); } @@ -482,7 +480,7 @@ class EntryController extends AbstractController $message ); - $redirectUrl = $this->redirectHelper->to($request->getSession()->get('prevUrl')); + $redirectUrl = $this->redirectHelper->to($request->query->get('redirect')); return $this->redirect($redirectUrl); } @@ -502,8 +500,7 @@ class EntryController extends AbstractController // to avoid redirecting to the deleted entry. Ugh. $url = $this->generateUrl( 'view', - ['id' => $entry->getId()], - UrlGeneratorInterface::ABSOLUTE_PATH + ['id' => $entry->getId()] ); // entry deleted, dispatch event about it! @@ -518,7 +515,7 @@ class EntryController extends AbstractController ); // don't redirect user to the deleted entry (check that the referer doesn't end with the same url) - $prev = $request->getSession()->get('prevUrl'); + $prev = $request->query->get('redirect', ''); $to = (1 !== preg_match('#' . $url . '$#i', $prev) ? $prev : null); $redirectUrl = $this->redirectHelper->to($to); @@ -617,7 +614,6 @@ class EntryController extends AbstractController { $searchTerm = (isset($request->get('search_entry')['term']) ? $request->get('search_entry')['term'] : ''); $currentRoute = (null !== $request->query->get('currentRoute') ? $request->query->get('currentRoute') : ''); - $request->getSession()->set('prevUrl', $request->getRequestUri()); $formOptions = []; diff --git a/src/Wallabag/CoreBundle/Controller/TagController.php b/src/Wallabag/CoreBundle/Controller/TagController.php index d0e5a5aa6..804512563 100644 --- a/src/Wallabag/CoreBundle/Controller/TagController.php +++ b/src/Wallabag/CoreBundle/Controller/TagController.php @@ -104,7 +104,7 @@ class TagController extends AbstractController $this->entityManager->flush(); } - $redirectUrl = $this->redirectHelper->to($request->getSession()->get('prevUrl'), '', true); + $redirectUrl = $this->redirectHelper->to($request->query->get('redirect'), true); return $this->redirect($redirectUrl); } @@ -185,7 +185,7 @@ class TagController extends AbstractController $form = $this->createForm(RenameTagType::class, new Tag()); $form->handleRequest($request); - $redirectUrl = $this->redirectHelper->to($request->getSession()->get('prevUrl'), '', true); + $redirectUrl = $this->redirectHelper->to($request->query->get('redirect'), true); if ($form->isSubmitted() && $form->isValid()) { $newTag = new Tag(); @@ -257,7 +257,7 @@ class TagController extends AbstractController $this->entityManager->flush(); } - return $this->redirect($this->redirectHelper->to($request->getSession()->get('prevUrl'), '', true)); + return $this->redirect($this->redirectHelper->to($request->query->get('redirect'), true)); } /** @@ -279,7 +279,7 @@ class TagController extends AbstractController $this->entityManager->remove($tag); $this->entityManager->flush(); } - $redirectUrl = $this->redirectHelper->to($request->getSession()->get('prevUrl'), '', true); + $redirectUrl = $this->redirectHelper->to($request->query->get('redirect'), true); return $this->redirect($redirectUrl); } diff --git a/src/Wallabag/CoreBundle/Helper/Redirect.php b/src/Wallabag/CoreBundle/Helper/Redirect.php index 99bc192a6..dd0a61a6a 100644 --- a/src/Wallabag/CoreBundle/Helper/Redirect.php +++ b/src/Wallabag/CoreBundle/Helper/Redirect.php @@ -2,6 +2,7 @@ namespace Wallabag\CoreBundle\Helper; +use GuzzleHttp\Psr7\Uri; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Wallabag\CoreBundle\Entity\Config; @@ -23,16 +24,23 @@ class Redirect /** * @param string $url URL to redirect - * @param string $fallback Fallback URL if $url is null * @param bool $ignoreActionMarkAsRead Ignore configured action when mark as read * * @return string */ - public function to($url, $fallback = '', $ignoreActionMarkAsRead = false) + public function to($url, $ignoreActionMarkAsRead = false) { $user = $this->tokenStorage->getToken() ? $this->tokenStorage->getToken()->getUser() : null; if (!$user instanceof User) { + if (null === $url) { + return $this->router->generate('homepage'); + } + + if (!Uri::isAbsolutePathReference(new Uri($url))) { + return $this->router->generate('homepage'); + } + return $url; } @@ -41,14 +49,14 @@ class Redirect return $this->router->generate('homepage'); } - if (null !== $url) { - return $url; - } - - if ('' === $fallback) { + if (null === $url) { return $this->router->generate('homepage'); } - return $fallback; + if (!Uri::isAbsolutePathReference(new Uri($url))) { + return $this->router->generate('homepage'); + } + + return $url; } } diff --git a/src/Wallabag/CoreBundle/Resources/views/Entry/_card_actions.html.twig b/src/Wallabag/CoreBundle/Resources/views/Entry/_card_actions.html.twig index 26e78215b..d77a48ec0 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/_card_actions.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/_card_actions.html.twig @@ -7,18 +7,20 @@ + {% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %} + diff --git a/src/Wallabag/CoreBundle/Resources/views/Entry/_card_list.html.twig b/src/Wallabag/CoreBundle/Resources/views/Entry/_card_list.html.twig index 78e97ea37..4e112e774 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/_card_list.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/_card_list.html.twig @@ -9,12 +9,15 @@ {% endif %} {% include "@WallabagCore/Entry/Card/_content.html.twig" with {'entry': entry, 'withMetadata': true, 'subClass': 'metadata'} only %} + + {% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %} + diff --git a/src/Wallabag/CoreBundle/Resources/views/Entry/_tags.html.twig b/src/Wallabag/CoreBundle/Resources/views/Entry/_tags.html.twig index b6c84d959..2ab67e1c8 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/_tags.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/_tags.html.twig @@ -4,7 +4,8 @@
  • {{ tag.label }} {% if withRemove is defined and withRemove == true %} - + {% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %} + delete {% endif %} diff --git a/src/Wallabag/CoreBundle/Resources/views/Entry/entries.html.twig b/src/Wallabag/CoreBundle/Resources/views/Entry/entries.html.twig index 95023e714..e01d614ee 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/entries.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/entries.html.twig @@ -19,18 +19,19 @@ {% endblock %} {% block content %} + {% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %} {% set list_mode = app.user.config.listMode %} {% set entries_with_archived_class_routes = ['tag_entries', 'search', 'all'] %} {% set current_route = app.request.attributes.get('_route') %} {% if current_route == 'homepage' %} {% set current_route = 'unread' %} {% endif %} -
    +
    {{ 'entry.list.number_on_the_page'|trans({'%count%': entries.count}) }} {% if entries.count > 0 %} - {% if list_mode == 0 %}view_list{% else %}view_module{% endif %} + {% if list_mode == 0 %}view_list{% else %}view_module{% endif %} {% endif %} {% if entries.count > 0 %} @@ -39,7 +40,7 @@ {% include "@WallabagCore/Entry/_feed_link.html.twig" %} {% endif %}
    - {% if current_route == 'search' %}
    {{ 'entry.list.assign_search_tag'|trans }}
    {% endif %} + {% if current_route == 'search' %}
    {{ 'entry.list.assign_search_tag'|trans }}
    {% endif %} {% if entries.getNbPages > 1 %} {{ pagerfanta(entries, 'default_wallabag') }} {% endif %} diff --git a/src/Wallabag/CoreBundle/Resources/views/Entry/entry.html.twig b/src/Wallabag/CoreBundle/Resources/views/Entry/entry.html.twig index 7ae98bf94..9ce783bbf 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/entry.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/entry.html.twig @@ -4,6 +4,8 @@ {% block body_class %}entry{% endblock %} +{% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %} + {% block menu %}
    diff --git a/src/Wallabag/CoreBundle/Resources/views/Tag/tags.html.twig b/src/Wallabag/CoreBundle/Resources/views/Tag/tags.html.twig index f951fcd65..bff22436e 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Tag/tags.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Tag/tags.html.twig @@ -3,6 +3,8 @@ {% block title %}{{ 'tag.page_title'|trans }}{% endblock %} {% block content %} + {% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %} +
    {{ 'tag.list.number_on_the_page'|trans({'%count%': tags|length}) }}
    @@ -18,7 +20,7 @@ {{ tag.label }} ({{ tag.nbEntries }}) {% if renameForms is defined and renameForms[tag.id] is defined %} - + {{ form_widget(renameForms[tag.id].label, {'attr': {'value': tag.label}}) }} {{ form_rest(renameForms[tag.id]) }}
    @@ -26,7 +28,7 @@ mode_edit {% endif %} - + delete {% if app.user.config.feedToken %} diff --git a/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php b/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php index a6e0f395b..db5555cda 100644 --- a/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php +++ b/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php @@ -1302,8 +1302,10 @@ class EntryControllerTest extends WallabagCoreTestCase $this->getEntityManager()->flush(); - $client->request('GET', '/view/' . $entry->getId()); - $client->request('GET', '/archive/' . $entry->getId()); + $crawler = $client->request('GET', '/view/' . $entry->getId()); + + $link = $crawler->filter('a[id="markAsRead"]')->link(); + $client->click($link); $this->assertSame(302, $client->getResponse()->getStatusCode()); $this->assertStringContainsString('/view/' . $entry->getId(), $client->getResponse()->headers->get('location')); @@ -1656,7 +1658,7 @@ class EntryControllerTest extends WallabagCoreTestCase // the deletion link of the first tag $link = $crawler->filter('body div#article div.tools ul.tags li.chip a')->extract(['href'])[1]; - $this->assertSame(sprintf('/remove-tag/%s/%s', $entry->getId(), $tag->getId()), $link); + $this->assertStringStartsWith(sprintf('/remove-tag/%s/%s', $entry->getId(), $tag->getId()), $link); } public function testRandom() diff --git a/tests/Wallabag/CoreBundle/Controller/TagControllerTest.php b/tests/Wallabag/CoreBundle/Controller/TagControllerTest.php index fe21693a2..9c5f61fd0 100644 --- a/tests/Wallabag/CoreBundle/Controller/TagControllerTest.php +++ b/tests/Wallabag/CoreBundle/Controller/TagControllerTest.php @@ -123,9 +123,11 @@ class TagControllerTest extends WallabagCoreTestCase $this->getEntityManager()->clear(); // We make a first request to set an history and test redirection after tag deletion - $client->request('GET', '/view/' . $entry->getId()); + $crawler = $client->request('GET', '/view/' . $entry->getId()); $entryUri = $client->getRequest()->getRequestUri(); - $client->request('GET', '/remove-tag/' . $entry->getId() . '/' . $tag->getId()); + + $link = $crawler->filter('a[href^="/remove-tag/' . $entry->getId() . '/' . $tag->getId() . '"]')->link(); + $client->click($link); $this->assertSame(302, $client->getResponse()->getStatusCode()); $this->assertSame($entryUri, $client->getResponse()->getTargetUrl()); diff --git a/tests/Wallabag/CoreBundle/Helper/RedirectTest.php b/tests/Wallabag/CoreBundle/Helper/RedirectTest.php index 780eeddac..192ef6efd 100644 --- a/tests/Wallabag/CoreBundle/Helper/RedirectTest.php +++ b/tests/Wallabag/CoreBundle/Helper/RedirectTest.php @@ -33,7 +33,7 @@ class RedirectTest extends TestCase $this->routerMock->expects($this->any()) ->method('generate') ->with('homepage') - ->willReturn('homepage'); + ->willReturn('/'); $this->user = new User(); $this->user->setName('youpi'); @@ -59,18 +59,11 @@ class RedirectTest extends TestCase $this->redirect = new Redirect($this->routerMock, $tokenStorage); } - public function testRedirectToNullWithFallback() - { - $redirectUrl = $this->redirect->to(null, 'fallback'); - - $this->assertSame('fallback', $redirectUrl); - } - - public function testRedirectToNullWithoutFallback() + public function testRedirectToNull() { $redirectUrl = $this->redirect->to(null); - $this->assertSame($this->routerMock->generate('homepage'), $redirectUrl); + $this->assertSame('/', $redirectUrl); } public function testRedirectToValidUrl() @@ -80,6 +73,13 @@ class RedirectTest extends TestCase $this->assertSame('/unread/list', $redirectUrl); } + public function testRedirectToAbsoluteUrl() + { + $redirectUrl = $this->redirect->to('https://www.google.com/'); + + $this->assertSame('/', $redirectUrl); + } + public function testWithNotLoggedUser() { $redirect = new Redirect($this->routerMock, new TokenStorage()); @@ -94,24 +94,24 @@ class RedirectTest extends TestCase $redirectUrl = $this->redirect->to('/unread/list'); - $this->assertSame($this->routerMock->generate('homepage'), $redirectUrl); + $this->assertSame('/', $redirectUrl); } public function testUserForRedirectWithIgnoreActionMarkAsRead() { $this->user->getConfig()->setActionMarkAsRead(Config::REDIRECT_TO_HOMEPAGE); - $redirectUrl = $this->redirect->to('/unread/list', '', true); + $redirectUrl = $this->redirect->to('/unread/list', true); $this->assertSame('/unread/list', $redirectUrl); } - public function testUserForRedirectNullWithFallbackWithIgnoreActionMarkAsRead() + public function testUserForRedirectNullWithIgnoreActionMarkAsRead() { $this->user->getConfig()->setActionMarkAsRead(Config::REDIRECT_TO_HOMEPAGE); - $redirectUrl = $this->redirect->to(null, 'fallback', true); + $redirectUrl = $this->redirect->to(null, true); - $this->assertSame('fallback', $redirectUrl); + $this->assertSame('/', $redirectUrl); } }