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 scalarFALSE
on validation failure(s), instead of filtered
array. - validate_input_array() - Almost the same as
filter_input_array()
except it returns scalarFALSE
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
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
Hi Christoph,
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.
- 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
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 scalarFALSE
on validation failure(s), instead of filtered
array.- validate_input_array() - Almost the same as
filter_input_array()
except it returns scalarFALSE
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
Hi all,
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
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]
Hi Rowan,
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
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]
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
Hi Dan,
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
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
Hi Lester,
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
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
Hi Christian and all,
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
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]
Am 04.08.2016 um 12:10 schrieb Yasuo Ohgaki:
Hi Christian and all,
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
Hi Christian,
Am 04.08.2016 um 12:10 schrieb Yasuo Ohgaki:
Hi Christian and all,
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
Hi Yasuo,
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]
Hi Rowan,
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
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]
Hi Rowan,
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
Hi Rowan,
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
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
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 ...
Right and wrong? Both have to be done in a software.
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.
Modern softwares are complex enough to handle inputs securely.
Softwares should use "divide and conquer" principle.
Input validation is like assertion that should never happen.
It should be validated(asserted) according to the software input spec and
it should be checked as soon as inputs arrive to the software, and leave
behind mistakes, logical inconsistency, etc.
Validation(assertion) errors are:
- Broken char encoding
- Broken input format. e.g. Broken int/float/bool, strings like hash/id/etc
- Invalid chars in string. i.e. NUL and other control chars
- Too large/small values. e.g. Too long/short string, too large/small
int, float. - Too few/many inputs. ("Too many inputs" cannot be address by filter
functions, but can be addressed input validation code)
Some of above are need to be addressed by software business logic
on occasions depending on client output, but input validation should
make sure input values validity as much as possible to reduce
software complexity.
After input validation, business logic should take care of the rest.
Input error(user mistake, logical inconsistency) check should be checked
separately by the software as a part of the software business logic.
This way, business logic code can concentrate on jobs that it should
and reduce complexity of input handling.
This is my view of what input validation should do. (and what business
logic should do)
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
I should have been more precise.
Input validation is like assertion that should never happen.
It should be validated(asserted) according to the software input spec and
it should be checked as soon as inputs arrive to the software, and leave
behind mistakes, logical inconsistency, etc.
Input validation is like assertion that should never happen.
Inputs should be validated(asserted) according to the software input spec and
inputs should be checked as soon as inputs arrive to the software, and leave
behind mistakes, logical inconsistency, etc.
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi all,
Pierre suggested the functions should be named "filter_*" and I agree.
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
Pierre suggested the functions should be named "filter_*" and I agree.
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
Hi Christoph and all,
I think the names are okay, but it should be pointed out that they are
not related toassert()
(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
assert_require_*()
I mean filter_require_*(), of course...
Anyway, I think I finished proposed implementation.
I would like to start the vote from 8/10.
If you have any concerns, please let me know.
https://wiki.php.net/rfc/add_validate_functions_to_filter
Thank you!
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Am 04.08.2016 um 21:29 schrieb Yasuo Ohgaki:
Am 04.08.2016 um 12:10 schrieb Yasuo Ohgaki:
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
Hi Christian,
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.
2016-08-07 14:20 GMT+02:00 Pierre Joye pierre.php@gmail.com:
Hi Christian,
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
2016-08-07 14:20 GMT+02:00 Pierre Joye pierre.php@gmail.com:
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
Hi Pierre, Niklas
I would not rush it into 7.1.
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
2016-08-08 0:20 GMT+02:00 Yasuo Ohgaki yohgaki@ohgaki.net:
Hi Pierre, Niklas
I would not rush it into 7.1.
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
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
Hi Darvey,
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
2016-08-08 12:03 GMT+02:00 Yasuo Ohgaki yohgaki@ohgaki.net:
Hi Darvey,
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
Thanks.
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Niklas and Davey,
2016-08-08 12:03 GMT+02:00 Yasuo Ohgaki yohgaki@ohgaki.net:
Hi Darvey,
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.
It's new feature indeed.
Even if use of current filter module for secure coding is misuse, I
recently realized that there are some users who are trying to do. It's
partly because PHP is providing APIs that seems to work with secure
coding, but it is lacking proper APIs infact. Lack of proper API is
almost the same as bug to me especially when there are users misusing
them.
Anyway, this kind of changes wouldn't go into released versions.
Davey, it's your call if this RFC will have target version choice or
not.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net