[BUGFIX] EXT:felogin: Multiple bugs with preserveGETvars
authorJigal van Hemert <jigal@xs4all.nl>
Fri, 30 Dec 2011 19:32:32 +0000 (20:32 +0100)
committerChristian Kuhn <lolli@schwarzbu.ch>
Thu, 15 Nov 2012 10:44:04 +0000 (11:44 +0100)
Refactor and fix handling of preserveGETvars in felogin.
 * Add support for multi dimensional arrays
 * urlencode() values
 * Preserve only parameters defined in 'preserveGETvars'

The patch adds a new ArrayUtility method to handle
recursive array intersections. See the unit tests for details.

Change-Id: I90e2f8eb79586369a9c15c7ef19f7658b1d65ed3
Fixes: #19938
Fixes: #23324
Fixes: #23649
Fixes: #36894
Fixes: #38589
Releases: 6.0, 4.7
Reviewed-on: http://review.typo3.org/7638
Reviewed-by: Anja Leichsenring
Tested-by: Anja Leichsenring
Reviewed-by: Jigal van Hemert
Tested-by: Jigal van Hemert
Reviewed-by: Christian Kuhn
Tested-by: Christian Kuhn
typo3/sysext/core/Classes/Utility/ArrayUtility.php
typo3/sysext/core/Tests/Unit/Utility/ArrayUtilityTest.php
typo3/sysext/felogin/Classes/Controller/FrontendLoginController.php
typo3/sysext/felogin/Tests/Unit/Controller/FrontendLoginControllerTest.php

index 0562971..63a265b 100644 (file)
@@ -48,18 +48,18 @@ class ArrayUtility {
         * - Needle: 'findMe'
         * - Given array:
         * array(
-        * 'foo' => 'noMatch',
-        * 'bar' => 'findMe',
-        * 'foobar => array(
-        * 'foo' => 'findMe',
-        * ),
+        *   'foo' => 'noMatch',
+        *   'bar' => 'findMe',
+        *   'foobar => array(
+        *     'foo' => 'findMe',
+        *   ),
         * );
         * - Result:
         * array(
-        * 'bar' => 'findMe',
-        * 'foobar' => array(
-        * 'foo' => findMe',
-        * ),
+        *   'bar' => 'findMe',
+        *   'foobar' => array(
+        *     'foo' => findMe',
+        *   ),
         * );
         *
         * See the unit tests for more examples and expected behaviour
@@ -92,14 +92,15 @@ class ArrayUtility {
        /**
         * Checks if a given path exists in array
         *
-        * array:
+        * Example:
+        * - array:
         * array(
-        * 'foo' => array(
-        * 'bar' = 'test',
-        * )
+        *   'foo' => array(
+        *     'bar' = 'test',
+        *   )
         * );
-        * path: 'foo/bar'
-        * return: TRUE
+        * path: 'foo/bar'
+        * return: TRUE
         *
         * @param array $array Given array
         * @param string $path Path to test, 'foo/bar/foobar'
@@ -120,24 +121,24 @@ class ArrayUtility {
        /**
         * Returns a value by given path
         *
-        * Simple example
-        * Input array:
+        * Example
+        * - array:
         * array(
-        * 'foo' => array(
-        * 'bar' => array(
-        * 'baz' => 42
-        * )
-        * )
+        *   'foo' => array(
+        *     'bar' => array(
+        *       'baz' => 42
+        *     )
+        *   )
         * );
-        * Path to get: foo/bar/baz
-        * Will return: 42
+        * - path: foo/bar/baz
+        * - return: 42
         *
-        * If a path segments contains a delimeter character, the path segment
+        * If a path segments contains a delimiter character, the path segment
         * must be enclosed by " (double quote), see unit tests for details
         *
         * @param array $array Input array
         * @param string $path Path within the array
-        * @param string $delimiter Defined path delimeter, default /
+        * @param string $delimiter Defined path delimiter, default /
         * @return mixed
         * @throws \RuntimeException
         */
@@ -164,25 +165,26 @@ class ArrayUtility {
        /**
         * Modifies or sets a new value in an array by given path
         *
-        * Input array:
+        * Example:
+        * - array:
         * array(
-        * 'foo' => array(
-        * 'bar' => 42,
-        * ),
+        *   'foo' => array(
+        *     'bar' => 42,
+        *   ),
         * );
-        * Path to get: foo/bar
-        * Value to set: 23
-        * Will return:
+        * - path: foo/bar
+        * - value: 23
+        * - return:
         * array(
-        * 'foo' => array(
-        * 'bar' => 23,
-        * ),
+        *   'foo' => array(
+        *     'bar' => 23,
+        *   ),
         * );
         *
         * @param array $array Input array to manipulate
         * @param string $path Path in array to search for
         * @param mixed $value Value to set at path location in array
-        * @param string $delimiter Path delimeter
+        * @param string $delimiter Path delimiter
         * @return array Modified array
         * @throws \RuntimeException
         */
@@ -235,9 +237,12 @@ class ArrayUtility {
         * Exports an array as string.
         * Similar to var_export(), but representation follows the TYPO3 core CGL.
         *
+        * See unit tests for detailed examples
+        *
         * @param array $array Array to export
         * @param integer $level Internal level used for recursion, do *not* set from outside!
         * @return string String representation of array
+        * @throws \RuntimeException
         */
        static public function arrayExport(array $array = array(), $level = 0) {
                $lines = 'array(' . LF;
@@ -290,9 +295,31 @@ class ArrayUtility {
        /**
         * Converts a multidimensional array to a flat representation.
         *
-        * array('first.' => array('second' => 1)) and array('first' => array('second' => 1))
-        * will become
-        * array('first.second' => 1)
+        * See unit tests for more details
+        *
+        * Example:
+        * - array:
+        * array(
+        *   'first.' => array(
+        *     'second' => 1
+        *   )
+        * )
+        * - result:
+        * array(
+        *   'first.second' => 1
+        * )
+        *
+        * Example:
+        * - array:
+        * array(
+        *   'first' => array(
+        *     'second' => 1
+        *   )
+        * )
+        * - result:
+        * array(
+        *   'first.second' => 1
+        * )
         *
         * @param array $array The (relative) array to be converted
         * @param string $prefix The (relative) prefix to be used (e.g. 'section.')
@@ -312,6 +339,59 @@ class ArrayUtility {
                return $flatArray;
        }
 
+       /**
+        * Determine the intersections between two arrays, recursively comparing keys
+        * A complete sub array of $source will be preserved, if the key exists in $mask.
+        *
+        * See unit tests for more examples and edge cases.
+        *
+        * Example:
+        * - source:
+        * array(
+        *   'key1' => 'bar',
+        *   'key2' => array(
+        *     'subkey1' => 'sub1',
+        *     'subkey2' => 'sub2',
+        *   ),
+        *   'key3' => 'baz',
+        * )
+        * - mask:
+        * array(
+        *   'key1' => NULL,
+        *   'key2' => array(
+        *     'subkey1' => exists',
+        *   ),
+        * )
+        * - return:
+        * array(
+        *   'key1' => 'bar',
+        *   'key2' => array(
+        *     'subkey1' => 'sub1',
+        *   ),
+        * )
+        *
+        * @param array $source Source array
+        * @param array $mask Array that has the keys which should be kept in the source array
+        * @return array Keys which are present in both arrays with values of the source array
+        */
+       public static function intersectRecursive(array $source, array $mask = array()) {
+               $intersection = array();
+               $sourceArrayKeys = array_keys($source);
+               foreach ($sourceArrayKeys as $key) {
+                       if (!array_key_exists($key, $mask)) {
+                               continue;
+                       }
+                       if (is_array($source[$key]) && is_array($mask[$key])) {
+                               $value = self::intersectRecursive($source[$key], $mask[$key]);
+                               if (!empty($value)) {
+                                       $intersection[$key] = $value;
+                               }
+                       } else {
+                               $intersection[$key] = $source[$key];
+                       }
+               }
+               return $intersection;
+       }
 }
 
 
index 6d689f1..292f66d 100644 (file)
@@ -771,20 +771,15 @@ class ArrayUtilityTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                $this->assertSame($expected, \TYPO3\CMS\Core\Utility\ArrayUtility::arrayExport($array));
        }
 
-       /**
-        * @param array $array
-        * @param array $expected
-        * @test
-        * @dataProvider arrayIsFlatDataProvider
-        */
-       public function arrayIsFlat(array $array, array $expected) {
-               $this->assertEquals($expected, \TYPO3\CMS\Core\Utility\ArrayUtility::flatten($array));
-       }
+
+       ///////////////////////
+       // Tests concerning flatten
+       ///////////////////////
 
        /**
         * @return array
         */
-       public function arrayIsFlatDataProvider() {
+       public function flattenCalculatesExpectedResultDataProvider() {
                return array(
                        'plain array' => array(
                                array(
@@ -873,6 +868,227 @@ class ArrayUtilityTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                );
        }
 
+       /**
+        * @test
+        * @param array $array
+        * @param array $expected
+        * @dataProvider flattenCalculatesExpectedResultDataProvider
+        */
+       public function flattenCalculatesExpectedResult(array $array, array $expected) {
+               $this->assertEquals($expected, \TYPO3\CMS\Core\Utility\ArrayUtility::flatten($array));
+       }
+
+
+       ///////////////////////
+       // Tests concerning intersectRecursive
+       ///////////////////////
+
+       /**
+        * @return array
+        */
+       public function intersectRecursiveCalculatesExpectedResultDataProvider() {
+               $sameObject = new \stdClass();
+               return array(
+                       // array($source, $mask, $expected)
+                       'empty array is returned if source is empty array' => array(
+                               array(),
+                               array(
+                                       'foo' => 'bar',
+                               ),
+                               array(),
+                       ),
+                       'empty array is returned if mask is empty' => array(
+                               array(
+                                       'foo' => 'bar',
+                               ),
+                               array(),
+                               array(),
+                       ),
+                       'key is kept on first level if exists in mask' => array(
+                               array(
+                                       'foo' => 42,
+                               ),
+                               array(
+                                       'foo' => 42,
+                               ),
+                               array(
+                                       'foo' => 42,
+                               ),
+                       ),
+                       'value of key in source is kept if mask has different value' => array(
+                               array(
+                                       'foo' => 42,
+                               ),
+                               array(
+                                       'foo' => new \stdClass(),
+                               ),
+                               array(
+                                       'foo' => 42,
+                               ),
+                       ),
+                       'key is kept on first level if according mask value is NULL' => array(
+                               array(
+                                       'foo' => 42,
+                               ),
+                               array(
+                                       'foo' => NULL,
+                               ),
+                               array(
+                                       'foo' => 42,
+                               ),
+                       ),
+                       'null in source value is kept' => array(
+                               array(
+                                       'foo' => NULL,
+                               ),
+                               array(
+                                       'foo' => 'bar',
+                               ),
+                               array(
+                                       'foo' => NULL,
+                               )
+                       ),
+                       'mask does not add new keys' => array(
+                               array(
+                                       'foo' => 42,
+                               ),
+                               array(
+                                       'foo' => 23,
+                                       'bar' => array(
+                                               4711
+                                       ),
+                               ),
+                               array(
+                                       'foo' => 42,
+                               ),
+                       ),
+                       'mask does not overwrite simple values with arrays' => array(
+                               array(
+                                       'foo' => 42,
+                               ),
+                               array(
+                                       'foo' => array(
+                                               'bar' => 23,
+                                       ),
+                               ),
+                               array(
+                                       'foo' => 42,
+                               ),
+                       ),
+                       'key is kept on first level if according mask value is array' => array(
+                               array(
+                                       'foo' => 42,
+                               ),
+                               array(
+                                       'foo' => array(
+                                               'bar' => 23
+                                       ),
+                               ),
+                               array(
+                                       'foo' => 42,
+                               ),
+                       ),
+                       'full array is kept if value is array and mask value is simple type' => array(
+                               array(
+                                       'foo' => array(
+                                               'bar' => 23
+                                       ),
+                               ),
+                               array(
+                                       'foo' => 42,
+                               ),
+                               array(
+                                       'foo' => array(
+                                               'bar' => 23
+                                       ),
+                               ),
+                       ),
+                       'key handling is type agnostic' => array(
+                               array(
+                                       42 => 'foo',
+                               ),
+                               array(
+                                       '42' => 'bar',
+                               ),
+                               array(
+                                       42 => 'foo',
+                               ),
+                       ),
+                       'value is same if value is object' => array(
+                               array(
+                                       'foo' => $sameObject,
+                               ),
+                               array(
+                                       'foo' => 'something',
+                               ),
+                               array(
+                                       'foo' => $sameObject,
+                               ),
+                       ),
+                       'mask does not add simple value to result if key does not exist in source' => array(
+                               array(
+                                       'foo' => '42',
+                               ),
+                               array(
+                                       'foo' => '42',
+                                       'bar' => 23
+                               ),
+                               array(
+                                       'foo' => '42',
+                               ),
+                       ),
+                       'array of source is kept if value of mask key exists but is no array' => array(
+                               array(
+                                       'foo' => '42',
+                                       'bar' => array(
+                                               'baz' => 23
+                                       ),
+                               ),
+                               array(
+                                       'foo' => 'value is not significant',
+                                       'bar' => NULL,
+                               ),
+                               array(
+                                       'foo' => '42',
+                                       'bar' => array(
+                                               'baz' => 23
+                                       ),
+                               ),
+                       ),
+                       'sub arrays are kept if mask has according sub array key and is similar array' => array(
+                               array(
+                                       'first1' => 42,
+                                       'first2' => array(
+                                               'second1' => 23,
+                                               'second2' => 4711,
+                                       ),
+                               ),
+                               array(
+                                       'first1' => 42,
+                                       'first2' => array(
+                                               'second1' => 'exists but different',
+                                       ),
+                               ),
+                               array(
+                                       'first1' => 42,
+                                       'first2' => array(
+                                               'second1' => 23,
+                                       ),
+                               ),
+                       ),
+               );
+       }
+
+       /**
+        * @test
+        * @param array $source
+        * @param array $mask
+        * @param array $expected
+        * @dataProvider intersectRecursiveCalculatesExpectedResultDataProvider
+        */
+       public function intersectRecursiveCalculatesExpectedResult(array $source, array $mask, array $expected) {
+               $this->assertSame($expected, \TYPO3\CMS\Core\Utility\ArrayUtility::intersectRecursive($source, $mask));
+       }
 }
 
 ?>
\ No newline at end of file
index 855e244..0460399 100644 (file)
@@ -761,32 +761,31 @@ class FrontendLoginController extends \TYPO3\CMS\Frontend\Plugin\AbstractPlugin
        }
 
        /**
-        * Is used by TS-setting preserveGETvars
-        * possible values are "all" or a commaseperated list of GET-vars
-        * they are used as additionalParams for link generation
+        * Add additional parameters for links according to TS setting preserveGETvars.
+        * Possible values are "all" or a comma separated list of allowed GET-vars.
+        * Supports multi-dimensional GET-vars.
+        * Some hardcoded values are dropped.
         *
         * @return string additionalParams-string
         */
        protected function getPreserveGetVars() {
-               $params = '';
-               $preserveVars = !($this->conf['preserveGETvars'] || $this->conf['preserveGETvars'] == 'all' ? array() : implode(',', (array) $this->conf['preserveGETvars']));
                $getVars = \TYPO3\CMS\Core\Utility\GeneralUtility::_GET();
-               foreach ($getVars as $key => $val) {
-                       if (stristr($key, $this->prefixId) === FALSE) {
-                               if (is_array($val)) {
-                                       foreach ($val as $key1 => $val1) {
-                                               if ($this->conf['preserveGETvars'] == 'all' || in_array($key . '[' . $key1 . ']', $preserveVars)) {
-                                                       $params .= '&' . $key . '[' . $key1 . ']=' . $val1;
-                                               }
-                                       }
-                               } else {
-                                       if (!in_array($key, array('id', 'no_cache', 'logintype', 'redirect_url', 'cHash'))) {
-                                               $params .= '&' . $key . '=' . $val;
-                                       }
-                               }
-                       }
+               unset(
+                       $getVars['id'],
+                       $getVars['no_cache'],
+                       $getVars['logintype'],
+                       $getVars['redirect_url'],
+                       $getVars['cHash']
+               );
+               if ($this->conf['preserveGETvars'] === 'all') {
+                       $preserveQueryParts = $getVars;
+               } else {
+                       $preserveQueryParts = \TYPO3\CMS\Core\Utility\GeneralUtility::trimExplode(',', $this->conf['preserveGETvars']);
+                       $preserveQueryParts = \TYPO3\CMS\Core\Utility\GeneralUtility::explodeUrl2Array(implode('=1&', $preserveQueryParts) . '=1', TRUE);
+                       $preserveQueryParts = \TYPO3\CMS\Core\Utility\ArrayUtility::intersectRecursive($getVars, $preserveQueryParts);
                }
-               return $params;
+               $parameters = \TYPO3\CMS\Core\Utility\GeneralUtility::implodeArrayForUrl('', $preserveQueryParts);
+               return $parameters;
        }
 
        /**
index ddeeeb9..9334d39 100644 (file)
@@ -37,13 +37,6 @@ namespace TYPO3\CMS\Felogin\Tests\Unit\Controller;
 class FrontendLoginTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
 
        /**
-        * Enable backup of global and system variables
-        *
-        * @var boolean
-        */
-       protected $backupGlobals = TRUE;
-
-       /**
         * @var \TYPO3\CMS\Core\Database\DatabaseConnection
         */
        protected $typo3DbBackup;
@@ -293,6 +286,134 @@ class FrontendLoginTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                $this->assertEquals($url, $this->accessibleFixture->_call('validateRedirectUrl', $url));
        }
 
+
+       /*************************
+        * Test concerning getPreverveGetVars
+        *************************/
+
+       /**
+        * @return array
+        */
+       public function getPreserveGetVarsReturnsCorrectResultDataProvider() {
+               $getArray = array(
+                       'id' => '10',
+                       'L' => '3',
+                       'tx_ext2' => 'ext2value',
+                       'tx_ext3' => array('ext3key' => 44),
+                       'tx_someext' => array(
+                               '@widget_0' => array('currentPage' => '3', 'perPage' => '8'),
+                               'controller' => 'controller1',
+                               'action' => 'action1'
+                       ),
+                       'no_cache' => 1,
+                       'logintype' => 'login',
+                       'redirect_url' => 'someurl',
+                       'cHash' => '1c9b08081c416bada560b4cac62ec64d',
+               );
+
+               return array(
+                       'special get var id is not preserved' => array(
+                               array(
+                                       'id' => 42,
+                               ),
+                               '',
+                               '',
+                       ),
+                       'simple additional parameter is not preserved if not specified in preservedGETvars' => array(
+                               array(
+                                       'id' => 42,
+                                       'special' => 23,
+                               ),
+                               '',
+                               '',
+                       ),
+                       'all params except ignored ones are preserved if preservedGETvars is set to "all"' => array(
+                               array(
+                                       'id' => 42,
+                                       'special1' => 23,
+                                       'special2' => array(
+                                               'foo' => 'bar',
+                                       ),
+                               ),
+                               'all',
+                               '&special1=23&special2[foo]=bar',
+                       ),
+                       'preserve single parameter' => array(
+                               array(
+                                       'L' => 42,
+                               ),
+                               'L',
+                               '&L=42'
+                       ),
+                       'preserve whole parameter array' => array(
+                               array(
+                                       'L' => 3,
+                                       'tx_someext' => array(
+                                               'foo' => 'simple',
+                                               'bar' => array(
+                                                       'baz' => 'simple',
+                                               ),
+                                       ),
+                               ),
+                               'L,tx_someext',
+                               '&L=3&tx_someext[foo]=simple&tx_someext[bar][baz]=simple',
+                       ),
+                       'preserve part of sub array' => array(
+                               array(
+                                       'L' => 3,
+                                       'tx_someext' => array(
+                                               'foo' => 'simple',
+                                               'bar' => array(
+                                                       'baz' => 'simple',
+                                               ),
+                                       ),
+                               ),
+                               'L,tx_someext[bar]',
+                               '&L=3&tx_someext[bar][baz]=simple',
+                       ),
+                       'preserve keys on different levels' => array(
+                               array(
+                                       'L' => 3,
+                                       'no-preserve' => 'whatever',
+                                       'tx_ext2' => array(
+                                               'foo' => 'simple',
+                                       ),
+                                       'tx_ext3' => array(
+                                               'bar' => array(
+                                                       'baz' => 'simple',
+                                               ),
+                                               'go-away' => '',
+                                       ),
+                               ),
+                               'L,tx_ext2,tx_ext3[bar]',
+                               '&L=3&tx_ext2[foo]=simple&tx_ext3[bar][baz]=simple',
+                       ),
+                       'preserved value that does not exist in get' => array(
+                               array(),
+                               'L,foo[bar]',
+                               ''
+                       ),
+                       'url params are encoded' => array(
+                               array('tx_ext1' => 'param with spaces and \\ %<>& /'),
+                               'L,tx_ext1',
+                               '&tx_ext1=param%20with%20spaces%20and%20%20%25%3C%3E%26%20%2F'
+                       ),
+               );
+       }
+
+       /**
+        * @test
+        * @dataProvider getPreserveGetVarsReturnsCorrectResultDataProvider
+        * @param array $getArray
+        * @param string $preserveVars
+        * @param string $expected
+        * @return void
+        */
+       public function getPreserveGetVarsReturnsCorrectResult(array $getArray, $preserveVars, $expected) {
+               $_GET = $getArray;
+               $this->accessibleFixture->conf['preserveGETvars'] = $preserveVars;
+               $this->assertSame($expected, $this->accessibleFixture->_call('getPreserveGetVars'));
+       }
 }
 
 ?>
\ No newline at end of file