Remove exceptions from #4734

[weierophinney]

#4734 introduced some type checking into a number of filters in order to remove notices that were being raised about "array to string" conversions. However, it did so by raising exceptions, which will be a non-starter for anybody who was previously passing arrays and/or objects to these filters.

Likely the more appropriate method would be to simply return the value presented, and potentially raise an E_USER_WARNING indicating the filter's inability to process the value.

[gag237]

If we accept an agreement that any Filter instance of Zend\Filter\FilterInterface should generate an error if it can not filter out, it is best to leave the exception.

I want to draw attention to such cases:

Lets we have $input (instance of InputFilrer\Input) based on config below

$inputSpec = array(
    'title' => array(
           'required' => false,
            'validators' => array(
                array(
                    'name' => 'stringLength',
                    'options' => array(
                        'max' => 2000
                    )
                )
            ),
            'filters' => array(
                array(
                    'name' => 'stripTags',
                ),
                array(
                    'name' => 'null',
                ),
            ),
     )
);

and we have any $data (of course not valid).

$data = array(
  'title' => array()
);

then i call $input->isValid() will be triggered notice.

If this data is from form it's logical trigger notice to developer to show that form does not correspond to inputFilter. But this data may be from any part, for example from GET.
And i think it is not good idea trigger notice, better leave Exception, because possible to make small changes in InputFilter\Input something like that:

public function getValue()
{
    $filter = $this->getFilterChain();
    try {
        return $filter->filter($this->value);
    } catch (\Zend\Filter\Exception\InputFilterException $e) {
       return $this->getRawValue();
    }
}

[weierophinney]

@gag237 It's a bad idea to use try/catch for logical branching -- if nothing else, because it adds a ton of processing overhead. Additionally, you're confusing filtering and validation. In the InputFilter, filters happen prior to validation -- so even if the values cannot be filtered, they will likely still fail validation.

Throwing the notice is still useful in these situations, however, as you can then see in the logs that some location on your site is submitting data incorrectly -- or somebody is attempting to trigger failures to try and find vulnerabilities.

[gag237]

Perhaps you are right.

Is it possible to change the description of Filter\FilterInterfatse::filter() when the filter should throw an exception RuntimeException, when trigger error.

[weierophinney]

@gag237 Go for it.

[Maks3w]

That methods have a contract for return string but with this approach are returning null or mixed

If this try to resolve a BC Break I suggest revert the original PR (#4734) and apply it for the next "major" version

[weierophinney]

@Maks3w It's not a BC break; it's correcting already released behavior to be more consistent and less brittle.

[Maks3w]

Can you update the docblocks for reflect the changes? (return types and some note about the E_USER_WARNING)

[weierophinney]

@Maks3w Done.