Re: REMINDER - stream wrappers in include_path

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.