From 6222eea71536452992766c7c66f60b49dfa88c2c Mon Sep 17 00:00:00 2001
From: Kevin Ditscheid <kevinditscheid@gmail.com>
Date: Wed, 11 Apr 2018 10:27:30 +0200
Subject: [PATCH] [BUGFIX] Remove the usage of GeneralUtility::_GETset

Alterring GET parameters should never be done. Instead using method
parameters to alter functionality.
This fixes side effects on the canonical URL generation, if the news
plugin is present with one of the list-Actions assigned.
---
 .../Controller/ListByCategoryController.php   | 19 +++++--
 Classes/Controller/OverviewController.php     | 52 +++++++++++++------
 2 files changed, 52 insertions(+), 19 deletions(-)

diff --git a/Classes/Controller/ListByCategoryController.php b/Classes/Controller/ListByCategoryController.php
index 9a5d65e..d17c917 100644
--- a/Classes/Controller/ListByCategoryController.php
+++ b/Classes/Controller/ListByCategoryController.php
@@ -52,15 +52,28 @@ class ListByCategoryController extends AbstractController {
 	 */
 	protected $newsRepository;
 
+	/**
+	 * Initialize the indexAction to set the currentPageBrowserPage parameter
+	 *
+	 * @throws \TYPO3\CMS\Extbase\Mvc\Exception\InvalidArgumentNameException
+	 */
+	public function initializeIndexAction(){
+		$currentPageBrowserPage = (int) GeneralUtility::_GP('tx_sgnews_pagebrowser')['currentPage'];
+		if($currentPageBrowserPage > 0){
+			$this->request->setArgument('currentPageBrowserPage', $currentPageBrowserPage);
+		}
+	}
+
 	/**
 	 * Renders the news list of a category
 	 *
 	 * @param array $newsMetaData
+	 * @param int $currentPageBrowserPage
 	 * @return void
 	 * @throws \InvalidArgumentException
 	 * @throws \TYPO3\CMS\Extbase\Persistence\Exception\InvalidQueryException
 	 */
-	public function indexAction(array $newsMetaData = []) {
+	public function indexAction(array $newsMetaData = [], $currentPageBrowserPage = 0) {
 		$filterByCategories = FALSE;
 		$categoryUids = GeneralUtility::intExplode(',', $this->settings['categories']);
 		$tagUids = GeneralUtility::intExplode(',', $this->settings['tags'], TRUE);
@@ -85,7 +98,6 @@ class ListByCategoryController extends AbstractController {
 
 		$offset = 0;
 		$newsPerPage = (int) $this->settings['newsLimitPerPage'];
-		$currentPageBrowserPage = (int) GeneralUtility::_GP('tx_sgnews_pagebrowser')['currentPage'];
 		if ($currentPageBrowserPage && $newsPerPage) {
 			$offset = $currentPageBrowserPage * $newsPerPage;
 		}
@@ -116,8 +128,7 @@ class ListByCategoryController extends AbstractController {
 		if (count($newsMetaData) < $newsPerPage) {
 			$nextPage = $currentPageBrowserPage + 1;
 			if ($nextPage <= $numberOfPages) {
-				GeneralUtility::_GETset(['tx_sgnews_pagebrowser' => ['currentPage' => $nextPage]]);
-				$this->indexAction($newsMetaData);
+				$this->indexAction($newsMetaData, $nextPage);
 				return;
 			}
 		}
diff --git a/Classes/Controller/OverviewController.php b/Classes/Controller/OverviewController.php
index e924a01..a0aa8e8 100644
--- a/Classes/Controller/OverviewController.php
+++ b/Classes/Controller/OverviewController.php
@@ -56,22 +56,35 @@ class OverviewController extends AbstractController {
 	 */
 	protected $newsRepository;
 
+	/**
+	 * Initialize the overviewAction to set the currentPageBrowserPage parameter
+	 *
+	 * @throws \TYPO3\CMS\Extbase\Mvc\Exception\InvalidArgumentNameException
+	 */
+	public function initializeOverviewAction(){
+		$currentPageBrowserPage = (int) GeneralUtility::_GP('tx_sgnews_pagebrowser')['currentPage'];
+		if($currentPageBrowserPage > 0){
+			$this->request->setArgument('currentPageBrowserPage', $currentPageBrowserPage);
+		}
+	}
+
 	/**
 	 * Renders the news overview
 	 *
 	 * @param array $newsFilter
+	 * @param int $currentPageBrowserPage
 	 * @return void
 	 * @throws \InvalidArgumentException
 	 * @throws \TYPO3\CMS\Extbase\Persistence\Exception\InvalidQueryException
 	 * @throws \TYPO3\CMS\Extbase\Mvc\Exception\StopActionException
 	 */
-	public function overviewAction(array $newsFilter = []) {
+	public function overviewAction(array $newsFilter = [], $currentPageBrowserPage = 0) {
 		switch ((int) $this->settings['groupBy']) {
 			case 1:
-				$this->overviewWithCategories([], [], $newsFilter);
+				$this->overviewWithCategories([], [], $newsFilter, $currentPageBrowserPage);
 				break;
 			case 2:
-				$this->overviewWithTags([], [], $newsFilter);
+				$this->overviewWithTags([], [], $newsFilter, $currentPageBrowserPage);
 				break;
 			default:
 				$this->forward('overviewWithoutCategories', NULL, NULL, $this->request->getArguments());
@@ -121,19 +134,19 @@ class OverviewController extends AbstractController {
 	 * @param array $allNews
 	 * @param array $newsFilter
 	 * @param boolean $isInitialCall
+	 * @param int $currentPageBrowserPage
 	 * @return void
 	 * @throws \InvalidArgumentException
 	 * @throws \TYPO3\CMS\Extbase\Persistence\Exception\InvalidQueryException
 	 */
 	protected function overviewWithCategories(
-		array $newsByCategory = [], array $allNews = [], array $newsFilter = [], $isInitialCall = TRUE
+		array $newsByCategory = [], array $allNews = [], array $newsFilter = [], $isInitialCall = TRUE, $currentPageBrowserPage = 0
 	) {
 		$newsLimitPerCategory = (int) $this->settings['newsLimit'];
 		$this->categoryRepository->setDefaultOrderings(['sorting' => Query::ORDER_ASCENDING]);
 
 		$offset = 0;
 
-		$currentPageBrowserPage = (int) GeneralUtility::_GP('tx_sgnews_pagebrowser')['currentPage'];
 		if ($currentPageBrowserPage && $newsLimitPerCategory) {
 			$offset = ($currentPageBrowserPage * $newsLimitPerCategory) - ($isInitialCall ? 0 : 1);
 		}
@@ -233,8 +246,7 @@ class OverviewController extends AbstractController {
 		if ($maxNewsPerCategory < $newsLimitPerCategory && count($allNews) < $newsLimitPerCategory) {
 			$nextPage = $currentPageBrowserPage + 1;
 			if ($nextPage <= $numberOfPages) {
-				GeneralUtility::_GETset(['tx_sgnews_pagebrowser' => ['currentPage' => $nextPage]]);
-				$this->overviewWithCategories($newsByCategory, $allNews, $newsFilter, FALSE);
+				$this->overviewWithCategories($newsByCategory, $allNews, $newsFilter, FALSE, $nextPage);
 				return;
 			}
 		}
@@ -267,18 +279,18 @@ class OverviewController extends AbstractController {
 	 * @param array $allNews
 	 * @param array $newsFilter
 	 * @param boolean $isInitialCall
+	 * @param int $currentPageBrowserPage
 	 * @return void
 	 * @throws \InvalidArgumentException
 	 * @throws \TYPO3\CMS\Extbase\Persistence\Exception\InvalidQueryException
 	 */
 	protected function overviewWithTags(
-		array $newsByTag = [], array $allNews = [], array $newsFilter = [], $isInitialCall = TRUE
+		array $newsByTag = [], array $allNews = [], array $newsFilter = [], $isInitialCall = TRUE, $currentPageBrowserPage = 0
 	) {
 		$newsLimitPerTag = (int) $this->settings['newsLimit'];
 		$this->tagRepository->setDefaultOrderings(['sorting' => Query::ORDER_ASCENDING]);
 
 		$offset = 0;
-		$currentPageBrowserPage = (int) GeneralUtility::_GP('tx_sgnews_pagebrowser')['currentPage'];
 		if ($currentPageBrowserPage && $newsLimitPerTag) {
 			$offset = ($currentPageBrowserPage * $newsLimitPerTag) - ($isInitialCall ? 0 : 1);
 		}
@@ -387,8 +399,7 @@ class OverviewController extends AbstractController {
 		if ($maxNewsPerTag < $newsLimitPerTag && count($allNews) < $newsLimitPerTag) {
 			$nextPage = $currentPageBrowserPage + 1;
 			if ($nextPage <= $numberOfPages) {
-				GeneralUtility::_GETset(['tx_sgnews_pagebrowser' => ['currentPage' => $nextPage]]);
-				$this->overviewWithTags($newsByTag, $allNews, $newsFilter, FALSE);
+				$this->overviewWithTags($newsByTag, $allNews, $newsFilter, FALSE, $nextPage);
 				return;
 			}
 		}
@@ -412,19 +423,32 @@ class OverviewController extends AbstractController {
 		$this->view->assign('tagTabs', TRUE);
 	}
 
+	/**
+	 * Initialize the overviewWithoutCategoriesAction to set the currentPageBrowserPage parameter
+	 *
+	 * @throws \TYPO3\CMS\Extbase\Mvc\Exception\InvalidArgumentNameException
+	 */
+	public function initializeOverviewWithoutCategoriesAction(){
+		$currentPageBrowserPage = (int) GeneralUtility::_GP('tx_sgnews_pagebrowser')['currentPage'];
+		if($currentPageBrowserPage > 0){
+			$this->request->setArgument('currentPageBrowserPage', $currentPageBrowserPage);
+		}
+	}
+
 	/**
 	 * Renders the news in a paginated list
 	 *
 	 * @param array $newsMetaData
 	 * @param array $newsFilter
 	 * @param boolean $isInitialCall
+	 * @param int $currentPageBrowserPage
 	 * @return void
 	 * @throws \InvalidArgumentException
 	 * @throws \TYPO3\CMS\Extbase\Persistence\Exception\InvalidQueryException
 	 * @throws \TYPO3\CMS\Extbase\Mvc\Exception\NoSuchArgumentException
 	 */
 	protected function overviewWithoutCategoriesAction(
-		array $newsMetaData = [], array $newsFilter = NULL, $isInitialCall = TRUE
+		array $newsMetaData = [], array $newsFilter = NULL, $isInitialCall = TRUE, $currentPageBrowserPage = 0
 	) {
 		// remember selection of the filter values, if any
 		$selectedTag = $this->tagRepository->findByUid((int) $newsFilter['tag']);
@@ -434,7 +458,6 @@ class OverviewController extends AbstractController {
 
 		$offset = 0;
 		$newsPerPage = (int) $this->settings['newsLimit'];
-		$currentPageBrowserPage = (int) GeneralUtility::_GP('tx_sgnews_pagebrowser')['currentPage'];
 		if ($currentPageBrowserPage && $newsPerPage) {
 			// might be necessary for the recursive calling performance wise
 			$offset = ($currentPageBrowserPage * $newsPerPage) - ($isInitialCall ? 0 : 1);
@@ -505,8 +528,7 @@ class OverviewController extends AbstractController {
 		if (count($newsMetaData) < $newsPerPage) {
 			$nextPage = $currentPageBrowserPage + 1;
 			if ($nextPage <= $numberOfPages) {
-				GeneralUtility::_GETset(['tx_sgnews_pagebrowser' => ['currentPage' => $nextPage]]);
-				$this->overviewWithoutCategoriesAction($newsMetaData, $newsFilter, FALSE);
+				$this->overviewWithoutCategoriesAction($newsMetaData, $newsFilter, FALSE, $nextPage);
 				return;
 			}
 		}
-- 
GitLab