Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:95815 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 76838 invoked from network); 8 Sep 2016 23:13:45 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 8 Sep 2016 23:13:45 -0000 Authentication-Results: pb1.pair.com smtp.mail=yohgaki@ohgaki.net; spf=pass; sender-id=pass Authentication-Results: pb1.pair.com header.from=yohgaki@ohgaki.net; sender-id=pass Received-SPF: pass (pb1.pair.com: domain ohgaki.net designates 180.42.98.130 as permitted sender) X-PHP-List-Original-Sender: yohgaki@ohgaki.net X-Host-Fingerprint: 180.42.98.130 ns1.es-i.jp Received: from [180.42.98.130] ([180.42.98.130:59383] helo=es-i.jp) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id 1E/C7-61313-7A0F1D75 for ; Thu, 08 Sep 2016 19:13:45 -0400 Received: (qmail 71032 invoked by uid 89); 8 Sep 2016 23:13:40 -0000 Received: from unknown (HELO mail-qk0-f171.google.com) (yohgaki@ohgaki.net@209.85.220.171) by 0 with ESMTPA; 8 Sep 2016 23:13:40 -0000 Received: by mail-qk0-f171.google.com with SMTP id v123so65309511qkh.2 for ; Thu, 08 Sep 2016 16:13:40 -0700 (PDT) X-Gm-Message-State: AE9vXwPt6BX59FuOsyNgY4vGS78Z3fzTY/cZM1dOXNOchoHs4dsWpChrULe8tGwelhK0Afq6kc4zIV7v/LSB3A== X-Received: by 10.55.167.68 with SMTP id q65mr549938qke.61.1473376413486; Thu, 08 Sep 2016 16:13:33 -0700 (PDT) MIME-Version: 1.0 Received: by 10.140.84.168 with HTTP; Thu, 8 Sep 2016 16:12:51 -0700 (PDT) In-Reply-To: References: <232F1604-2211-4351-B830-EDC958A25D6D@strojny.net> <2de35db0-9974-cc96-83dd-3d2dbd48f7f8@lsces.co.uk> <5b72e9da-068a-bc79-82c2-f36f723f42bb@gmail.com> <8E15A146-F5B6-41AA-8177-4DAA5ACB35C4@koalephant.com> Date: Fri, 9 Sep 2016 08:12:51 +0900 X-Gmail-Original-Message-ID: Message-ID: To: Stephen Reay Cc: "internals@lists.php.net" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [PHP-DEV] [RFC][VOTE] Add validation functions to filter module From: yohgaki@ohgaki.net (Yasuo Ohgaki) On Thu, Sep 8, 2016 at 9:13 PM, Stephen Reay wro= te: >> On 8 Sep 2016, at 17:49, Yasuo Ohgaki wrote: >> >> Hi Stephen, >> >> On Thu, Sep 8, 2016 at 7:34 PM, Stephen Reay = wrote: >>> Adding a bunch of new functions is IMO the wrong approach to this type = of thing. >>> The existing filter_var/filter_input infrastructure works well, if you = want to define more rules I would definitely encourage building on/improvin= g that system not adding a bunch of extra functions. >> >> Do you really think filter module works well as optimal validator? > > It=E2=80=99s not perfect, but nothing is. As I said, I believe the issues= can largely be resolved by building on the existing functionality. > >> It cannot enforce even whitelisting well=E2=80=A6 > > VALIDATE_INT already accepts $max and $min options. Those options could b= e applied to VALIDATE_FLOAT, and $charset, $accepted_chars, $max_len, $min_= len could be implemented on a new VALIDATE_STRING filter. But it trims input because it is filter based. For example, int input like ' 1234 = ' must be invalid with whitelisting approach, but it's allowed with current implementation. > > I understand the use-case for multiple validation per input, and for vali= dating multiple inputs, but frankly the way this implements that is both co= nfusing to use, and has a less than ideal error-mode. > I agree. As filter, it works well mostly. As validator, it's unuseable due to filter like behavior/non whitelisting behaviors. > The =E2=80=9Cfilter spec=E2=80=9D input is an array of arrays of arrays, = most of which will also contain an array for =E2=80=98options=E2=80=99. To = me that=E2=80=99s getting dangerously close to JavaScript=E2=80=99s callbac= k hell for impossible to read code. > I can understand your concern. Issue would be callback validator, but callback nesting would not be needed unlike JS callback hell. Since it's simple array, content/rule can be viewed easily also. > The error mode is also not ideal in a real world use case in my opinion. = If I am validating a dozen input fields, I do *not* want to know just the f= irst one that failed. Can you imagine using a web form that made you submit= 12 times to tell you each time you got a field wrong, rather than trying t= o validate them ALL and telling you ALL the errors at once? You can omit validation where you would like. So your concern wouldn't be problem. (I strongly suggest to validate all inputs for all entry points, though) > > Personally I think a better approach is: > 1. improve/adding to the filters available, and if desired, add extra fla= gs/options e.g, to throw an exception on failure (which, btw was requested = via bugs.php.net 6 years ago), to set min/max values for FILTER_VALIDATE_FL= OAT, etc. > > 2a. Leave the multiple rules per input to userland (e.g. dev uses foreach= , array_walk, etc on a rules array or what have you) > 2b. *maybe* add an alternative to filter_(input/var)_array where it=E2=80= =99s 1 input and multiple rules, e.g. filter_(input|var)_multiple > > If you wanted to follow 2b, I=E2=80=99d suggest perhaps tackling it as a = separate RFC - improving *what* can be validated isn=E2=80=99t necessarily = tied to *how* you define what you want validated. Thank you for the suggestion. There are issues this approach. 1. Existing validator does not perform strict validations. We need new validator rules. e.g. FILTER_VALIDATE_STRICT_FLOAT - Do not allow white space prefix/postfix, raise exception. FILTER_VALIDATE_STRICT_INT - Do not allow white space prefix/postfix, raise exception. Having FILTER_VALIDATE_STRICT_FLOAT and FILTER_VALIDATE_FLOAT would be problematic. Current filter functions fallback to FILTER_UNSAFE_RAW when something goes wrong which is unacceptable for validator. 2a. Letting user to use foreach will results in the same, at least similar, rule definition array. Please take a look at the last reply to Rowan's post. 2b. This sounds good. I should think about implementation. 2a issue that result in the same or similar array definition remains though. Thank you for suggestions! Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net