Browse Source

Fix Hash::merge() emitting a warning when merging strings.

Check the value types before merging. PHP doesn't handle slice offsets
into strings safely, as non-integer values are cast to 0 when used as
a string offset.

Refs #8819
Mark Story 10 years ago
parent
commit
37dadeeb21
2 changed files with 52 additions and 29 deletions
  1. 4 2
      src/Utility/Hash.php
  2. 48 27
      tests/TestCase/Utility/HashTest.php

+ 4 - 2
src/Utility/Hash.php

@@ -747,9 +747,11 @@ class Hash
         while (!empty($stack)) {
             foreach ($stack as $curKey => &$curMerge) {
                 foreach ($curMerge[0] as $key => &$val) {
-                    if (!empty($curMerge[1][$key]) && (array)$curMerge[1][$key] === $curMerge[1][$key] && (array)$val === $val) {
+                    $isArray = is_array($curMerge[1]);
+                    if ($isArray && !empty($curMerge[1][$key]) && (array)$curMerge[1][$key] === $curMerge[1][$key] && (array)$val === $val) {
+                        // Recurse into the current merge data as it is an array.
                         $stack[] = [&$val, &$curMerge[1][$key]];
-                    } elseif ((int)$key === $key && isset($curMerge[1][$key])) {
+                    } elseif ((int)$key === $key && $isArray && isset($curMerge[1][$key])) {
                         $curMerge[1][] = $val;
                     } else {
                         $curMerge[1][$key] = $val;

+ 48 - 27
tests/TestCase/Utility/HashTest.php

@@ -658,9 +658,6 @@ class HashTest extends TestCase
         $result = Hash::merge(['foo'], ['bar']);
         $this->assertEquals($result, ['foo', 'bar']);
 
-        $result = Hash::merge(['foo'], ['user' => 'bob', 'no-bar'], 'bar');
-        $this->assertEquals($result, ['foo', 'user' => 'bob', 'no-bar', 'bar']);
-
         $a = ['foo', 'foo2'];
         $b = ['bar', 'bar2'];
         $expected = ['foo', 'foo2', 'bar', 'bar2'];
@@ -681,30 +678,6 @@ class HashTest extends TestCase
         $expected = ['users' => 'none'];
         $this->assertEquals($expected, Hash::merge($a, $b));
 
-        $a = ['users' => ['lisa' => ['id' => 5, 'pw' => 'secret']], 'cakephp'];
-        $b = ['users' => ['lisa' => ['pw' => 'new-pass', 'age' => 23]], 'ice-cream'];
-        $expected = [
-            'users' => ['lisa' => ['id' => 5, 'pw' => 'new-pass', 'age' => 23]],
-            'cakephp',
-            'ice-cream'
-        ];
-        $result = Hash::merge($a, $b);
-        $this->assertEquals($expected, $result);
-
-        $c = [
-            'users' => ['lisa' => ['pw' => 'you-will-never-guess', 'age' => 25, 'pet' => 'dog']],
-            'chocolate'
-        ];
-        $expected = [
-            'users' => ['lisa' => ['id' => 5, 'pw' => 'you-will-never-guess', 'age' => 25, 'pet' => 'dog']],
-            'cakephp',
-            'ice-cream',
-            'chocolate'
-        ];
-        $this->assertEquals($expected, Hash::merge($a, $b, $c));
-
-        $this->assertEquals($expected, Hash::merge($a, $b, [], $c));
-
         $a = [
             'Tree',
             'CounterCache',
@@ -737,6 +710,54 @@ class HashTest extends TestCase
     }
 
     /**
+     * Test that merge() works with variadic arguments.
+     *
+     * @return void
+     */
+    public function testMergeVariadic()
+    {
+        $result = Hash::merge(
+            ['hkuc' => ['lion']],
+            ['hkuc' =>'lion']
+        );
+        $expected = ['hkuc' => 'lion'];
+        $this->assertEquals($expected, $result);
+
+        $result = Hash::merge(
+            ['hkuc' => ['lion']],
+            ['hkuc' => ['lion']],
+            ['hkuc' => 'lion']
+        );
+        $this->assertEquals($expected, $result);
+
+        $result = Hash::merge(['foo'], ['user' => 'bob', 'no-bar'], 'bar');
+        $this->assertEquals($result, ['foo', 'user' => 'bob', 'no-bar', 'bar']);
+
+        $a = ['users' => ['lisa' => ['id' => 5, 'pw' => 'secret']], 'cakephp'];
+        $b = ['users' => ['lisa' => ['pw' => 'new-pass', 'age' => 23]], 'ice-cream'];
+        $expected = [
+            'users' => ['lisa' => ['id' => 5, 'pw' => 'new-pass', 'age' => 23]],
+            'cakephp',
+            'ice-cream'
+        ];
+        $result = Hash::merge($a, $b);
+        $this->assertEquals($expected, $result);
+
+        $c = [
+            'users' => ['lisa' => ['pw' => 'you-will-never-guess', 'age' => 25, 'pet' => 'dog']],
+            'chocolate'
+        ];
+        $expected = [
+            'users' => ['lisa' => ['id' => 5, 'pw' => 'you-will-never-guess', 'age' => 25, 'pet' => 'dog']],
+            'cakephp',
+            'ice-cream',
+            'chocolate'
+        ];
+        $this->assertEquals($expected, Hash::merge($a, $b, $c));
+        $this->assertEquals($expected, Hash::merge($a, $b, [], $c));
+    }
+
+    /**
      * test normalizing arrays
      *
      * @return void