Class FilteredIterator: add input validation #604
Conversation
This commit adds input validation to the class constructor.
* The `$data` parameter is documented to expect an `array`, but as the class uses `ArrayIterator`, any iterable should be accepted as valid.
For non-iterable data an exception will now be thrown.
* The `$callback` parameter has to be a valid callback. In contrast to most input validation, for invalid callbacks passed, this parameter will just be ignored (property not set).
This is in line with the handling of the property in the rest of the class, where the `$callback` property may be unset after unserialization anyway.
As for the other methods:
* The `public` magic/ArrayIterator methods should not need input validation as they should not be called directly, but only indirectly and when called that way, will receive the correct input type.
* The other `public` methods do not take parameters.
Includes adding perfunctory tests for the added input validation.
**Open question:**
* Should the input check for the `$callback` parameter even be added ? After all, a type check is already done before using it in the `current()` method anyway.
Yes, the integrity of the class should already be ensured in the constructor. What good is an iterator you can instantiate but then cannot iterate upon? EDIT: Ah, right, the callback is only added/used conditionally. |
| parent::__construct($data); | ||
|
|
||
| $this->callback = $callback; | ||
| if (is_callable($callback)) { |
There was a problem hiding this comment.
As mentioned in the comment, I'd stick with the check here, but remove the one in current() (which runs unnecessarily often).
There was a problem hiding this comment.
We can't remove the check in current() as $callback will be unset when the object is serialized and then unserialized (previous security fix).
This commit adds input validation to the class constructor.
$dataparameter is documented to expect anarray, but as the class usesArrayIterator, any iterable should be accepted as valid.For non-iterable data an exception will now be thrown.
$callbackparameter has to be a valid callback. In contrast to most input validation, for invalid callbacks passed, this parameter will just be ignored (property not set).This is in line with the handling of the property in the rest of the class, where the
$callbackproperty may be unset after unserialization anyway.As for the other methods:
publicmagic/ArrayIterator methods should not need input validation as they should not be called directly, but only indirectly and when called that way, will receive the correct input type.publicmethods do not take parameters.Includes adding perfunctory tests for the added input validation.
Open question:
$callbackparameter even be added ? After all, a type check is already done before using it in thecurrent()method anyway.