Re: Re: PHP 8 is_file/is_dir input handling

On Tue, 1 Dec 2020 at 18:35, Paul Crovella <paul.crovella@gmail.com> wrote: > On Tue, Dec 1, 2020 at 10:23 AM G. P. B. <george.banyard@gmail.com> wrote: > > > > On Tue, 1 Dec 2020 at 18:07, Paul Crovella <paul.crovella@gmail.com> > wrote: > >> > >> 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. > >> > >> -- > >> PHP Internals - PHP Runtime Development Mailing List > >> To unsubscribe, visit: https://www.php.net/unsub.php > > > > > > So why having is_file()/is_dir() throw a warning for the past 8 years > > (since PHP 5.4) a non-issue? Because by that logic it shouldn't > > have been emitting warnings either. > > That's correct, it shouldn't have. > > > Would it have been fine if this would have been a TypeError as it was > > originally intended? > > No, I imagine it would've been fixed sooner. > Incorrect, because using strict_types would make this exact case throw a TypeError in PHP 7. See: https://3v4l.org/0P2O4 > > Is a warning fine because null bytes indicate a potential attack as in > no sane > > context should null bytes be passed around? > > Null bytes come from many places and do not necessarily indicate an > "attack." There's plenty of UTF-16 in the world, for example, that's > got oodles of them. > > > I don't personally *care* that it throws a ValueError, but why is this > issue only > > brought up *now* when it should have been shouting for 8 years > > Because it didn't break userland code for 8 years. A ValueError is a > much louder thing - that's the whole point of it in fact. It shouldn't > be a surprise that it comes up now. > Because returning null instead of false is not a surprise? George P. Banyard