Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:112343
MIME-Version: 1.0
References: <b845d5bf-63da-d564-1f0f-cc934188d693@aimeos.com>
 <b546b5bf-27f8-62cc-cc4b-d859132998a4@gmx.de> <0774c293-afd7-d8b9-175f-217ed600d1ea@aimeos.com>
 <fad4cc49-0f40-53f1-400c-90ff01ffdccb@gmx.de>
In-Reply-To: <fad4cc49-0f40-53f1-400c-90ff01ffdccb@gmx.de>
Date: Tue, 1 Dec 2020 10:07:31 -0800
Message-ID: <CAN2symiHHJVGvaLYMD=Qes0Wy06dZRtzPeonNBUWOBRbiTdo5g@mail.gmail.com>
To: "Christoph M. Becker" <cmbecker69@gmx.de>
Cc: "Aimeos | Norbert Sendetzky" <norbert@aimeos.com>, PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [PHP-DEV] Re: PHP 8 is_file/is_dir input handling
From: paul.crovella@gmail.com (Paul Crovella)

On Tue, Dec 1, 2020 at 9:43 AM Christoph M. Becker <cmbecker69@gmx.de> wrote:
>
> On 01.12.2020 at 18:35, Aimeos | Norbert Sendetzky wrote:
>
> > Am 01.12.20 um 18:24 schrieb Christoph M. Becker:
> >>
> >>> In PHP 7, this returns FALSE:
> >>>
> >>> php -r 'var_dump(is_file("ab\0c"));'
> >>>
> >>> In PHP 8, the same code throws a ValueException. Problem is now that
> >>> it's not possible to check upfront if the passed argument is a valid
> >>> path to avoid the exception being thrown.
> >>
> >> This is only about the NUL byte in the filename.  You can easily check
> >> for that yourself. :)
> >
> > There may be other checks that will throw a ValueException. I'm not sure
> > how it's implemented in detail because the filestat.c file doesn't
> > thrown an exception at all:
>
> The exception is thrown from inside the parameter parsing routines
> (zend_parse_parameters() and friends).  Internal function differenciate
> between string and path, whereas the latter is an arbitrary string which
> does not contain NUL bytes.
>
> It would likely make sense to document that.  OTOH, it's probably a good
> idea to check (almost) all user input for NUL bytes.

Would it not make more sense for something like is_file to have
obvious sane behavior and simply return false itself? I don't
understand the resistance to making it more difficult for a developer
to screw something up.