Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:112360
To: "Reindl Harald (privat)" <harry@rhsoft.net>,
 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>
 <29529061-dc71-c759-590a-b4786936f8c5@aimeos.com>
 <96e40442-a649-f9af-a0cc-dd43cfd1bd0c@gmx.de>
 <CAF+90c-xkV0PUBmOysrQYKoH2-muw0RHSamNJmM27HX1n+Px1Q@mail.gmail.com>
 <d798030e-e383-2ae5-f84c-d1175858970d@gmail.com>
 <2729ea34-b44d-da3d-f33e-4a31666112a2@rhsoft.net>
Message-ID: <8defcd2f-b887-eddc-a2b5-9d830eb12756@gmail.com>
Date: Tue, 1 Dec 2020 12:21:33 -0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:78.0)
 Gecko/20100101 Thunderbird/78.5.0
MIME-Version: 1.0
In-Reply-To: <2729ea34-b44d-da3d-f33e-4a31666112a2@rhsoft.net>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] Re: PHP 8 is_file/is_dir input handling
From: smalyshev@gmail.com (Stanislav Malyshev)

Hi!

> yeah, you should think about external input *before* do anything with 
> it, always! if you pass a random path with NULL you did not do anything 
> to validate the input

Yes, and? is_file should be safe (as in, not exploding and breaking the 
whole app) on any input (leaving typing discussion aside, any string 
input). It should return true if the input is a name of an existing 
file, false otherwise. It's simple, not?

> millions of security issues in whatever programming language are the 
> result of "i throw the input somewhere and don't mind"

This is a general banality which is not applicable to this specific 
functions. Sure, there are security issues that come from input 
validation failure. It is not the case here. As somebody who added those 
checks in most of the code personally, I can tell you not bailing out 
but returning false on is_file would not make security of this function 
worse in any way.

I know why it happens - because it has been treated as a type error 
(which was a nice hack but in retrospect probably not the most correct 
way) and then we decided to make type error throw and the fact that this 
is not actually a type came to bite us in the butt. I think the solution 
for this is to refactor this code and separate null checks from type 
checks. It was a nice hack for the time, but its time has expired.

> if you ever reach that exception you have a stacktrace up to the point 
> where you should have stopped proceed at all

Nope, there's no reason to stop processing when I check whether a random 
string signifies valid files. There might be a reason to stop processing 
later, after I discovered it is not, or continue processing, depending 
on the code intent - e.g. use alternative filename, or the default, or 
different code path. Exploding functions take this ability from me as an 
author of the code. So I will be forced to take it back by replacing 
every use of is_file with safe_is_file which would catch the exception 
and return false. Which just adds work for me which I hadn't to do 
before. That's not how the language should evolve - it shouldn't make 
things that are now easy harder.
-- 
Stas Malyshev
smalyshev@gmail.com