Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:90935
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.42 as permitted sender)
MIME-Version: 1.0
Sender: julienpauli@gmail.com
In-Reply-To: <CAGa2bXbiQDPpcKzM78OG6T1dpy5bWKQitXUKsRiDbSHJLe9wzQ@mail.gmail.com>
References: <9D.00.64206.7C430A65@pb1.pair.com> <CAGa2bXb0kUT9UstGHn1RTeKkmNQaVXmb6hbhfc-WCPmUMfgJxA@mail.gmail.com>
 <CAGa2bXbXsCKFHSPnw82vkVDmgrd-OQOtahozBdw8qMdN-n3PdQ@mail.gmail.com>
 <0D.E3.12955.15522A65@pb1.pair.com> <CAGa2bXbiQDPpcKzM78OG6T1dpy5bWKQitXUKsRiDbSHJLe9wzQ@mail.gmail.com>
Date: Tue, 26 Jan 2016 11:50:01 +0100
Message-ID: <CAMUwpuRqdENJYgP4VZpVQ=ZktH3ZPxGJBzQ3MpdNNw0VZA9WEw@mail.gmail.com>
To: Yasuo Ohgaki <yohgaki@ohgaki.net>, Daniel Lowrey <rdlowrey@php.net>
Cc: Umberto Salsi <salsi@icosaedro.it>, "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] Severe safety fail in file access and stream filters
From: jpauli@php.net (Julien Pauli)

On Tue, Jan 26, 2016 at 5:15 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
> Hi Umberto,
>
> On Fri, Jan 22, 2016 at 9:49 PM, Umberto Salsi <salsi@icosaedro.it> wrote:
>> thank you very much for the reply, I now start understanding better what
>> happen and why currently i/o cannot be handled in user's space.
>
> You're welcome.
> I think you have pointed out important missing parts in PHP.
>
>> Rather than an array of error, which might be quite expensive to handle,
>> why not a simple "char * lasterror" field added to the stream struct,
>> normally set to NULL, where a description of the last error occurred may
>> be stored? The high-level functions, that is PHP fopen, fread, fwrite,
>> fflush, fseek, fclose, may then check that string and, if set, trigger
>> E_WARNING with a meaningful message after having reset that string back
>> to NULL again. Many C libraries already provide such error strings,
>> starting from the same C strerror().
>
> Pgsql's "notice" logging is implemented this way at first. Then I changed
> it to array of notices recently.  (Notice is raised during
> transaction, for example)
>
> It's very difficult or impossible to detect where something wrong happened
> if there is only the last error. That's the reason why pgsql module is changed
> to have array of notices.
>
> The same would happen in streams, so it's better to store all errors during
> operation. Zend hash is extremely fast and good enough for logging errors,
> so we don't have to worry overheads.
>
>>
>> The same should then be done with the filters, which currently only may
>> return PSFS_ERR_FATAL, but might have their "char * lasterror" string
>> too with the same purpose. Knowing the exact reason of the failure is
>> essential to distinguish, for example, a dead disk drive or corrupted file
>> system, from a bare file mistakenly opened with the wrong decompressor
>> or a file which was badly encoded.
>
> I agree.
>
>>
>> Then all the specific stream and filter implementations can be adjusted
>> to set that error string; for those still not fixed that string remains
>> NULL and fopen, ferror, etc. will not report the error just as they do
>> today, and will be fixed later step by step.
>
> Agree here, too.
> Someone have to volunteer for this. It's not difficult for people on this list.
> I have too many backlogs for PHP project, so I hope someone volunteers
> for this.
>
> Regards,
>
> --
> Yasuo Ohgaki
> yohgaki@ohgaki.net
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>

I think what we should do is sit around the table with people
interested (Daniel Lowrey may be one of them), and plan a full rewrite
of streams for PHP next major (PHP 8). I don't think addind stuff to
next minors is a good idea, knowing we must prevent BC breaks in such
versions. We'll probably have to introduce some BC breaks is our tidy.

All our stream API is missing lots of features, and failing in many parts.

We could as well add to the reflection Async I/O and scatter/gather,
something I already thought about
for PHP 7, but found no time nor motivation nor friend contributors to
push to the level of quality we expect it for PHP.

While beeing perfectly valid, fixing just one little problem among
many others is not a durable solution for our stream API,
which is old, patched everywhere, but pretty stable and serving its
purpose as-is.

There are lots of problems in extensions as well, regarding streams,
where it is far from beeing easy to change one of the stream layer
behavior, or implement your own.
This should get addressed as well.


Julien.Pauli