Hi Dan,

Thank you for sharing idea!


On Mon, Aug 15, 2016 at 10:25 AM, Dan Ackroyd <dan...@basereality.com> wrote:
>
> On 15 August 2016 at 01:53, Yasuo Ohgaki <yohg...@ohgaki.net> wrote:
>
>> One more usual request.
>> Please describe reason(s) why you object proposal.
>
>
> I'm not entirely sure why you ask for reasons when people vote no. The
> reasons are almost always the same as the reasons given before the
> voting starts.

Without feedback, there is no clue which way should go or improve.
I didn't realize some of your idea and I think your feedback is great.

>
> But for posterity:
>
> i) Validation error messages need to specify what is wrong.....which
> is bespoke to the application. Which is a reason why validation code
> belongs in userland.

When exception is enabled, offensive key name is written in exception message.

When exception is disabled, your statement is true. This could be improved.
Good feedback.

>
> ii) Validation error message need to be in the correct language for an
> application. It is not a good approach for people to be trying to
> match strings emitted by internal code and trying to convert them to
> the correct language.

It seems there is misunderstanding.
These new functions are intended for "secure coding input validation" that
should never fail. It means something unexpected in input data that
cannot/shouldn't keep program running. Why do you need to parse
message?

All needed info, filter name, key and value, is in exception message and
exception object, BTW.

This one is good feedback, too.
I appreciate better error message suggestions.

>
> iii) The argument that it needs to be fast could be applied to
> anything and everything, and so is bogus. The RFC doesn't even show
> that userland implementations are slow enought to be a concern.

I thought I don't have to have example of userland implementation, so
it's good feedback also.

Typical OO implementation uses number of setters to define validation rules.
In addition, it validates validation rule is OK for it. e.g. It will
check input data type at least. Setters and validation rule validation
makes execution slower obviously.

One may optimize validation rules to plain array (like I do).
In this case, performance is could be better than previously mentioned
validators do all in the production environment.

I also thought the performance issue is not much important because
there is no PHP feature to compare. All of us knew PHP function call
overheads are relatively large and proposed almost all in C implementation
would be faster than userland.

>
> iv) The RFC makes an assumption that programs should exit when validation 
> fails.
>
> "Input data validation should accept only valid and possible inputs.
> If not, reject it and terminate program."
>
> and the code example:
>
>> catch (FilterValidateException $e) {
>>    var_dump($e->getMessage());
>>    die('Invalid input detected!'); // Should terminate execution when input 
>> validation fails
>> }
>
> This assumption is bogus.
>
> Any program that accepts data from users should provide useful error
> messages when the data is wrong with someting as simple as a string
> being too long.

There is misunderstanding on this.
As I wrote explicitly in the RFC, input validation and user input
mistakes must be handled differently.

"The input validation (or think it as assertion or requirement) error"
that this RFC is dealing, is should never happen conditions (or think
it as contract should never fail).

The point of having the input validation is accept only inputs that
program expects and can work correctly. Accepting unexpected
data that program cannot work correctly is pointless.

Don't misunderstood me. I'm not saying "You should reject user input mistakes".
"User input mistakes" and "input validation error" is totally different error.

>
> v) I don't like the current filter functions, and recommend people
> avoid using them. Adding to them with an even harder to use API is the
> wrong way to go.

I didn't recommend it either because it could not be used for input
validation easily, escaping or sanitization could be done for
dedicated API.

Having new module is one of my idea also. However, I realized many of
filter module codes could be reused after investigation. That's the
reason why I added to filter module. I also named new functions to
have "validate_*" prefix, rather than "filter_*" to emphasis it's
for validations. I renamed them to "filter_*" to comply CODING_STANDARDS.

This feedback is great because I'm worrying about the same thing.
Please feedback this kind of things during discussion so that I can
do something on issues.

Thank you for comments.
I think it's very helpful for improvements!

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to