[BUGFIX] Allow "0" as path segment in ArrayUtility 31/50431/3
authorHelmut Hummel <info@helhum.io>
Fri, 28 Oct 2016 23:17:53 +0000 (01:17 +0200)
committerNicole Cordes <typo3@cordes.co>
Sat, 29 Oct 2016 11:14:35 +0000 (13:14 +0200)
The empty() checks are too loose when checking for path segments
of an array, as "0" is a valid segment.
Instead we need to check for an empty string in the according places.

Also add a missing string check in getValueByPath.

Resolves: #78495
Releases: master, 7.6
Change-Id: I6b2a0c286c345aa94595a4a74da077bc8adea292
Reviewed-on: https://review.typo3.org/50431
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Nicole Cordes <typo3@cordes.co>
Tested-by: Nicole Cordes <typo3@cordes.co>
typo3/sysext/core/Classes/Utility/ArrayUtility.php
typo3/sysext/core/Tests/Unit/Utility/ArrayUtilityTest.php

index 6fa222a..3273d9a 100644 (file)
@@ -130,7 +130,10 @@ class ArrayUtility
      */
     public static function getValueByPath(array $array, $path, $delimiter = '/')
     {
-        if (empty($path)) {
+        if (!is_string($path)) {
+            throw new \RuntimeException('Path must be a string', 1477699595);
+        }
+        if ($path === '') {
             throw new \RuntimeException('Path must not be empty', 1341397767);
         }
         // Extract parts of the path
@@ -177,12 +180,12 @@ class ArrayUtility
      */
     public static function setValueByPath(array $array, $path, $value, $delimiter = '/')
     {
-        if (empty($path)) {
-            throw new \RuntimeException('Path must not be empty', 1341406194);
-        }
         if (!is_string($path)) {
             throw new \RuntimeException('Path must be a string', 1341406402);
         }
+        if ($path === '') {
+            throw new \RuntimeException('Path must not be empty', 1341406194);
+        }
         // Extract parts of the path
         $path = str_getcsv($path, $delimiter);
         // Point to the root of the array
@@ -190,7 +193,7 @@ class ArrayUtility
         // Find path in given array
         foreach ($path as $segment) {
             // Fail if the part is empty
-            if (empty($segment)) {
+            if ($segment === '') {
                 throw new \RuntimeException('Invalid path segment specified', 1341406846);
             }
             // Create cell if it doesn't exist
@@ -216,12 +219,12 @@ class ArrayUtility
      */
     public static function removeByPath(array $array, $path, $delimiter = '/')
     {
-        if (empty($path)) {
-            throw new \RuntimeException('Path must not be empty', 1371757718);
-        }
         if (!is_string($path)) {
             throw new \RuntimeException('Path must be a string', 1371757719);
         }
+        if ($path === '') {
+            throw new \RuntimeException('Path must not be empty', 1371757718);
+        }
         // Extract parts of the path
         $path = str_getcsv($path, $delimiter);
         $pathDepth = count($path);
@@ -231,7 +234,7 @@ class ArrayUtility
         foreach ($path as $segment) {
             $currentDepth++;
             // Fail if the part is empty
-            if (empty($segment)) {
+            if ($segment === '') {
                 throw new \RuntimeException('Invalid path segment specified', 1371757720);
             }
             if (!array_key_exists($segment, $pointer)) {
@@ -249,7 +252,7 @@ class ArrayUtility
     /**
      * Sorts an array recursively by key
      *
-     * @param $array Array to sort recursively by key
+     * @param array $array Array to sort recursively by key
      * @return array Sorted array
      */
     public static function sortByKeyRecursive(array $array)
index 527b89b..c11d5b1 100644 (file)
@@ -223,6 +223,17 @@ class ArrayUtilityTest extends UnitTestCase
     /**
      * @test
      * @expectedException \RuntimeException
+     * @expectedExceptionCode 1477699595
+     */
+    public function getValueByPathThrowsExceptionIfPathIsNotString()
+    {
+        ArrayUtility::getValueByPath([], ['']);
+    }
+
+    /**
+     * @test
+     * @expectedException \RuntimeException
+     * @expectedExceptionCode 1341397767
      */
     public function getValueByPathThrowsExceptionIfPathIsEmpty()
     {
@@ -230,6 +241,22 @@ class ArrayUtilityTest extends UnitTestCase
     }
 
     /**
+     * @test
+     */
+    public function getValueByPathReturnsFirstIndexIfPathIsZero()
+    {
+        $this->assertSame('foo', ArrayUtility::getValueByPath(['foo'], '0'));
+    }
+
+    /**
+     * @test
+     */
+    public function getValueByPathReturnsFirstIndexIfPathSegmentIsZero()
+    {
+        $this->assertSame('bar', ArrayUtility::getValueByPath(['foo' => ['bar']], 'foo/0'));
+    }
+
+    /**
      * Data provider for getValueByPathThrowsExceptionIfPathNotExists
      * Every array splits into:
      * - Array to get value from
@@ -240,6 +267,13 @@ class ArrayUtilityTest extends UnitTestCase
     public function getValueByPathInvalidPathDataProvider()
     {
         return [
+            'not existing index' => [
+                [
+                    'foo' => ['foo']
+                ],
+                'foo/1',
+                false
+            ],
             'not existing path 1' => [
                 [
                     'foo' => []
@@ -286,6 +320,7 @@ class ArrayUtilityTest extends UnitTestCase
      * @test
      * @dataProvider getValueByPathInvalidPathDataProvider
      * @expectedException \RuntimeException
+     * @expectedExceptionCode 1341397869
      * @param array $array
      * @param string $path
      */
@@ -427,6 +462,7 @@ class ArrayUtilityTest extends UnitTestCase
     /**
      * @test
      * @expectedException \RuntimeException
+     * @expectedExceptionCode 1341406194
      */
     public function setValueByPathThrowsExceptionIfPathIsEmpty()
     {
@@ -436,6 +472,7 @@ class ArrayUtilityTest extends UnitTestCase
     /**
      * @test
      * @expectedException \RuntimeException
+     * @expectedExceptionCode 1341406402
      */
     public function setValueByPathThrowsExceptionIfPathIsNotAString()
     {
@@ -443,6 +480,32 @@ class ArrayUtilityTest extends UnitTestCase
     }
 
     /**
+     * @test
+     * @expectedException \RuntimeException
+     * @expectedExceptionCode 1341406846
+     */
+    public function setValueByPathThrowsExceptionIfPathSegmentIsEmpty()
+    {
+        ArrayUtility::setValueByPath(['foo' => 'bar'], '/foo', 'value');
+    }
+
+    /**
+     * @test
+     */
+    public function setValueByPathCanUseZeroAsPathSegment()
+    {
+        $this->assertSame(['foo' => ['value']], ArrayUtility::setValueByPath(['foo' => []], 'foo/0', 'value'));
+    }
+
+    /**
+     * @test
+     */
+    public function setValueByPathCanUseZeroAsPath()
+    {
+        $this->assertSame(['value', 'bar'], ArrayUtility::setValueByPath(['foo', 'bar'], '0', 'value'));
+    }
+
+    /**
      * Data provider for setValueByPathSetsCorrectValueDataProvider
      *
      * Every array splits into:
@@ -637,6 +700,7 @@ class ArrayUtilityTest extends UnitTestCase
     /**
      * @test
      * @expectedException \RuntimeException
+     * @expectedExceptionCode 1371757718
      */
     public function removeByPathThrowsExceptionIfPathIsEmpty()
     {
@@ -646,6 +710,7 @@ class ArrayUtilityTest extends UnitTestCase
     /**
      * @test
      * @expectedException \RuntimeException
+     * @expectedExceptionCode 1371757719
      */
     public function removeByPathThrowsExceptionIfPathIsNotAString()
     {
@@ -655,6 +720,7 @@ class ArrayUtilityTest extends UnitTestCase
     /**
      * @test
      * @expectedException \RuntimeException
+     * @expectedExceptionCode 1371757720
      */
     public function removeByPathThrowsExceptionWithEmptyPathSegment()
     {
@@ -668,7 +734,29 @@ class ArrayUtilityTest extends UnitTestCase
 
     /**
      * @test
+     */
+    public function removeByPathRemovesFirstIndexWithZeroAsPathSegment()
+    {
+        $inputArray = [
+            'foo' => ['bar']
+        ];
+        $this->assertSame(['foo' => []], ArrayUtility::removeByPath($inputArray, 'foo/0'));
+    }
+
+    /**
+     * @test
+     */
+    public function removeByPathRemovesFirstIndexWithZeroAsPath()
+    {
+        $inputArray = ['bar'];
+
+        $this->assertSame([], ArrayUtility::removeByPath($inputArray, '0'));
+    }
+
+    /**
+     * @test
      * @expectedException \RuntimeException
+     * @expectedExceptionCode 1371758436
      */
     public function removeByPathThrowsExceptionIfPathDoesNotExistInArray()
     {
@@ -940,6 +1028,7 @@ class ArrayUtilityTest extends UnitTestCase
     /**
      * @test
      * @expectedException \RuntimeException
+     * @expectedExceptionCode 1373727309
      */
     public function sortArraysByKeyThrowsExceptionForNonExistingKey()
     {
@@ -991,6 +1080,7 @@ class ArrayUtilityTest extends UnitTestCase
     /**
      * @test
      * @expectedException \RuntimeException
+     * @expectedExceptionCode 1342294987
      */
     public function arrayExportThrowsExceptionIfObjectShouldBeExported()
     {