Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » Adding validate_var_array()/validate_input_array() to which version? »

Adding validate_var_array()/validate_input_array() to which version?

php.internals

Yasuo Ohgaki

10 years ago
Hi all, We have filter_var_array()/filter_input_array() currently. They are designed as filter functions. i.e. They convert offending elements to NULL/FALSE. Therefore, it's difficult to validate and see if inputs are valid with specified specifications. https://github.com/php/php-src/pull/2048 This patch adds true validation functions - validate_var_array() - Almost the same as filter_var_array() except it returns scalar FALSE on validation failure(s), instead of filtered array. - validate_input_array() - Almost the same as filter_input_array() except it returns scalar FALSE on validation failure(s), instead of filtered array. These functions are handy for input validation that stops script execution upon invalid(attacker's) inputs. Question is which version should I target for? It's simple enough patch to be merged to 7.1. IMO. Comments are appreciated! Regards, P.S. It's possible to return array that contains offending values. It is not included since users can store whole offending input array. Whole input is more useful for attack analysis.
-- Yasuo Ohgaki yohgaki@ohgaki.net

Christoph Becker

10 years ago
On 01.08.2016 at 10:23, Yasuo Ohgaki wrote:
> We have filter_var_array()/filter_input_array() currently. They are > designed as filter functions. i.e. They convert offending elements to > NULL/FALSE. Therefore, it's difficult to validate and see if inputs > are valid with specified specifications.
Maybe I'm missing something, but wouldn't a comparision of the original array with the filtered array already suffice, see <https://3v4l.org/GlT49>?
-- Christoph M. Becker

Yasuo Ohgaki

10 years ago
Hi Christoph, On Mon, Aug 1, 2016 at 7:08 PM, Christoph Becker <cmbecker69@gmx.de> wrote:
>> We have filter_var_array()/filter_input_array() currently. They are >> designed as filter functions. i.e. They convert offending elements to >> NULL/FALSE. Therefore, it's difficult to validate and see if inputs >> are valid with specified specifications. > > Maybe I'm missing something, but wouldn't a comparision of the original > array with the filtered array already suffice, see <https://3v4l.org/GlT49>?
It may work, but it changes data types when it's possible. Float value could be headache. https://3v4l.org/3DUmn - Comparing array by == is too slow for extensive use of validations. - "Validate and reject" is better than "Filter and accept" for most applications. Regards,
-- Yasuo Ohgaki yohgaki@ohgaki.net

Yasuo Ohgaki

10 years ago
On Mon, Aug 1, 2016 at 5:23 PM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
> We have filter_var_array()/filter_input_array() currently. They are > designed as filter functions. i.e. They convert offending elements to > NULL/FALSE. Therefore, it's difficult to validate and see if inputs > are valid with specified specifications. > > https://github.com/php/php-src/pull/2048 > > This patch adds true validation functions > - validate_var_array() - Almost the same as filter_var_array() except > it returns scalar FALSE on validation failure(s), instead of filtered > array. > - validate_input_array() - Almost the same as filter_input_array() > except it returns scalar FALSE on validation failure(s), instead of > filtered array. > > > These functions are handy for input validation that stops script > execution upon invalid(attacker's) inputs. > > Question is which version should I target for? > It's simple enough patch to be merged to 7.1. IMO. > > Comments are appreciated! > > Regards,
Raising Exception would be prefered. Any comment raising exception? ExceptionFilterValidate wouldn't cause much BC, IMO. Regards,
-- Yasuo Ohgaki yohgaki@ohgaki.net

Yasuo Ohgaki

10 years ago
Hi all, On Tue, Aug 2, 2016 at 7:12 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
> > Raising Exception would be prefered. > Any comment raising exception? ExceptionFilterValidate wouldn't > cause much BC, IMO.
Any comments on using SPL exception? http://php.net/manual/en/class.unexpectedvalueexception.php With Exception or UnexpedtedValueException, there is no BC. Regards,
-- Yasuo Ohgaki yohgaki@ohgaki.net

Rowan Collins

10 years ago
On 01/08/2016 23:12, Yasuo Ohgaki wrote:
> Raising Exception would be prefered. > Any comment raising exception? ExceptionFilterValidate wouldn't > cause much BC, IMO.
I don't follow. Do you mean throwing an exception to the existing filter-and-continue functions? If so, that would surely break every single use of those functions. Or do you mean adding a new function which either silently succeeds or throws an exception? That doesn't sound good to me - an exception shouldn't be the expected result of something, and invalid input is an expected condition (just an undesirable one). It's easier to add a throw statement based on a boolean result than to catch an exception and continue with other checks. I may have misunderstood your question, though. Regards,
-- Rowan Collins [IMSoP]

Yasuo Ohgaki

10 years ago
Hi Rowan, On Tue, Aug 2, 2016 at 11:37 PM, Rowan Collins <rowan.collins@gmail.com> wrote:
> On 01/08/2016 23:12, Yasuo Ohgaki wrote: >> >> Raising Exception would be prefered. >> Any comment raising exception? ExceptionFilterValidate wouldn't >> cause much BC, IMO. > > > I don't follow. Do you mean throwing an exception to the existing > filter-and-continue functions? If so, that would surely break every single > use of those functions.
Existing filters currently used are not changed at all. They behave exactly as they are now. If not, it's a bug. Exceptions are raised only when validate_*() functions are used. Input validations should pass almost always. So checking return value from validation functions are not prefered.
> > Or do you mean adding a new function which either silently succeeds or > throws an exception? That doesn't sound good to me - an exception shouldn't > be the expected result of something, and invalid input is an expected > condition (just an undesirable one). It's easier to add a throw statement > based on a boolean result than to catch an exception and continue with other > checks. > > I may have misunderstood your question, though.
Invalid inputs that violate input validation rules should not happen almost always because input validation is _not_ the same input mistakes by normal users. e.g. typo, user send a little too long password. These mistakes should be handled as input errors/mistakes, not input validation error. Example input validation errors are, - Broken UTF-8 encoding - NUL, etc control chars in string. - Too long or too short string. e.g. Client side validation inputs - JS validated and values set by server programs like <select>/<radio>/etc, 100 chars for username, 1000 chars for password, empty ID for a database record, etc. - Broken number string for a database record ID. - Newline chars in <input>, hash value, etc. - and so on. If there are these kind of inputs, they are intentionally broken inputs. It should result in Exception and program should be _terminated_ script execution after logging attack and sending nicely formatted page that warns attackers, for instance. validate_*() functions should be used to handle above input violations that should never happen under normal condition. Did I understand your concern correctly? I hope explained well. Regards,
-- Yasuo Ohgaki yohgaki@ohgaki.net

Rowan Collins

10 years ago
On 04/08/2016 11:45, Yasuo Ohgaki wrote:
> Invalid inputs that violate input validation rules should not happen > almost always because input validation is _not_ the same input > mistakes by normal users. e.g. typo, user send a little too long > password. These mistakes should be handled as input errors/mistakes, > not input validation error.
To me, "validation" absolutely means "checking if the user input the right things". What you're talking about sounds more like assertions, which would fit with the "this must be OK else die" implication of exceptions. Perhaps the functions could be called filter_assert_var_array() etc? Or perhaps some other term, to make it clear that these functions are not intended for the common task of "form validation". Regards,
-- Rowan Collins [IMSoP]

Dan Ackroyd

10 years ago
On 1 August 2016 at 09:23, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
> > Question is which version should I target for?
Why does this need to be in PHP core? Why can't this just be a userland library? cheers Dan

Yasuo Ohgaki

10 years ago
Hi Dan, On Tue, Aug 2, 2016 at 7:29 AM, Dan Ackroyd <danack@basereality.com> wrote:
> On 1 August 2016 at 09:23, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote: >> >> Question is which version should I target for? >
> Why does this need to be in PHP core?
Input validation is the most important security measure. https://www.securecoding.cert.org/confluence/display/seccode/Top+10+Secure+Coding+Practices https://www.owasp.org/index.php/OWASP_Secure_Coding_Practices_-_Quick_Reference_Guide Input validation best practice is "Validate values by whitelist and Reject invalid", not "Filter values and Accept". PHP should have function follows best practices. (I'm not saying nobody should not "Filter values and Accept". It's okay if your security policy allows it.)
> Why can't this just be a userland library?
PHP must have input validation feature that achieves previously described validation. Basic feature like input validation must be able to perform quickly, so it should be provided as core feature like basic escaping functions. Regards, P.S. I'll add string validation filters (e.g. min/max, encoding check,etc) and validation function callback (i.e. Must return bool) soon.
-- Yasuo Ohgaki yohgaki@ohgaki.net

Lester Caine

10 years ago
On 01/08/16 23:51, Yasuo Ohgaki wrote:
> PHP must have input validation feature that achieves previously > described validation. Basic feature like input validation must be able > to perform quickly, so it should be provided as core feature like > basic escaping functions. > > Regards, > > P.S. I'll add string validation filters (e.g. min/max, encoding > check,etc) and validation function callback (i.e. Must return bool) > soon.
Once again lots of additional code is being added which only fixes HALF of the input validation problem. The same as 'strict typing'. All of these extras can simply be eliminated if you address the problem of adding a set of rules to the basic 'var' that allow proper validation of each individually ... and *I* include in those rules adding the correct escaping for that particular variable. Which is EXACTLY what oe does in the user land libraries that currently fill the gap. On one hand we are being pushed to add things like getter and setter and all that overhead to create proper objects, while this option is back with handling a raw set of variables as an array?
-- Lester Caine - G8HFL ----------------------------- Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

Yasuo Ohgaki

10 years ago
Hi Lester, On Tue, Aug 2, 2016 at 2:56 PM, Lester Caine <lester@lsces.co.uk> wrote:
> > Once again lots of additional code is being added which only fixes HALF > of the input validation problem. The same as 'strict typing'.
I'm not trying to solve all of input validation issues by this proposal. Large amount of responsibilities are left to programmers. These could be done by callback, regex, multiple filter definitions. If you feel there is missing critical feature, please let me know. I think basic features required by security best practices are provided by this RFC changes.
> > All of these extras can simply be eliminated if you address the problem > of adding a set of rules to the basic 'var' that allow proper validation > of each individually ... and *I* include in those rules adding the > correct escaping for that particular variable. Which is EXACTLY what oe > does in the user land libraries that currently fill the gap. > > On one hand we are being pushed to add things like getter and setter and > all that overhead to create proper objects, while this option is back > with handling a raw set of variables as an array?
Do you mean validation rule definition? If so, yes. It's an array. Array definition rules could be wrong/broken, e.g. typo, and consequence of a broken definition is severe. So I added definition validation function, validate_check_definition(). (Better name might be preferred "validate" and "check" sounds strange) validate_check_definition() could be called via assert() during development. Regards,
-- Yasuo Ohgaki yohgaki@ohgaki.net

Christian Stadler

10 years ago
Am 01.08.2016 um 10:23 schrieb Yasuo Ohgaki:
> P.S. It's possible to return array that contains offending values. It > is not included since users can store whole offending input array. > Whole input is more useful for attack analysis.
Actually I wanted to suggest exactly that for ppl. who want to give Feedback to their users, what values failed to validate to the users. Probably with a fourth optional param, like `$return_invalid = false`? Of course logging is a different topic and should always use the whole offending input array. Regards, Christian Stadler

Yasuo Ohgaki

10 years ago
Hi Christian and all, On Thu, Aug 4, 2016 at 10:07 AM, Christian Stadler <stadli@gmx.de> wrote:
> Am 01.08.2016 um 10:23 schrieb Yasuo Ohgaki: >> P.S. It's possible to return array that contains offending values. It >> is not included since users can store whole offending input array. >> Whole input is more useful for attack analysis. > > Actually I wanted to suggest exactly that for ppl. who want to give > Feedback to their users, what values failed to validate to the users. > Probably with a fourth optional param, like `$return_invalid = false`? > Of course logging is a different topic and should always use the whole > offending input array.
I can set offending value to filter globals so that it can be retrieved later in catch block. I cannot return or modify referenced parameter because of raised exception. I don't mind adding this feature. It requires an API like validate_get_offending_value(). (The name should be nicer) How many of us are interested in this feature? Thank you for feedback!
-- Yasuo Ohgaki yohgaki@ohgaki.net

Rowan Collins

10 years ago
On 04/08/2016 11:10, Yasuo Ohgaki wrote:
> I cannot return or modify referenced > parameter because of raised exception.
This is another reason why exceptions are not appropriate for this case. As soon as you make an exception the expected result of something, you end up tying yourself in knots trying to build useful control flow around it. Regards,
-- Rowan Collins [IMSoP]

Christian Stadler

10 years ago
Am 04.08.2016 um 12:10 schrieb Yasuo Ohgaki:
> Hi Christian and all, > > On Thu, Aug 4, 2016 at 10:07 AM, Christian Stadler <stadli@gmx.de> wrote: >> Am 01.08.2016 um 10:23 schrieb Yasuo Ohgaki: >>> P.S. It's possible to return array that contains offending values. It >>> is not included since users can store whole offending input array. >>> Whole input is more useful for attack analysis. >> Actually I wanted to suggest exactly that for ppl. who want to give >> Feedback to their users, what values failed to validate to the users. >> Probably with a fourth optional param, like `$return_invalid = false`? >> Of course logging is a different topic and should always use the whole >> offending input array. > I can set offending value to filter globals so that it can be > retrieved later in catch block. I cannot return or modify referenced > parameter because of raised exception.
Well, since some people have objections about raising exceptions here, this should probably be either in a seperate vote or additional options in the main vote. Probably something, like: Yes, either | Yes, without the exception | Yes, with the exception | No Personally I would vote for 'Yes, either'. If I could, that is.
> I don't mind adding this feature. It requires an API like > validate_get_offending_value(). (The name should be nicer) > How many of us are interested in this feature?
Then this new function should have an offset param. With this I could check, if the array has any offending values and then continue with the rest ... mmh, now that I think of it, this isn't really necessary. Uhm, well anyway: I'd suggest, that the ind(ex/ices) should be returned rather, than the actual value names. Regards, Christian Stadler

Yasuo Ohgaki

10 years ago
Hi Christian, On Thu, Aug 4, 2016 at 8:27 PM, Christian Stadler <stadli@gmx.de> wrote:
> Am 04.08.2016 um 12:10 schrieb Yasuo Ohgaki: >> Hi Christian and all, >> >> On Thu, Aug 4, 2016 at 10:07 AM, Christian Stadler <stadli@gmx.de> wrote: >>> Am 01.08.2016 um 10:23 schrieb Yasuo Ohgaki: >>>> P.S. It's possible to return array that contains offending values. It >>>> is not included since users can store whole offending input array. >>>> Whole input is more useful for attack analysis. >>> Actually I wanted to suggest exactly that for ppl. who want to give >>> Feedback to their users, what values failed to validate to the users. >>> Probably with a fourth optional param, like `$return_invalid = false`? >>> Of course logging is a different topic and should always use the whole >>> offending input array. >> I can set offending value to filter globals so that it can be >> retrieved later in catch block. I cannot return or modify referenced >> parameter because of raised exception. > > Well, since some people have objections about raising exceptions here, > this should probably be either in a seperate vote or additional options > in the main vote. Probably something, like: > Yes, either | Yes, without the exception | Yes, with the exception | No > Personally I would vote for 'Yes, either'. If I could, that is.
One of my objective is following best practices. Prefer exception over error is one of them. Although, I strongly suggest to use exception for validation errors, I will have choices. (Exception should be used error cases that should not happen usually, but usual error handling would work. Error message could be more user friendly because php_error_docref() supports va arg)
> >> I don't mind adding this feature. It requires an API like >> validate_get_offending_value(). (The name should be nicer) >> How many of us are interested in this feature? > > Then this new function should have an offset param. With this I could > check, if the array has any offending values and then continue with the > rest ... mmh, now that I think of it, this isn't really necessary. > > Uhm, well anyway: I'd suggest, that the ind(ex/ices) should be returned > rather, than the actual value names.
OK. Thank you. I'll add this. The reason why I said store "value" is the code. To get index, it has to store index somewhere or change many lines of code. Regards,
-- Yasuo Ohgaki yohgaki@ohgaki.net

Rowan Collins

10 years ago
Hi Yasuo, On 04/08/2016 20:29, Yasuo Ohgaki wrote:
> One of my objective is following best practices. > Prefer exception over error is one of them. Although, I strongly suggest > to use exception for validation errors, I will have choices.
Best practice is to use exceptions to indicate an error *inside* the function, i.e. something going wrong with the function itself. If I run (as a simplified example) validate_var('abc', FILTER_NUMERIC) that's not something going wrong inside validate_var(), so validate_var() shouldn't be throwing an exception, IMO. Now, the *reason* I'm calling validate_var may be that the function I'm writing will go wrong if the validation doesn't pass, but that's up to *me* to throw an exception: if ( validate_var($foo, FILTER_NUMERIC) ) { throw new InvalidArgumentException; } That said, in another message you explained that this isn't intended for "validation" in the sense of "form validation", but in a more restricted sense of "dangerous data"; I think the name misled me, and may mislead others, if that is the sole purpose of the function. I'm still not entirely convinced, though, that a boolean-returning function wouldn't be enough to fill both purposes. Regards,
-- Rowan Collins [IMSoP]

Yasuo Ohgaki

10 years ago
Hi Rowan, On Fri, Aug 5, 2016 at 6:04 AM, Rowan Collins <rowan.collins@gmail.com> wrote:
> On 04/08/2016 20:29, Yasuo Ohgaki wrote: >> >> One of my objective is following best practices. >> Prefer exception over error is one of them. Although, I strongly suggest >> to use exception for validation errors, I will have choices. > > > Best practice is to use exceptions to indicate an error *inside* the > function, i.e. something going wrong with the function itself. If I run (as > a simplified example) validate_var('abc', FILTER_NUMERIC) that's not > something going wrong inside validate_var(), so validate_var() shouldn't be > throwing an exception, IMO. > > Now, the *reason* I'm calling validate_var may be that the function I'm > writing will go wrong if the validation doesn't pass, but that's up to *me* > to throw an exception: > > if ( validate_var($foo, FILTER_NUMERIC) ) { > throw new InvalidArgumentException; > }
Input validation is like assertion shouldn't fail. However, it's possible to design code like this.
> > That said, in another message you explained that this isn't intended for > "validation" in the sense of "form validation", but in a more restricted > sense of "dangerous data"; I think the name misled me, and may mislead > others, if that is the sole purpose of the function. I'm still not entirely > convinced, though, that a boolean-returning function wouldn't be enough to > fill both purposes.
I understands there are use cases for both with/without exception. In fact, I implemented it without exception at first. My preference is to raise exception to encourage users to handle validation error correctly, i.e. terminate script execution, but my intention may not be interpreted correctly with or without exception. How about to have switch like assert()? It has exception on/off switch. Anyone prefer parameter flag? Regards,
-- Yasuo Ohgaki yohgaki@ohgaki.net

Rowan Collins

10 years ago
On 04/08/2016 22:25, Yasuo Ohgaki wrote:
> My preference is to raise exception to encourage users to handle > validation error correctly, i.e. terminate script execution, but my > intention may not be interpreted correctly with or without exception.
I know naming things is hard, but I really do think "validation" is not the correct name for what you are talking about. Search online for "form validation", and you will see what I mean. The correct response to a form validation error is to show a message to the user with as much detail as possible, not to simply terminate the script and assume they are trying to attack your application. Regards,
-- Rowan Collins [IMSoP]

Yasuo Ohgaki

10 years ago
Hi Rowan, On Fri, Aug 5, 2016 at 6:38 AM, Rowan Collins <rowan.collins@gmail.com> wrote:
> On 04/08/2016 22:25, Yasuo Ohgaki wrote: >> >> My preference is to raise exception to encourage users to handle >> validation error correctly, i.e. terminate script execution, but my >> intention may not be interpreted correctly with or without exception. > > > I know naming things is hard, but I really do think "validation" is not the > correct name for what you are talking about. Search online for "form > validation", and you will see what I mean. > > The correct response to a form validation error is to show a message to the > user with as much detail as possible, not to simply terminate the script and > assume they are trying to attack your application.
We are talking about different things. I'll document it clearly in RFC. Thank you!
-- Yasuo Ohgaki yohgaki@ohgaki.net

Yasuo Ohgaki

10 years ago
Hi Rowan, On Fri, Aug 5, 2016 at 6:47 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
>> I know naming things is hard, but I really do think "validation" is not the >> correct name for what you are talking about. Search online for "form >> validation", and you will see what I mean. >> >> The correct response to a form validation error is to show a message to the >> user with as much detail as possible, not to simply terminate the script and >> assume they are trying to attack your application. > > We are talking about different things. > I'll document it clearly in RFC.
Different function name may help to avoid confusions. How about filter_assert_*() (I'm using filter_ prefix suggested by Pierre. It's few chars longer, but better consistency :) These functions could be used for input mistake/error checks, but I would like to users to use them only to assert external inputs, see if valid(acceptable) or invalid(unacceptable) for the software. The name may help. Hopefully. Regards,
-- Yasuo Ohgaki yohgaki@ohgaki.net

Lester Caine

10 years ago
On 04/08/16 22:47, Yasuo Ohgaki wrote:
>> The correct response to a form validation error is to show a message to the >> > user with as much detail as possible, not to simply terminate the script and >> > assume they are trying to attack your application. > We are talking about different things. > I'll document it clearly in RFC.
But both need to be done at the same time ... Validating the input data requires that you have a set of rules for each variable, and if all are correct then one can process the 'array', but if an element fails validation then one needs to handle the error either as suspicious, or simply out of range. I repeat that handling the complexities of EACH variable of the validation is a package of work in it's own right, and trying to handle some aspects via an array function does not remove the more obvious need to decide what order to handle validation errors on each element, or to return error messages for each variable that fails validation. If the element is some attempt to create an injection of js or html tags then it should fail validation, but equally it may be a valid input as long as it is escaped and stored in a correct manor. The complexity of what you plan for your array validation elements start with all the same rules applied to a single variable.
-- Lester Caine - G8HFL ----------------------------- Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

Yasuo Ohgaki

10 years ago
Hi all, Pierre suggested the functions should be named "filter_*" and I agree. On Fri, Aug 5, 2016 at 6:25 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
> Input validation is like assertion shouldn't fail.
How about filter_assert_var() <- validate_var() filter_assert_var_array() <- validate_var_array() filter_assert_input() <- validate_input() filter_assert_input_array() <- validate_input_array() It seems better than filter_validate_*() to me. These names encourage users to use them for input assertion(validation). Any other good names? Regards,
-- Yasuo Ohgaki yohgaki@ohgaki.net

Christoph Becker

10 years ago
On 05.08.2016 at 02:15, Yasuo Ohgaki wrote:
> Pierre suggested the functions should be named "filter_*" and I agree. > > On Fri, Aug 5, 2016 at 6:25 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote: >> Input validation is like assertion shouldn't fail. > > How about > > filter_assert_var() <- validate_var() > filter_assert_var_array() <- validate_var_array() > filter_assert_input() <- validate_input() > filter_assert_input_array() <- validate_input_array() > > It seems better than > > filter_validate_*() > > to me. These names encourage users to use them for input assertion(validation). > > Any other good names?
I think the names are okay, but it should be pointed out that they are not related to assert() (particularly, that they are not affected by the assert.* ini directives). Maybe "assume" or "require" instead of "assert" would therefore be better in this regard.
-- Christoph M. Becker

Yasuo Ohgaki

10 years ago
Hi Christoph and all, On Fri, Aug 5, 2016 at 6:17 PM, Christoph Becker <cmbecker69@gmx.de> wrote:
> > I think the names are okay, but it should be pointed out that they are > not related to assert() (particularly, that they are not affected by the > assert.* ini directives). Maybe "assume" or "require" instead of > "assert" would therefore be better in this regard.
Good suggestion! Users may confuse with assert() and filter_assert()... I don't mind changing function names at all. "assume" sounds to weak for me. "require" sounds better to me. assert_require_*() may be the best choice among - validate_*() - filter_assert_*() - filter_assume_*() - filter_require_*() Do you like/feel ok with filter_require_*()? If you have better suggestion, it is appreciated. If there isn't comment in a few days, I'll use filter_require_var() <- validate_var() filter_require_var_array() <- validate_var_array() filter_require_input() <- validate_input() filter_require_input_array() <- validate_input_array() Regards,
-- Yasuo Ohgaki yohgaki@ohgaki.net

Christian Stadler

10 years ago
Am 04.08.2016 um 21:29 schrieb Yasuo Ohgaki:
> On Thu, Aug 4, 2016 at 8:27 PM, Christian Stadler <stadli@gmx.de> wrote: >> Am 04.08.2016 um 12:10 schrieb Yasuo Ohgaki: >>> On Thu, Aug 4, 2016 at 10:07 AM, Christian Stadler <stadli@gmx.de> wrote: >>>> Am 01.08.2016 um 10:23 schrieb Yasuo Ohgaki: >>> I don't mind adding this feature. It requires an API like >>> validate_get_offending_value(). (The name should be nicer) >>> How many of us are interested in this feature? >> Then this new function should have an offset param. With this I could >> check, if the array has any offending values and then continue with the >> rest ... mmh, now that I think of it, this isn't really necessary. >> >> Uhm, well anyway: I'd suggest, that the ind(ex/ices) should be returned >> rather, than the actual value names. > OK. Thank you. > I'll add this. The reason why I said store "value" is the code. > To get index, it has to store index somewhere or change many lines of code.
Actually I've suggested returning indices, because of the resulting userland-code. Actually I could work with both. So you should choose, whatever is better optimized for the codebase and/or userland-code. On the other hand (@Yasuo and all others): What's your opinion about adding options, rather than more and more new params? e. g.: - FILTER_VALIDATE_EXCEPTION_ON_FIRST_INVALID --> $options & 1 == 1 (default, should be a better/shorter name) - FILTER_VALIDATE_RETURN_VALUES --> $options & 2 == 0 (default) - FILTER_VALIDATE_RETURN_INDICES --> $options & 2 == 1 Just a quick&dirty example, so don't nail me on that ^^ Regards, Christian Stadler

Pierre Joye

10 years ago
On Aug 5, 2016 2:30 AM, "Yasuo Ohgaki" <yohgaki@ohgaki.net> wrote:
> > Hi Christian, > > On Thu, Aug 4, 2016 at 8:27 PM, Christian Stadler <stadli@gmx.de> wrote: > > Am 04.08.2016 um 12:10 schrieb Yasuo Ohgaki: > >> Hi Christian and all, > >> > >> On Thu, Aug 4, 2016 at 10:07 AM, Christian Stadler <stadli@gmx.de>
wrote:
> >>> Am 01.08.2016 um 10:23 schrieb Yasuo Ohgaki: > >>>> P.S. It's possible to return array that contains offending values. It > >>>> is not included since users can store whole offending input array. > >>>> Whole input is more useful for attack analysis. > >>> Actually I wanted to suggest exactly that for ppl. who want to give > >>> Feedback to their users, what values failed to validate to the users. > >>> Probably with a fourth optional param, like `$return_invalid = false`? > >>> Of course logging is a different topic and should always use the whole > >>> offending input array. > >> I can set offending value to filter globals so that it can be > >> retrieved later in catch block. I cannot return or modify referenced > >> parameter because of raised exception. > > > > Well, since some people have objections about raising exceptions here, > > this should probably be either in a seperate vote or additional options > > in the main vote. Probably something, like: > > Yes, either | Yes, without the exception | Yes, with the exception | No > > Personally I would vote for 'Yes, either'. If I could, that is. > > One of my objective is following best practices. > Prefer exception over error is one of them. Although, I strongly suggest > to use exception for validation errors, I will have choices.
I see them as conditions flow not errors per se but flow. Invalid options could raise exceptions but it brings inconcistencies with the other filter functions. I feel like this rfc needs more discussions and maybe we will add more things to filter as well. But anything proposed is already possible very easily in userland. I would not rush it into 7.1.

Niklas Keller

10 years ago
2016-08-07 14:20 GMT+02:00 Pierre Joye <pierre.php@gmail.com>:
> On Aug 5, 2016 2:30 AM, "Yasuo Ohgaki" <yohgaki@ohgaki.net> wrote: > > > > Hi Christian, > > > > On Thu, Aug 4, 2016 at 8:27 PM, Christian Stadler <stadli@gmx.de> wrote: > > > Am 04.08.2016 um 12:10 schrieb Yasuo Ohgaki: > > >> Hi Christian and all, > > >> > > >> On Thu, Aug 4, 2016 at 10:07 AM, Christian Stadler <stadli@gmx.de> > wrote: > > >>> Am 01.08.2016 um 10:23 schrieb Yasuo Ohgaki: > > >>>> P.S. It's possible to return array that contains offending values. > It > > >>>> is not included since users can store whole offending input array. > > >>>> Whole input is more useful for attack analysis. > > >>> Actually I wanted to suggest exactly that for ppl. who want to give > > >>> Feedback to their users, what values failed to validate to the users. > > >>> Probably with a fourth optional param, like `$return_invalid = > false`? > > >>> Of course logging is a different topic and should always use the > whole > > >>> offending input array. > > >> I can set offending value to filter globals so that it can be > > >> retrieved later in catch block. I cannot return or modify referenced > > >> parameter because of raised exception. > > > > > > Well, since some people have objections about raising exceptions here, > > > this should probably be either in a seperate vote or additional options > > > in the main vote. Probably something, like: > > > Yes, either | Yes, without the exception | Yes, with the exception | No > > > Personally I would vote for 'Yes, either'. If I could, that is. > > > > One of my objective is following best practices. > > Prefer exception over error is one of them. Although, I strongly suggest > > to use exception for validation errors, I will have choices. > > I see them as conditions flow not errors per se but flow. > > Invalid options could raise exceptions but it brings inconcistencies with > the other filter functions. > > I feel like this rfc needs more discussions and maybe we will add more > things to filter as well. > > But anything proposed is already possible very easily in userland. I would > not rush it into 7.1. >
Isn't it a bit late to target 7.1 anyway? Regards, Niklas

Davey

10 years ago
On Sun, Aug 7, 2016 at 8:20 AM, Niklas Keller <me@kelunik.com> wrote:
> 2016-08-07 14:20 GMT+02:00 Pierre Joye <pierre.php@gmail.com>: > > > On Aug 5, 2016 2:30 AM, "Yasuo Ohgaki" <yohgaki@ohgaki.net> wrote: > > > > > > Hi Christian, > > > > > > On Thu, Aug 4, 2016 at 8:27 PM, Christian Stadler <stadli@gmx.de> > wrote: > > > > Am 04.08.2016 um 12:10 schrieb Yasuo Ohgaki: > > > >> Hi Christian and all, > > > >> > > > >> On Thu, Aug 4, 2016 at 10:07 AM, Christian Stadler <stadli@gmx.de> > > wrote: > > > >>> Am 01.08.2016 um 10:23 schrieb Yasuo Ohgaki: > > > >>>> P.S. It's possible to return array that contains offending values. > > It > > > >>>> is not included since users can store whole offending input array. > > > >>>> Whole input is more useful for attack analysis. > > > >>> Actually I wanted to suggest exactly that for ppl. who want to give > > > >>> Feedback to their users, what values failed to validate to the > users. > > > >>> Probably with a fourth optional param, like `$return_invalid = > > false`? > > > >>> Of course logging is a different topic and should always use the > > whole > > > >>> offending input array. > > > >> I can set offending value to filter globals so that it can be > > > >> retrieved later in catch block. I cannot return or modify referenced > > > >> parameter because of raised exception. > > > > > > > > Well, since some people have objections about raising exceptions > here, > > > > this should probably be either in a seperate vote or additional > options > > > > in the main vote. Probably something, like: > > > > Yes, either | Yes, without the exception | Yes, with the exception | > No > > > > Personally I would vote for 'Yes, either'. If I could, that is. > > > > > > One of my objective is following best practices. > > > Prefer exception over error is one of them. Although, I strongly > suggest > > > to use exception for validation errors, I will have choices. > > > > I see them as conditions flow not errors per se but flow. > > > > Invalid options could raise exceptions but it brings inconcistencies with > > the other filter functions. > > > > I feel like this rfc needs more discussions and maybe we will add more > > things to filter as well. > > > > But anything proposed is already possible very easily in userland. I > would > > not rush it into 7.1. > > > > Isn't it a bit late to target 7.1 anyway? >
Yes, it is much too late for 7.1, this will have to be targeted towards 7.2+ - Davey

Yasuo Ohgaki

10 years ago
Hi Pierre, Niklas On Sun, Aug 7, 2016 at 9:20 PM, Pierre Joye <pierre.php@gmail.com> wrote:
> I would not rush it into 7.1.
On Mon, Aug 8, 2016 at 12:20 AM, Niklas Keller <me@kelunik.com> wrote:
> Isn't it a bit late to target 7.1 anyway?
I feel the same way indeed. However, it does not change existing feature at all, but this is a pure addition. It's one of the largest mandatory piece that PHP is missing. It's good for 7.1.0, IMO. (Another largest piece is escaping API for basic needs. e.g. we don't even have JavaScript string escape API.) I made choices for targeted version, 7.1.0 or 7.2.0 https://wiki.php.net/rfc/add_validate_functions_to_filter If this RFC is passed for 7.2.0, then we can adjust the feature until its release.
> But anything proposed is already possible very easily in userland.
This part is debatable. To implement feature that provides filter_require_*() and validation filter behavior with them, users have to write many lines of code. Some features are hard to implement in user code such as INT/OCTAL/HEX overflow detection.
> I see them as conditions flow not errors per se but flow. > Invalid options could raise exceptions but it brings inconsistencies with the other filter functions.
I really think we should have Exception handling standard in CODING_STANDARD, too. This issue could be handled by PHP 8.0 or PHP 7.2.
> I feel like this rfc needs more discussions and maybe we will add more things to filter as well.
Validation filters with new functions are made to work conservative way. So it would not be a issue relaxing them later. Use of exception is issue. IMO, it would be more useful if there is filter validation exception object that has more details in it. i.e. Offensive input key, data, filter name and filter options, remove filter_get_invalid_key(). I may add the new exception, FilterValidateException. (I choose "Validate" here, since most exceptions will be raised by validation filters) In fact, I don't like the way exception works currently. I'll add mentioned FilterValidateException before vote. Does this make you feel better? I feel the feature is fully implemented if I add FilterValidateException. Regards, P.S. There is conflicting pressures on RFC. I need to minimize changes so that reviewer can understand the RFC easily, but I would like to implement feature fully. Some changes cannot add/work well individually. This exception issue is one of them, another is https://wiki.php.net/rfc/precise_session_management It includes mandatory feature for proper/secure session management and implements all possible features that would be required. Yet, it is declined :(
-- Yasuo Ohgaki yohgaki@ohgaki.net

Niklas Keller

10 years ago
2016-08-08 0:20 GMT+02:00 Yasuo Ohgaki <yohgaki@ohgaki.net>:
> Hi Pierre, Niklas > > On Sun, Aug 7, 2016 at 9:20 PM, Pierre Joye <pierre.php@gmail.com> wrote: > > I would not rush it into 7.1. > On Mon, Aug 8, 2016 at 12:20 AM, Niklas Keller <me@kelunik.com> wrote: > > Isn't it a bit late to target 7.1 anyway? > > I feel the same way indeed. However, it does not change existing > feature at all, but this is a pure addition. It's one of the largest > mandatory piece that PHP is missing. It's good for 7.1.0, IMO. > (Another largest piece is escaping API for basic needs. e.g. > we don't even have JavaScript string escape API.) > > I made choices for targeted version, 7.1.0 or 7.2.0 >
Feature freeze has been beta1. If the RM says it's too late, we don't need a vote. It's too late. Regards, Niklas

Yasuo Ohgaki

10 years ago
Hi Darvey, On Mon, Aug 8, 2016 at 4:24 PM, Niklas Keller <me@kelunik.com> wrote:
>> I made choices for targeted version, 7.1.0 or 7.2.0 > > Feature freeze has been beta1. If the RM says it's too late, we don't need a > vote. It's too late.
Niklas is right. I might be too late for 7.1. Darvey. Although diff is large, changes are simple and straightforward additions (no changes in existing functionalities at all) to make filter module work right with secure coding. It's more like bug fix to me, just like URL rewriter fix. Can the RFC have voting choices for targeted version or not? https://wiki.php.net/rfc/add_validate_functions_to_filter I don't mind delaying this to 7.2 at all. Thanks.
-- Yasuo Ohgaki yohgaki@ohgaki.net

Niklas Keller

10 years ago
2016-08-08 12:03 GMT+02:00 Yasuo Ohgaki <yohgaki@ohgaki.net>:
> Hi Darvey, > > On Mon, Aug 8, 2016 at 4:24 PM, Niklas Keller <me@kelunik.com> wrote: > >> I made choices for targeted version, 7.1.0 or 7.2.0 > > > > Feature freeze has been beta1. If the RM says it's too late, we don't > need a > > vote. It's too late. > > Niklas is right. I might be too late for 7.1. > > Darvey. Although diff is large, changes are simple and straightforward > additions (no changes in existing functionalities at all) to make > filter module work right with secure coding. It's more like bug fix to > me, just like URL rewriter fix. >
This is clearly a new feature and not a bug fix.
> Can the RFC have voting choices for targeted version or not? > > https://wiki.php.net/rfc/add_validate_functions_to_filter > > I don't mind delaying this to 7.2 at all. >
If you don't mind, let's delay it to 7.2, as there still seems to be discussion. Regards, Niklas