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