Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:112467
Content-Type: text/plain;
	charset=us-ascii
Content-Transfer-Encoding: quoted-printable
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.4\))
Date: Tue, 8 Dec 2020 11:16:50 -0500
References: <b845d5bf-63da-d564-1f0f-cc934188d693@aimeos.com>
To: PHP internals <internals@lists.php.net>
In-Reply-To: <b845d5bf-63da-d564-1f0f-cc934188d693@aimeos.com>
Message-ID: <CA1EE0E0-E451-427A-9BE8-C84A8E5523DD@newclarity.net>
Subject: Re: [PHP-DEV] PHP 8 is_file/is_dir input handling
From: mike@newclarity.net (Mike Schinkel)

> On Dec 1, 2020, at 12:18 PM, Aimeos | Norbert Sendetzky =
<norbert@aimeos.com> wrote:
>=20
> Hi internals
>=20
> PHP 8 is stricter in checking input data then PHP 7. This is good but
> has some side effects for is_file(), is_dir() and similar functions =
when
> invalid paths are passed for checking.
>=20
> In PHP 7, this returns FALSE:
>=20
> php -r 'var_dump(is_file("ab\0c"));'
>=20
> 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.
>=20
> My suggestion for simpler client side code would be to return FALSE in
> this situation for PHP 8 too instead of throwing the ValueException.
> Otherwise, it's not possible to use is_file() and related functions
> without adding a try/catch block around in any web application.
>=20

This is a general case of throwing exceptions in PHP whenever an =
unwanted condition occurs, especially where code previously did not =
throw an exception.

The two schools of thought for error handling could be classified as:

1.) Throw for every unwanted condition and then handle later in a catch =
block, and=20
2.) Handle every unwanted condition at the point it is discovered. =20

Some believe only one strategy is valid but others disagree, so I argue =
there is no settled best practice.

When a PHP function throws an error if forces developers to wrap in =
try/catch, and the more functions throwing errors the more wrapping is =
needed.  Especially when the function could just have returned a false.

It is extremely easy to throw an Exception for is_file() when try/catch =
is needed, which is rare that an exception is needed for \0.

When following best practices, having is_file() throw adds complexity to =
every is_file() call, or developers to create their own safe_is_file() =
as mentioned in the thread.

In summary:

1. Please consider making is_file() return false for an embedded \0 and =
no longer throw an exception or generate a warning.

2. Beyond is_file(), please consider allowing PHP to support both types =
of error handling strategies without forcing complexity just to use the =
2nd strategy.

-Mike=