Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:36230
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: <47E36763.3090506@zend.com>
Date: Fri, 21 Mar 2008 10:44:35 +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>
References: <47D0CD4F.7090409@chiaraquartet.net>
In-Reply-To: <47D0CD4F.7090409@chiaraquartet.net>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH] adding stream wrappers to include_path
From: dmitry@zend.com (Dmitry Stogov)

Hi Greg,

At least your patch is incomplete.
Even it handles stream wrappers in include(), include_once() it doesn't 
handle them in fopen() and some other functions.

In general I like the idea, so I'll look how to extend the support.

Thanks. Dmitry.

Gregory Beaver wrote:
> Hi,
> 
> I found some time to whip up a quick patch against 5.3.
> 
> This patch allows adding stream wrappers to include_path on both windows
> and unix.  This means one can set an include_path to
> ".:/usr/local/lib/php:phar:///path/to/ZF.phar/lib" or the windows
> equivalent ".;C:\php5;phar://C:/php5/ZF.phar/lib"  Any stream wrapper,
> userspace or built-in, will work as long as it satisfies security
> constraints and implements url_stat(), stream_open(), stat(), and read()
> or the C equivalents.
> 
> In other words, this test I already had created for pecl/phar works:
> 
> <?php
> $fname = dirname(__FILE__) . '/tempmanifest1.phar.php';
> $a = new Phar($fname);
> $a['file1.php'] = 'file1.php
> ';
> $a['test/file1.php'] = 'test/file1.php
> ';
> unset($a);
> set_include_path('.' . PATH_SEPARATOR . 'phar://' . $fname);
> include 'file1.php';
> set_include_path('.' . PATH_SEPARATOR . 'phar://' . $fname . '/test');
> include 'file1.php';
> include 'file2.php';
> ?>
> ===DONE===
> 
> with output:
> 
> --EXPECTF--
> file1.php
> test/file1.php
> 
> Warning: include(file2.php): failed to open stream: No such file or
> directory in %sinclude_path.php on line %d
> 
> Warning: include(): Failed opening 'file2.php' for inclusion
> (include_path='.:phar:///home/cellog/workspace/php5/ext/phar/tests/tempmanifest1.phar.php/test')
> in %sinclude_path.php on line %d
> ===DONE===
> 
> The internals of the patch borrows logic from
> php_stream_locate_url_wrapper to quickly detect a stream wrapper.  It
> excludes the data stream wrapper out of hand as this has no business in
> include_path for obvious reasons.  It also fully supports disabled
> allow_url_fopen/allow_url_include by passing STREAM_OPEN_FOR_INCLUDE to
> the eventual php_stream_locate_url_wrapper call.  The most important
> part of the patch is the way it finds a stream wrapper on unix.  Because
> PATH_SEPARATOR is ":" on unix, the include_path scanner will stop after
> "phar" in the sample include_paths above.  The new scanner checks for a
> valid stream wrapper (a sequence of alpha-numeric, +, -, or . characters
> followed by ://) and seeks to the next : if found.
> 
> My primary concern with the patch is performance.  I'm certain it can be
> tweaked.  I extracted the call to url_stat from php_stream_stat(), which
> removes the possibility of hitting stat cache but eliminates the
> redundant call to php_stream_locate_url_wrapper.  If it is indeed faster
> to use php_stream_stat(), that's one easy optimization.
> 
> In any case, it adds an additional scan of each include_path component
> to detect stream wrappers, which is called on each include.  This may be
> the biggest candidate for optimization, as a simple cache of paths with
> stream wrapper/not could be created whenever include_path is set and
> used instead of scanning the string every time.
> 
> By the way, there may be some unsafe scans in
> php_stream_locate_url_wrapper, specifically line 1531 in PHP_5_3:
> 
>     if ((*p == ':') && (n > 1) && (!strncmp("//", p+1, 2) ||
> !memcmp("data", path, 4))) {
> 
> It doesn't seem to verify that path is at least 4 or there is at least 2
> extra characters after p before checking for "//" or "data"
> 
> Thanks,
> Greg
>