Skip to content

Commit 62e8973

Browse files
committed
Throw exceptions from Hash::combine()
When the key + value counts do not match Hash should throw an exception. Silently doing the wrong thing is generally not a good idea. While this change could break existing applications, those applications were probably behaving incorrectly anyways. Fixes #2470
1 parent df4b978 commit 62e8973

File tree

2 files changed

+36
-3
lines changed

2 files changed

+36
-3
lines changed

lib/Cake/Test/Case/Utility/HashTest.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,6 +1473,34 @@ public function testCombine() {
14731473
$this->assertEquals($expected, $result);
14741474
}
14751475

1476+
/**
1477+
* test combine() giving errors on key/value length mismatches.
1478+
*
1479+
* @expectedException CakeException
1480+
* @return void
1481+
*/
1482+
public function testCombineErrorMissingValue() {
1483+
$data = array(
1484+
array('User' => array('id' => 1, 'name' => 'mark')),
1485+
array('User' => array('name' => 'jose')),
1486+
);
1487+
Hash::combine($data, '{n}.User.id', '{n}.User.name');
1488+
}
1489+
1490+
/**
1491+
* test combine() giving errors on key/value length mismatches.
1492+
*
1493+
* @expectedException CakeException
1494+
* @return void
1495+
*/
1496+
public function testCombineErrorMissingKey() {
1497+
$data = array(
1498+
array('User' => array('id' => 1, 'name' => 'mark')),
1499+
array('User' => array('id' => 2)),
1500+
);
1501+
Hash::combine($data, '{n}.User.id', '{n}.User.name');
1502+
}
1503+
14761504
/**
14771505
* test combine() with a group path.
14781506
*

lib/Cake/Utility/Hash.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -346,10 +346,15 @@ public static function combine(array $data, $keyPath, $valuePath = null, $groupP
346346
} elseif (!empty($valuePath)) {
347347
$vals = self::extract($data, $valuePath);
348348
}
349+
if (empty($vals)) {
350+
$vals = array_fill(0, count($keys), null);
351+
}
349352

350-
$count = count($keys);
351-
for ($i = 0; $i < $count; $i++) {
352-
$vals[$i] = isset($vals[$i]) ? $vals[$i] : null;
353+
if (count($keys) !== count($vals)) {
354+
throw new CakeException(__d(
355+
'cake_dev',
356+
'Hash::combine() needs an equal number of keys + values.'
357+
));
353358
}
354359

355360
if ($groupPath !== null) {

0 commit comments

Comments
 (0)