Bug #76406 [Com]: file_exists() misleading error message
| From: | spam2 at rhsoft dot net | Date: | Sun, 03 Jun 2018 12:22:34 +0000 |
| Subject: | Bug #76406 [Com]: file_exists() misleading error message | ||
| References: | 1 | Groups: | php.bugs |
| Request: | Send a blank email to php-bugs+get-215484@lists.php.net to get a copy of this message | ||
Edit report at https://bugs.php.net/bug.php?id=76406&edit=1
ID: 76406
Comment by: spam2 at rhsoft dot net
Reported by: salsi at icosaedro dot it
Summary: file_exists() misleading error message
Status: Open
Type: Bug
Package: Filesystem function related
PHP Version: Irrelevant
Block user comment: N
Private report: N
New Comment:
> So you prefer to be lied to? (/etc/passwd may very well exist?)
for my application it don't exist and spit warnings to the errorlog is not helpful at all
"Warning: This function returns FALSE for files inaccessible due to safe mode
restrictions" - no matter if safe_mode now is gone - it has to return true in case i can access
a file and in every other false
what currently happens is that code which was not tested in a chroot or open_basedir environemnt
spits the logfiles full with warning for file_exists('../path'); and enforces developers
to spit the code full of @file_exists() while verybody knows the @ is typically the wrong solution
Previous Comments:
------------------------------------------------------------------------
[2018-06-02 22:47:28] cmb@php.net
> [â¦] and "a path" is a certain type of parameter that the engine
> supports.
To clarify: a path is a string which does not contain NUL bytes[1].
We may consider to introduce it as pseudo-type in the documentation.
> [â¦] it is not helpful to spit any warning for file_exists() and
> friends for whatever reason (open_basedir as example) [â¦]
> [â¦] i want a true/false
So you prefer to be lied to? (/etc/passwd may very well existâ¦)
Anyhow, changing the behavior of file_exists() and maybe other
filesystem function would require the RFC process[2].
[1] <https://github.com/php/php-src/blob/php-7.2.6/README.PARAMETER_PARSING_API#L65-L66>
[2] <https://wiki.php.net/rfc/howto>
------------------------------------------------------------------------
[2018-06-02 09:23:02] spam2 at rhsoft dot net
don't change the fact that it is not helpful to spit any warning for file_exists() and friends
for whatever reason (open_basedir as example) because sane environments are running with
error_reporting E_ALL and display_errors off so you are forced in way too many situations typing
@file_exists() while i don't care in the code why i can't access a given file, that's
why i ask file_exists() and i want a true/false
> An error could possibly be triggered if and only if
> the argument is something badly wrong like an object or an array
with strict_types you get a type error and then it's okay because you optet-in for that
behavior and it's catchable
------------------------------------------------------------------------
[2018-06-02 09:16:49] requinix@php.net
1. No, file_exists does not test valid paths. It tests if a "file" exists. The presumption
is that the path you are giving is valid, because if not then it couldn't possibly exist. And
no filesystem I've heard of allows NULs.
2. "expects parameter %d to be a valid path, string given" is a bit unusual. The message
says that because the check is done during the parameter check phase that every function uses, and
"a path" is a certain type of parameter that the engine supports. That also means altering
the message for this particular case may not be easy.
3. http://php.net/manual/en/functions.internal.php
------------------------------------------------------------------------
[2018-06-02 08:28:28] salsi at icosaedro dot it
Same exact behavior for these functions too, with same misleading error message, same undocumented
NULL return value and triggered E_WARNING:
is_writable() / is_writeable()
is_readable()
is_executable()
is_file()
is_dir()
is_link()
Again, in my opinion:
- These functions should do their business silently by testing their specific case on the submitted
string; no further diagnostic error should be emitted, as this could require a much more articulated
API for being useful to a running program. For example, programs may wish to test for the specific
structure of the path, URL, network resource, etc. or test for permissions, or test for remote
server accessibility, etc. and all this would requires specific API and feedback.
- Misleading error message still there.
- Unexpected undocumented NULL return value.
------------------------------------------------------------------------
[2018-06-02 07:42:16] salsi at icosaedro dot it
Description:
------------
If the string contains simply NUL, file_exists() gives a misleading error message and an unexpected
return value:
$ php -r "var_export(file_exists(\"\\x00\"));"
PHP Warning: file_exists() expects parameter 1 to be a valid path, string given in Command line
code on line 1
NULL
Three issues here:
Issue 1. "file_exists() expects parameter 1 to be a valid path": should not complain about
invalid paths, as it is just its purpose to check for valid paths!
Issue 2. "string given": just the type of data the function is expecting to get; then
what's the issue?
Issue 3. NULL is an undocumented returned value; only FALSE and TRUE expected.
In my opinion:
- This function should always return only either TRUE or FALSE depending on the given argument: if
the argument can be successfully detected being a local file, a remote file or resource or whatever,
it should return TRUE, FALSE otherwise.
- No errors should ever be triggered because testing for valid paths is just the purpose of the
function, so basically this function should only tell if the argument is or is not a valid file,
resource, etc. An error could possibly be triggered if and only if the argument is something badly
wrong like an object or an array, that may indicate an internal program failure. Folks used to map
errors into exceptions are quite sensitive to this argument...
- If an error really worth to be reported, it should tell the argument cannot be parsed as a valid
file, URL or resource path or something like that. And in this case too, only either FALSE or TRUE
should be returned. Or, the manual page should be fixed accordingly.
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=76406&edit=1