Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:112347
To: PHP Internals <internals@lists.php.net>
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>
 <CAN2symiHHJVGvaLYMD=Qes0Wy06dZRtzPeonNBUWOBRbiTdo5g@mail.gmail.com>
 <CAFPFaMLEdq0+hYkE49YVbcFvCt00rne6roApcoyY4aK=1ZHpuA@mail.gmail.com>
Organization: Aimeos GmbH
Message-ID: <29529061-dc71-c759-590a-b4786936f8c5@aimeos.com>
Date: Tue, 1 Dec 2020 19:38:51 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <CAFPFaMLEdq0+hYkE49YVbcFvCt00rne6roApcoyY4aK=1ZHpuA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Re: PHP 8 is_file/is_dir input handling
From: norbert@aimeos.com (Aimeos | Norbert Sendetzky)

Am 01.12.20 um 19:23 schrieb G. P. B.:
> 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.
> Would it have been fine if this would have been a TypeError as it was
> originally intended?
> Is a warning fine because null bytes indicate a potential attack as in no
> sane
> context should null bytes be passed around?
> 
> 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 and is
> either an
> indication of a bug or of something larger at play.

Keep cool, the code we are currently using is similar to this one:

if( @is_file( $data ) === false ) {
    throw new \Aimeos\MW\Exception( 'Invalid file' );
}

We use the silence operator to suppress the warning so we can throw our
own exception in a clean way. Now, with support for PHP 8 it would be:

try
{
    if( @is_file( $data ) === false ) {
        throw new \Aimeos\MW\Exception( 'Invalid file' );
    }
}
catch( \ValueException $e )
{
    throw new \Aimeos\MW\Exception( $e->getMessage(), 0 , $e );
}

But it has two problems:
- It's much more code for the same thing
- The stack trace begins at the new exception, not the one at is_file()

We can't use the ValueException directly because this will lead to two
different exceptions depending on the used PHP version and our unittests
currently fails because of the different exceptions.


Norbert