Skip to content

Commit 2d600fb

Browse files
schlesseraschlessera
authored
Merge pull request #573 from WordPress/feature/hooks-add-input-validation
2 parents 2d78a4d + 94678c0 commit 2d600fb

2 files changed

Lines changed: 207 additions & 2 deletions

File tree

‎src/Hooks.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88

99
namespace WpOrg\Requests;
1010

11+
use WpOrg\Requests\Exception\InvalidArgument;
1112
use WpOrg\Requests\Hooker;
13+
use WpOrg\Requests\Utility\InputValidator;
1214

1315
/**
1416
* Handles adding and dispatching events
@@ -30,8 +32,23 @@ class Hooks implements Hooker {
3032
* @param string $hook Hook name
3133
* @param callback $callback Function/method to call on event
3234
* @param int $priority Priority number. <0 is executed earlier, >0 is executed later
35+
* @throws \WpOrg\Requests\Exception\InvalidArgument When the passed $hook argument is not a string.
36+
* @throws \WpOrg\Requests\Exception\InvalidArgument When the passed $callback argument is not callable.
37+
* @throws \WpOrg\Requests\Exception\InvalidArgument When the passed $priority argument is not an integer.
3338
*/
3439
public function register($hook, $callback, $priority = 0) {
40+
if (is_string($hook) === false) {
41+
throw InvalidArgument::create(1, '$hook', 'string', gettype($hook));
42+
}
43+
44+
if (is_callable($callback) === false) {
45+
throw InvalidArgument::create(2, '$callback', 'callable', gettype($callback));
46+
}
47+
48+
if (InputValidator::is_numeric_array_key($priority) === false) {
49+
throw InvalidArgument::create(3, '$priority', 'integer', gettype($priority));
50+
}
51+
3552
if (!isset($this->hooks[$hook])) {
3653
$this->hooks[$hook] = array(
3754
$priority => array(),
@@ -49,8 +66,19 @@ public function register($hook, $callback, $priority = 0) {
4966
* @param string $hook Hook name
5067
* @param array $parameters Parameters to pass to callbacks
5168
* @return boolean Successfulness
69+
* @throws \WpOrg\Requests\Exception\InvalidArgument When the passed $hook argument is not a string.
70+
* @throws \WpOrg\Requests\Exception\InvalidArgument When the passed $parameters argument is not an array.
5271
*/
5372
public function dispatch($hook, $parameters = array()) {
73+
if (is_string($hook) === false) {
74+
throw InvalidArgument::create(1, '$hook', 'string', gettype($hook));
75+
}
76+
77+
// Check strictly against array, as Array* objects don't work in combination with `call_user_func_array()`.
78+
if (is_array($parameters) === false) {
79+
throw InvalidArgument::create(2, '$parameters', 'array', gettype($parameters));
80+
}
81+
5482
if (empty($this->hooks[$hook])) {
5583
return false;
5684
}

‎tests/HooksTest.php‎

Lines changed: 179 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@
22

33
namespace WpOrg\Requests\Tests;
44

5+
use Closure;
6+
use stdClass;
7+
use WpOrg\Requests\Exception\InvalidArgument;
58
use WpOrg\Requests\Hooks;
9+
use WpOrg\Requests\Tests\Fixtures\ArrayAccessibleObject;
10+
use WpOrg\Requests\Tests\Fixtures\StringableObject;
611
use WpOrg\Requests\Tests\TestCase;
712

813
/**
@@ -70,8 +75,11 @@ public function testRegister() {
7075
'Registering a second callback on the same hook with the same priority failed'
7176
);
7277

73-
// Verify that new subkeys are created when needed.
74-
$this->hooks->register('hookname', 'is_int', 10);
78+
/*
79+
* Verify that new subkeys are created when needed.
80+
* Also verifies that the input validation isn't too strict for the priority.
81+
*/
82+
$this->hooks->register('hookname', 'is_int', '10');
7583
$this->assertSame(
7684
array(
7785
'hookname' => array(
@@ -89,6 +97,29 @@ public function testRegister() {
8997
);
9098
}
9199

100+
/**
101+
* Technical test to verify and safeguard Hooks::register() accepts closure callbacks.
102+
*
103+
* @covers ::register
104+
*
105+
* @return void
106+
*/
107+
public function testRegisterClosureCallback() {
108+
$this->hooks->register(
109+
'hookname',
110+
function($param) {
111+
return true;
112+
}
113+
);
114+
115+
$hooks_prop = $this->getPropertyValue($this->hooks, 'hooks');
116+
117+
$this->assertArrayHasKey('hookname', $hooks_prop, '$hooks property does not have key ["hookname"]');
118+
$this->assertArrayHasKey(0, $hooks_prop['hookname'], '$hooks property does not have key ["hookname"][0]');
119+
$this->assertArrayHasKey(0, $hooks_prop['hookname'][0], '$hooks property does not have key ["hookname"][0][0]');
120+
$this->assertInstanceof(Closure::class, $hooks_prop['hookname'][0][0], 'Closure callback is not registered correctly');
121+
}
122+
92123

93124
/**
94125
* Verify that the return value of the dispatch method is false when no hooks are registered.
@@ -171,6 +202,152 @@ public function testDispatchWithMultipleRegisteredHooks() {
171202
$this->assertTrue($this->hooks->dispatch('hook_b', array(10, 'text')));
172203
}
173204

205+
/**
206+
* Tests receiving an exception when an invalid input type is passed to `register()` as `$hook`.
207+
*
208+
* @dataProvider dataInvalidHookname
209+
*
210+
* @covers ::register
211+
*
212+
* @param mixed $input Invalid hook name input.
213+
*
214+
* @return void
215+
*/
216+
public function testRegisterInvalidHookname($input) {
217+
$this->expectException(InvalidArgument::class);
218+
$this->expectExceptionMessage('Argument #1 ($hook) must be of type string');
219+
220+
$this->hooks->register($input, 'is_string');
221+
}
222+
223+
/**
224+
* Tests receiving an exception when an invalid input type is passed to `dispatch()` as `$hook`.
225+
*
226+
* @dataProvider dataInvalidHookname
227+
*
228+
* @covers ::dispatch
229+
*
230+
* @param mixed $input Invalid hook name input.
231+
*
232+
* @return void
233+
*/
234+
public function testDispatchInvalidHookname($input) {
235+
$this->expectException(InvalidArgument::class);
236+
$this->expectExceptionMessage('Argument #1 ($hook) must be of type string');
237+
238+
$this->hooks->dispatch($input);
239+
}
240+
241+
/**
242+
* Data Provider.
243+
*
244+
* @return array
245+
*/
246+
public function dataInvalidHookname() {
247+
return array(
248+
'null' => array(null),
249+
'float' => array(1.1),
250+
'stringable object' => array(new StringableObject('value')),
251+
);
252+
}
253+
254+
/**
255+
* Tests receiving an exception when an invalid input type is passed to `register()` as `$callback`.
256+
*
257+
* @dataProvider dataRegisterInvalidCallback
258+
*
259+
* @covers ::register
260+
*
261+
* @param mixed $input Invalid callback.
262+
*
263+
* @return void
264+
*/
265+
public function testRegisterInvalidCallback($input) {
266+
$this->expectException(InvalidArgument::class);
267+
$this->expectExceptionMessage('Argument #2 ($callback) must be of type callable');
268+
269+
$this->hooks->register('hookname', $input);
270+
}
271+
272+
/**
273+
* Data Provider.
274+
*
275+
* @return array
276+
*/
277+
public function dataRegisterInvalidCallback() {
278+
return array(
279+
'null' => array(null),
280+
'non-existent function' => array('functionname'),
281+
'non-existent method' => array(array($this, 'dummyCallbackDoesNotExist')),
282+
'empty array' => array(array()),
283+
'plain object' => array(new stdClass(), 'method'),
284+
);
285+
}
286+
287+
/**
288+
* Tests receiving an exception when an invalid input type is passed to `register()` as `$priority`.
289+
*
290+
* @dataProvider dataRegisterInvalidPriority
291+
*
292+
* @covers ::register
293+
*
294+
* @param mixed $input Invalid priority.
295+
*
296+
* @return void
297+
*/
298+
public function testRegisterInvalidPriority($input) {
299+
$this->expectException(InvalidArgument::class);
300+
$this->expectExceptionMessage('Argument #3 ($priority) must be of type int');
301+
302+
$this->hooks->register('hookname', array($this, 'dummyCallback1'), $input);
303+
}
304+
305+
/**
306+
* Data Provider.
307+
*
308+
* @return array
309+
*/
310+
public function dataRegisterInvalidPriority() {
311+
return array(
312+
'null' => array(null),
313+
'float' => array(1.1),
314+
'string "123 abc"' => array('123 abc'),
315+
);
316+
}
317+
318+
/**
319+
* Tests receiving an exception when an invalid input type is passed to `dispatch()` as `$parameters`.
320+
*
321+
* @dataProvider dataDispatchInvalidParameters
322+
*
323+
* @covers ::dispatch
324+
*
325+
* @param mixed $input Invalid parameters array.
326+
*
327+
* @return void
328+
*/
329+
public function testDispatchInvalidParameters($input) {
330+
$this->expectException(InvalidArgument::class);
331+
$this->expectExceptionMessage('Argument #2 ($parameters) must be of type array');
332+
333+
$this->hooks->dispatch('hookname', $input);
334+
}
335+
336+
/**
337+
* Data Provider.
338+
*
339+
* @return array
340+
*/
341+
public function dataDispatchInvalidParameters() {
342+
return array(
343+
'null' => array(null),
344+
'bool false' => array(false),
345+
'float' => array(1.1),
346+
'string' => array('param'),
347+
'object implementing ArrayAccess' => array(new ArrayAccessibleObject()),
348+
);
349+
}
350+
174351
/**
175352
* Dummy callback method.
176353
*

0 commit comments

Comments
 (0)