Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn on actual alterations #6

Open
weierophinney opened this issue Dec 31, 2019 · 8 comments
Open

Warn on actual alterations #6

weierophinney opened this issue Dec 31, 2019 · 8 comments

Comments

@weierophinney
Copy link
Member

Our project is using Zend filters for security purposes. For example, we might want to ensure that a text input contains an integer using Zend\Filter\ToInt. filter() gives us the guarantee that the returned value is an integer. Unfortunately, it may quietly alter the input.

If the input is string "3" and the output is int 3, there is no problem.
But if the input is string "four" and the output is 0, there is a problem - either the wrong filter was used, or the UI should not have let "four" be entered.

To ensure that problems are detected, filter() should warn when an actual alteration is done, for example by triggering a PHP warning.


Originally posted by @Chealer at zendframework/zend-filter#63

@weierophinney
Copy link
Member Author

zend-filter is not a validation library. It's purpose is to transform data and/or normalize data.

The proper place to do checks like this is within a validator.

If you are using this particular filter with a data set used within zend-inputfilter, what I'd do instead is use the validator Zend\Validator\Digits to ensure every value within the string is a numeric digit.


Originally posted by @weierophinney at zendframework/zend-filter#63 (comment)

@weierophinney
Copy link
Member Author

Thank you @weierophinney ,
We do want data normalization. If we have a string which contains "00", we do want to get a null int. What we do not want, however, is that even strings which are not a valid way to represent 0, such as "four", end up quietly converted to 0. If filter() returns 0 for such cases, we expect the call to trigger a warning.

Note that while I requested a warning to be triggered, I would find it a proper solution too to just refuse to guess a result and - for example - throw an exception. To phrase this differently, "don't quietly take a guess".


Originally posted by @Chealer at zendframework/zend-filter#63 (comment)

@weierophinney
Copy link
Member Author

@Chealer
The filter uses PHP's normal behaviour for conversion from string to numbers:

If the string starts with valid numeric data, this will be the value used. Otherwise, the value will be 0 (zero).

http://php.net/manual/en/language.types.string.php#language.types.string.conversion

If your use case doesn't allow this conversion, than add a validator in front.


Originally posted by @froschdesign at zendframework/zend-filter#63 (comment)

@weierophinney
Copy link
Member Author

@Chealer Another route is to create a custom filter, if the current one does not implement the behavior you require.

Typically, we do not want our filters to raise exceptions. If they cannot filter the value, they should return the original value verbatim. So, in this case, I'd write a filter that checks to ensure the value is not malformed, and, if not, filters it, but otherwise returns it verbatim. Then use a validator to see if the value is valid for the domain.

Alternately, if you really want the exception and/or error, trigger it.

But we do not plan to implement such a feature in zend-filter, for the reasons stated above.


Originally posted by @weierophinney at zendframework/zend-filter#63 (comment)

@weierophinney
Copy link
Member Author

@froschdesign , I read ToInt's code and understand why it happens. If your point is that PHP itself already has a problematic behavior, I do not disagree.

It's not really that a specific use case doesn't allow such conversion. We use Zend filters to normalize inputs throughout the application. The designers must not have realized this problem, but this "filtering" is often quietly altering input and complicating debugging.


Originally posted by @Chealer at zendframework/zend-filter#63 (comment)

@weierophinney
Copy link
Member Author

Thank you @weierophinney ,
I understand that we can work around this, but this requires some effort, and our philosophy leads us to contribute to our upstreams.

If there is no real solution in sight, I recommend mitigating through documentation.


Originally posted by @Chealer at zendframework/zend-filter#63 (comment)

@weierophinney
Copy link
Member Author

@Chealer We'd gladly accept documentation to cover this scenario. Docs are in the repository itself.


Originally posted by @weierophinney at zendframework/zend-filter#63 (comment)

@weierophinney
Copy link
Member Author

@Chealer

If your point is that PHP itself already has a problematic behavior…

No, but we should document this behavior for the filter.


Originally posted by @froschdesign at zendframework/zend-filter#63 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

1 participant