[BUGFIX] Allow "0" as path segment in ArrayUtility 30/50430/8
authorHelmut Hummel <info@helhum.io>
Fri, 28 Oct 2016 23:17:53 +0000 (01:17 +0200)
committerChristian Kuhn <lolli@schwarzbu.ch>
Sat, 29 Oct 2016 09:40:24 +0000 (11:40 +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/50430
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Joerg Boesche <typo3@joergboesche.de>
Tested-by: Joerg Boesche <typo3@joergboesche.de>
Reviewed-by: Nicole Cordes <typo3@cordes.co>
Tested-by: Nicole Cordes <typo3@cordes.co>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
typo3/sysext/core/Classes/Utility/ArrayUtility.php
typo3/sysext/core/Tests/Unit/Utility/ArrayUtilityTest.php

index 6547186..6597588 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 8428064..c271cc4 100644 (file)
@@ -223,6 +223,17 @@ class ArrayUtilityTest extends UnitTestCase
     /**
      * @test
      */
+    public function getValueByPathThrowsExceptionIfPathIsNotString()
+    {
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionCode(1477699595);
+
+        ArrayUtility::getValueByPath([], ['']);
+    }
+
+    /**
+     * @test
+     */
     public function getValueByPathThrowsExceptionIfPathIsEmpty()
     {
         $this->expectException(\RuntimeException::class);
@@ -232,6 +243,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
@@ -242,6 +269,13 @@ class ArrayUtilityTest extends UnitTestCase
     public function getValueByPathInvalidPathDataProvider()
     {
         return [
+            'not existing index' => [
+                [
+                    'foo' => ['foo']
+                ],
+                'foo/1',
+                false
+            ],
             'not existing path 1' => [
                 [
                     'foo' => []
@@ -451,6 +485,33 @@ class ArrayUtilityTest extends UnitTestCase
     }
 
     /**
+     * @test
+     */
+    public function setValueByPathThrowsExceptionIfPathSegmentIsEmpty()
+    {
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionCode(1341406846);
+
+        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:
@@ -684,6 +745,28 @@ 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
+     */
     public function removeByPathThrowsExceptionIfPathDoesNotExistInArray()
     {
         $inputArray = [