Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:36432
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 212.25.124.162 as permitted sender)
Message-ID: <47E7C36D.106@zend.com>
Date: Mon, 24 Mar 2008 18:06:21 +0300
User-Agent: Thunderbird 2.0.0.9 (Windows/20071031)
MIME-Version: 1.0
To: Gregory Beaver <greg@chiaraquartet.net>
CC: internals Mailing List <internals@lists.php.net>, 
 Stanislav Malyshev <stas@zend.com>
References: <47E2F8FA.20107@chiaraquartet.net> <47E37C42.10308@zend.com> <47E3E7F1.3060200@chiaraquartet.net> <47E792E1.9070803@zend.com> <47E7AC60.1040705@chiaraquartet.net> <47E7B89C.7060708@zend.com> <47E7BD9C.3040506@chiaraquartet.net>
In-Reply-To: <47E7BD9C.3040506@chiaraquartet.net>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] REMINDER - stream wrappers in include_path
From: dmitry@zend.com (Dmitry Stogov)



Gregory Beaver wrote:
> Dmitry Stogov wrote:
>> REALPATH_FAILED looks like a hack :(
>> PHP itself doesn't need it at all, according to your patch
>> php_stream_open() may just return NULL immediately instead of setting
>> of this flag.
>> However I am not sure that it will work for all cases. The fact that
>> we cannot resolve the path doesn't mean that we cannot open the file.
>> (On some systems getcwd() may fail on some reasons).
> I remember Solaris having issues with this, now that you mention it.
> 
> However, the place that I added this already returned NULL if realpath
> fails, it doesn't change the behavior, just moves the realpath to an
> earlier position and eliminates an unnecessary double call.  However, if
> we are comfortable having the double realpath on files not found in
> include_path (this would only have a performance affect on apps with
> lots of conditional loading of drivers where the majority are not
> found), this is fine with me.  I've attached a patch that removes
> REALPATH_FAILED so there are options.
> 
> Another option would be to determine which OSes this can fail, and
> simply not use REALPATH_FAILED on those OSes.
>> I didn't test the patch with pecl/phar.
>> Could you explain why php goes into the endless loop?
> because the cut/paste to plain_wrapper.c:1419 needs a ptr = end, unlike
> the code in fopen_wrappers.c.

You are right.

> Once you fix this problem, then the real
> logic problem rears its head, which is that we go into plain_wrapper to
> open an external stream wrapper.  It is far less efficient (about 4-6%
> by my measurements) to do this, which is why I moved the include_path
> parsing into streams.c

Did you measure pecl/phar or regular files?

> This bug only happens when the last item in include_path is a stream
> wrapper and the file is not found in include_path.

In general I don't have objections except of changing 
zend_resolve_path() (which is a callback for ZE) into 
php_resolve_path(). Anyway I'll look into patch once again with fresh head.

I would also ask Stas and other interested people to look into it.

Thanks. Dmitry.