Merge pull request #7140 from yguedidi/remove-session-based-redirection

Remove session-based redirection

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

src/Wallabag/CoreBundle/Controller/ConfigController.php

@@ -32,6 +32,7 @@
 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,15 +49,17 @@ 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;
  $this->entryRepository = $entryRepository;
  $this->tagRepository = $tagRepository;
  $this->annotationRepository = $annotationRepository;
  $this->configRepository = $configRepository;
+ $this->redirectHelper = $redirectHelper;
  }
 
  /**
@@ -646,7 +649,9 @@ public function changeViewModeAction(Request $request)
  $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);
  }
 
  /**

src/Wallabag/CoreBundle/Controller/EntryController.php

@@ -15,7 +15,6 @@
 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 @@ function ($v) {
  $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 @@ public function redirectRandomEntryAction(string $type = 'all')
  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 @@ public function toggleArchiveAction(Request $request, Entry $entry)
  $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 @@ public function toggleStarAction(Request $request, Entry $entry)
  $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 @@ public function deleteEntryAction(Request $request, Entry $entry)
  // 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 @@ public function deleteEntryAction(Request $request, Entry $entry)
  );
 
  // 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 @@ private function showEntries($type, Request $request, $page)
  {
  $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 = [];
 

src/Wallabag/CoreBundle/Controller/TagController.php

@@ -104,7 +104,7 @@ public function removeTagFromEntry(Request $request, Entry $entry, Tag $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);
  }
@@ -185,7 +185,7 @@ public function renameTagAction(Tag $tag, Request $request, TagRepository $tagRe
  $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 @@ public function tagThisSearchAction($filter, Request $request, EntryRepository $
  $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 @@ public function removeTagAction(Tag $tag, Request $request, EntryRepository $ent
  $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);
  }

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 @@ public function __construct(UrlGeneratorInterface $router, TokenStorageInterface
 
  /**
  * @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 @@ public function to($url, $fallback = '', $ignoreActionMarkAsRead = false)
  return $this->router->generate('homepage');
  }
 
- if (null !== $url) {
- return $url;
+ if (null === $url) {
+ return $this->router->generate('homepage');
  }
 
- if ('' === $fallback) {
+ if (!Uri::isAbsolutePathReference(new Uri($url))) {
  return $this->router->generate('homepage');
  }
 
- return $fallback;
+ return $url;
  }
 }

src/Wallabag/CoreBundle/Resources/views/Entry/_card_actions.html.twig

@@ -7,18 +7,20 @@
  </div>
  </div>
 
+ {% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %}
+
  <ul class="tools right">
  <li>
- <a title="{{ 'entry.list.show_same_domain'|trans }}" class="tool grey-text" href="{{ path('same_domain', {'id': entry.id}) }}" data-action="same_domain" data-entry-id="{{ entry.id }}"><i class="material-icons">language</i></a>
+ <a title="{{ 'entry.list.show_same_domain'|trans }}" class="tool grey-text" href="{{ path('same_domain', {'id': entry.id, redirect: current_path}) }}" data-action="same_domain" data-entry-id="{{ entry.id }}"><i class="material-icons">language</i></a>
  </li>
  <li>
- <a title="{{ 'entry.list.toogle_as_read'|trans }}" class="tool grey-text" href="{{ path('archive_entry', {'id': entry.id}) }}" data-action="archived" data-entry-id="{{ entry.id }}"><i class="material-icons">{% if entry.isArchived == 0 %}done{% else %}unarchive{% endif %}</i></a>
+ <a title="{{ 'entry.list.toogle_as_read'|trans }}" class="tool grey-text" href="{{ path('archive_entry', {'id': entry.id, redirect: current_path}) }}" data-action="archived" data-entry-id="{{ entry.id }}"><i class="material-icons">{% if entry.isArchived == 0 %}done{% else %}unarchive{% endif %}</i></a>
  </li>
  <li>
- <a title="{{ 'entry.list.toogle_as_star'|trans }}" class="tool grey-text" href="{{ path('star_entry', {'id': entry.id}) }}" data-action="star" data-entry-id="{{ entry.id }}"><i class="material-icons">{% if entry.isStarred == 0 %}star_border{% else %}star{% endif %}</i></a>
+ <a title="{{ 'entry.list.toogle_as_star'|trans }}" class="tool grey-text" href="{{ path('star_entry', {'id': entry.id, redirect: current_path}) }}" data-action="star" data-entry-id="{{ entry.id }}"><i class="material-icons">{% if entry.isStarred == 0 %}star_border{% else %}star{% endif %}</i></a>
  </li>
  <li>
- <a title="{{ 'entry.list.delete'|trans }}" onclick="return confirm('{{ 'entry.confirm.delete'|trans|escape('js') }}')" data-action-confirm="{{ 'entry.confirm.delete'|trans }}" class="tool grey-text delete" href="{{ path('delete_entry', {'id': entry.id}) }}" data-action="delete" data-entry-id="{{ entry.id }}"><i class="material-icons">delete</i></a>
+ <a title="{{ 'entry.list.delete'|trans }}" onclick="return confirm('{{ 'entry.confirm.delete'|trans|escape('js') }}')" data-action-confirm="{{ 'entry.confirm.delete'|trans }}" class="tool grey-text delete" href="{{ path('delete_entry', {'id': entry.id, redirect: current_path}) }}" data-action="delete" data-entry-id="{{ entry.id }}"><i class="material-icons">delete</i></a>
  </li>
  </ul>
 </div>

src/Wallabag/CoreBundle/Resources/views/Entry/_card_list.html.twig

@@ -9,12 +9,15 @@
  </div>
  {% 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')) %}
+
  <ul class="tools-list hide-on-small-only">
  <li>
- <a title="{{ 'entry.list.show_same_domain'|trans }}" class="tool grey-text" href="{{ path('same_domain', {'id': entry.id}) }}"><i class="material-icons">language</i></a>
- <a title="{{ 'entry.list.toogle_as_read'|trans }}" class="tool grey-text" href="{{ path('archive_entry', {'id': entry.id}) }}"><i class="material-icons">{% if entry.isArchived == 0 %}done{% else %}unarchive{% endif %}</i></a>
- <a title="{{ 'entry.list.toogle_as_star'|trans }}" class="tool grey-text" href="{{ path('star_entry', {'id': entry.id}) }}"><i class="material-icons">{% if entry.isStarred == 0 %}star_border{% else %}star{% endif %}</i></a>
- <a title="{{ 'entry.list.delete'|trans }}" onclick="return confirm('{{ 'entry.confirm.delete'|trans|escape('js') }}')" class="tool grey-text delete" href="{{ path('delete_entry', {'id': entry.id}) }}"><i class="material-icons">delete</i></a>
+ <a title="{{ 'entry.list.show_same_domain'|trans }}" class="tool grey-text" href="{{ path('same_domain', {'id': entry.id, redirect: current_path}) }}"><i class="material-icons">language</i></a>
+ <a title="{{ 'entry.list.toogle_as_read'|trans }}" class="tool grey-text" href="{{ path('archive_entry', {'id': entry.id, redirect: current_path}) }}"><i class="material-icons">{% if entry.isArchived == 0 %}done{% else %}unarchive{% endif %}</i></a>
+ <a title="{{ 'entry.list.toogle_as_star'|trans }}" class="tool grey-text" href="{{ path('star_entry', {'id': entry.id, redirect: current_path}) }}"><i class="material-icons">{% if entry.isStarred == 0 %}star_border{% else %}star{% endif %}</i></a>
+ <a title="{{ 'entry.list.delete'|trans }}" onclick="return confirm('{{ 'entry.confirm.delete'|trans|escape('js') }}')" class="tool grey-text delete" href="{{ path('delete_entry', {'id': entry.id, redirect: current_path}) }}"><i class="material-icons">delete</i></a>
  </li>
  </ul>
 </div>

src/Wallabag/CoreBundle/Resources/views/Entry/_tags.html.twig

@@ -4,7 +4,8 @@
  <li class="chip">
  <a class="chip-label" href="{{ path('tag_entries', {'slug': tag.slug}) }}">{{ tag.label }}</a>
  {% if withRemove is defined and withRemove == true %}
- <a class="chip-action" href="{{ path('remove_tag', {'entry': entryId, 'tag': tag.id}) }}" onclick="return confirm('{{ 'entry.confirm.delete_tag'|trans|escape('js') }}')">
+ {% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %}
+ <a class="chip-action" href="{{ path('remove_tag', {'entry': entryId, 'tag': tag.id, redirect: current_path}) }}" onclick="return confirm('{{ 'entry.confirm.delete_tag'|trans|escape('js') }}')">
  <i class="material-icons vertical-align-middle">delete</i>
  </a>
  {% endif %}

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 %}
- <form name="form_mass_action" action="{{ path('mass_action') }}" method="post">
+ <form name="form_mass_action" action="{{ path('mass_action', {redirect: current_path}) }}" method="post">
  <div class="results">
  <div class="nb-results">
  {{ 'entry.list.number_on_the_page'|trans({'%count%': entries.count}) }}
  {% if entries.count > 0 %}
- <a class="results-item" href="{{ path('switch_view_mode') }}"><i class="material-icons">{% if list_mode == 0 %}view_list{% else %}view_module{% endif %}</i></a>
+ <a class="results-item" href="{{ path('switch_view_mode', {redirect: current_path}) }}"><i class="material-icons">{% if list_mode == 0 %}view_list{% else %}view_module{% endif %}</i></a>
  {% endif %}
  {% if entries.count > 0 %}
  <label for="mass-action-inputs-displayed" class="mass-action-toggle results-item tooltipped" data-position="right" data-delay="50" data-tooltip="{{ 'entry.list.toggle_mass_action'|trans }}"><i class="material-icons">library_add_check</i></label>
@@ -39,7 +40,7 @@
  {% include "@WallabagCore/Entry/_feed_link.html.twig" %}
  {% endif %}
  </div>
- {% if current_route == 'search' %}<div><a href="{{ path('tag_this_search', {'filter': searchTerm, 'currentRoute': app.request.get('currentRoute')}) }}" title="{{ 'entry.list.assign_search_tag'|trans }}">{{ 'entry.list.assign_search_tag'|trans }}</a></div>{% endif %}
+ {% if current_route == 'search' %}<div><a href="{{ path('tag_this_search', {'filter': searchTerm, 'currentRoute': app.request.get('currentRoute'), redirect: current_path}) }}" title="{{ 'entry.list.assign_search_tag'|trans }}">{{ 'entry.list.assign_search_tag'|trans }}</a></div>{% endif %}
  {% if entries.getNbPages > 1 %}
  {{ pagerfanta(entries, 'default_wallabag') }}
  {% endif %}