Skip to content

Commit 4924870

Browse files
lstrojnylstrojny
authored
Prevent hash collisions for object hashing (#225)
1 parent 52fc803 commit 4924870

File tree

8 files changed

+342
-52
lines changed

8 files changed

+342
-52
lines changed

‎.travis.yml‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
language: php
2-
sudo: false
2+
3+
cache:
4+
directories:
5+
- $HOME/.composer/cache/files
6+
- vendor
37

48
php:
59
- 7.1

‎composer.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@
114114
"src/Functional/True.php",
115115
"src/Functional/Truthy.php",
116116
"src/Functional/Unique.php",
117+
"src/Functional/ValueToKey.php",
117118
"src/Functional/With.php",
118119
"src/Functional/Zip.php",
119120
"src/Functional/ZipAll.php"

‎docs/functional-php.md‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
- [Partial application](#partial-application)
2222
- [Introduction](#introduction)
2323
- [partial_left() & partial_right()](#partial_left--partial_right)
24+
- [ary()](#ary)
2425
- [partial_any()](#partial_any)
2526
- [partial_method()](#partial_method)
2627
- [converge()](#converge)
@@ -49,7 +50,8 @@
4950
- [compose()](#compose)
5051
- [tail_recursion()](#tail_recursion)
5152
- [flip()](#flip)
52-
- [Other](#other)
53+
- [not](#not)
54+
- [Other](#other-1)
5355
- [Mathematical functions](#mathematical-functions)
5456
- [Transformation functions](#transformation-functions)
5557
- [partition()](#partition)
@@ -58,13 +60,14 @@
5860
- [flatten()](#flatten)
5961
- [reduce_left() & reduce_right()](#reduce_left--reduce_right)
6062
- [intersperse()](#intersperse)
61-
- [Other](#other-1)
63+
- [Other](#other-2)
6264
- [Conditional functions](#conditional-functions)
6365
- [if_else()](#if_else)
6466
- [match()](#match)
6567
- [Higher order comparison functions](#higher-order-comparison-functions)
66-
- [compare_on() & compare_object_hash_on()](#compare_on--compare_object_hash_on)
68+
- [compare_on & compare_object_hash_on](#compare_on--compare_object_hash_on)
6769
- [Miscellaneous](#miscellaneous)
70+
- [concat()](#concat)
6871
- [const_function()](#const_function)
6972
- [id()](#id)
7073
- [tap()](#tap)
@@ -904,9 +907,12 @@ var_dump($is_odd(2)); // false
904907

905908
## Other
906909

907-
`mixed Functional\memoize(callable $callback[, array $arguments = []], [mixed $key = null]])`
910+
`mixed Functional\memoize(callable $callback[, array $arguments = []], [string|array $key = null]])`
908911
Returns and stores the result of the function call. Second call to the same function will return the same result without calling the function again
909912

913+
`string value_to_key(...$values)`
914+
Builds an array key out of any values, correctly handling object identity and traversables. Resources are not supported
915+
910916
# Mathematical functions
911917

912918
`mixed Functional\maximum(array|Traversable $collection)`

‎src/Functional/Functional.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,11 @@ final class Functional
474474
*/
475475
const unique = '\Functional\unique';
476476

477+
/**
478+
* @see \Functional\value_to_key
479+
*/
480+
const value_to_key = '\Functional\value_to_key';
481+
477482
/**
478483
* @see \Functional\with
479484
*/

‎src/Functional/Memoize.php‎

Lines changed: 9 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
namespace Functional;
1212

13-
use Functional\Exceptions\InvalidArgumentException;
13+
use const E_USER_DEPRECATED;
1414

1515
/**
1616
* Memoizes callbacks and returns their value instead of calling them
@@ -23,42 +23,24 @@
2323
function memoize(callable $callback = null, $arguments = [], $key = null)
2424
{
2525
static $storage = [];
26-
2726
if ($callback === null) {
2827
$storage = [];
2928

3029
return null;
3130
}
3231

33-
if (\is_callable($arguments)) {
34-
$key = $arguments;
35-
$arguments = [];
36-
} else {
37-
InvalidArgumentException::assertCollection($arguments, __FUNCTION__, 2);
38-
}
39-
40-
static $keyGenerator = null;
41-
if (!$keyGenerator) {
42-
$keyGenerator = function ($value) use (&$keyGenerator) {
43-
$type = \gettype($value);
44-
if ($type === 'array') {
45-
$key = \join(':', map($value, $keyGenerator));
46-
} elseif ($type === 'object') {
47-
$key = \get_class($value) . ':' . \spl_object_hash($value);
48-
} else {
49-
$key = (string) $value;
50-
}
51-
52-
return $key;
53-
};
32+
if (\is_callable($key)) {
33+
\trigger_error('Passing a callable as key is deprecated and will be removed in 2.0', E_USER_DEPRECATED);
34+
$key = $key();
35+
} elseif (\is_callable($arguments)) {
36+
\trigger_error('Passing a callable as key is deprecated and will be removed in 2.0', E_USER_DEPRECATED);
37+
$key = $arguments();
5438
}
5539

5640
if ($key === null) {
57-
$key = $keyGenerator(\array_merge([$callback], $arguments));
58-
} elseif (\is_callable($key)) {
59-
$key = $keyGenerator($key());
41+
$key = value_to_key(\array_merge([$callback], $arguments));
6042
} else {
61-
$key = $keyGenerator($key);
43+
$key = value_to_key($key);
6244
}
6345

6446
if (!isset($storage[$key]) && !\array_key_exists($key, $storage)) {

‎src/Functional/ValueToKey.php‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
<?php
2+
3+
/**
4+
* @package Functional-php
5+
* @author Lars Strojny <lstrojny@php.net>
6+
* @copyright 2011-2017 Lars Strojny
7+
* @license https://opensource.org/licenses/MIT MIT
8+
* @link https://github.com/lstrojny/functional-php
9+
*/
10+
11+
namespace Functional;
12+
13+
use Functional\Exceptions\InvalidArgumentException;
14+
use Traversable;
15+
use WeakReference;
16+
17+
use function serialize;
18+
19+
use const PHP_VERSION_ID;
20+
21+
function value_to_key(...$any)
22+
{
23+
/** @var object[]|WeakReference[] $objectReferences */
24+
static $objectReferences = [];
25+
26+
static $objectToRef = null;
27+
if (!$objectToRef) {
28+
$objectToRef = static function ($value) use (&$objectReferences) {
29+
$hash = \spl_object_hash($value);
30+
/**
31+
* spl_object_hash() will return the same hash twice in a single request if an object goes out of scope
32+
* and is destructed.
33+
*/
34+
if (PHP_VERSION_ID >= 70400) {
35+
/**
36+
* For PHP >=7.4, we keep a weak reference to the relevant object that we use for hashing. Once the
37+
* object gets out of scope, the weak ref will no longer return the object, that’s how we know we
38+
* have a collision and increment a version in the collisions array.
39+
*/
40+
/** @var int[] $collisions */
41+
static $collisions = [];
42+
43+
if (isset($objectReferences[$hash])) {
44+
if ($objectReferences[$hash]->get() === null) {
45+
$collisions[$hash] = ($collisions[$hash] ?? 0) + 1;
46+
$objectReferences[$hash] = WeakReference::create($value);
47+
}
48+
} else {
49+
$objectReferences[$hash] = WeakReference::create($value);
50+
}
51+
52+
$key = \get_class($value) . ':' . $hash . ':' . ($collisions[$hash] ?? 0);
53+
} else {
54+
/**
55+
* For PHP < 7.4 we keep a static reference to the object so that cannot accidentally go out of
56+
* scope and mess with the object hashes
57+
*/
58+
$objectReferences[$hash] = $value;
59+
$key = \get_class($value) . ':' . $hash;
60+
}
61+
return $key;
62+
};
63+
}
64+
65+
static $valueToRef = null;
66+
if (!$valueToRef) {
67+
$valueToRef = static function ($value, $key = null) use (&$valueToRef, $objectToRef) {
68+
$type = \gettype($value);
69+
if ($type === 'array') {
70+
$ref = '[' . \implode(':', map($value, $valueToRef)) . ']';
71+
} elseif ($value instanceof Traversable) {
72+
$ref = $objectToRef($value) . '[' . \implode(':', map($value, $valueToRef)) . ']';
73+
} elseif ($type === 'object') {
74+
$ref = $objectToRef($value);
75+
} elseif ($type === 'resource') {
76+
throw new InvalidArgumentException(
77+
'Resource type cannot be used as part of a memoization key. Please pass a custom key instead'
78+
);
79+
} else {
80+
$ref = \serialize($value);
81+
}
82+
83+
return ($key !== null ? ($valueToRef($key) . '~') : '') . $ref;
84+
};
85+
}
86+
87+
return $valueToRef($any);
88+
}

‎tests/Functional/MemoizeTest.php‎

Lines changed: 59 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
use PHPUnit_Framework_MockObject_MockObject as MockObject;
1414
use BadMethodCallException;
15+
use RuntimeException;
1516

1617
use function Functional\memoize;
1718

@@ -123,7 +124,6 @@ public function testMemoizeWithCustomKey()
123124

124125
$this->assertSame('FOO BAR', memoize([$this->callback, 'execute'], ['FOO', 'BAR'], 'MY:CUSTOM:KEY'));
125126
$this->assertSame('FOO BAR', memoize([$this->callback, 'execute'], ['BAR', 'BAZ'], 'MY:CUSTOM:KEY'), 'Result already memoized');
126-
$this->assertSame('FOO BAR', memoize([$this->callback, 'execute'], ['BAR', 'BAZ'], ['MY', 'CUSTOM', 'KEY']), 'Result already memoized');
127127

128128
$this->assertSame('BAR BAZ', memoize([$this->callback, 'execute'], ['BAR', 'BAZ'], 'MY:DIFFERENT:KEY'));
129129
$this->assertSame('BAR BAZ', memoize([$this->callback, 'execute'], ['BAR', 'BAZ'], 'MY:DIFFERENT:KEY'), 'Result already memoized');
@@ -150,25 +150,6 @@ public function testResultIsNotStoredIfExceptionIsThrown()
150150
}
151151
}
152152

153-
public function testPassKeyGeneratorCallable()
154-
{
155-
$this->callback
156-
->expects($this->exactly(2))
157-
->method('execute');
158-
159-
$keyGenerator = function () {
160-
static $index;
161-
return ($index++ % 2) === 0;
162-
};
163-
164-
memoize([$this->callback, 'execute'], $keyGenerator);
165-
memoize([$this->callback, 'execute'], [], $keyGenerator);
166-
memoize([$this->callback, 'execute'], [], $keyGenerator);
167-
memoize([$this->callback, 'execute'], $keyGenerator);
168-
memoize([$this->callback, 'execute'], $keyGenerator);
169-
memoize([$this->callback, 'execute'], [], $keyGenerator);
170-
}
171-
172153
public function testResetByPassingNullAsCallable()
173154
{
174155
$this->callback
@@ -189,4 +170,62 @@ public function testPassNoCallable()
189170
$this->expectArgumentError('Argument 1 passed to Functional\memoize() must be callable');
190171
memoize('invalidFunction');
191172
}
173+
174+
public function testSplObjectHashCollisions()
175+
{
176+
self::assertSame(0, memoize(self::createFn(0, 1)));
177+
self::assertSame(1, memoize(self::createFn(1, 1)));
178+
self::assertSame(2, memoize(self::createFn(2, 1)));
179+
}
180+
181+
private static function createFn(int $id, int $number): callable
182+
{
183+
return new class ($id, $number) {
184+
private $id;
185+
private $expectedInvocations;
186+
private $actualInvocations = 0;
187+
188+
public function __construct(int $id, int $expectedInvocations)
189+
{
190+
$this->id = $id;
191+
$this->expectedInvocations = $expectedInvocations;
192+
}
193+
194+
public function getId(): int
195+
{
196+
return $this->id;
197+
}
198+
199+
public function __invoke(): int
200+
{
201+
$this->actualInvocations++;
202+
if ($this->actualInvocations > $this->expectedInvocations) {
203+
throw new RuntimeException(
204+
sprintf(
205+
'ID %d: Expected %d invocations, got %d',
206+
$this->id,
207+
$this->expectedInvocations,
208+
$this->actualInvocations
209+
)
210+
);
211+
}
212+
213+
return $this->id;
214+
}
215+
216+
public function __destruct()
217+
{
218+
if ($this->actualInvocations !== $this->expectedInvocations) {
219+
throw new RuntimeException(
220+
sprintf(
221+
'ID %d: Expected %d invocations, got %d',
222+
$this->id,
223+
$this->expectedInvocations,
224+
$this->actualInvocations
225+
)
226+
);
227+
}
228+
}
229+
};
230+
}
192231
}

0 commit comments

Comments
 (0)