Re: REMINDER - stream wrappers in include_path

Hi Greg, in php_resolve_path() - you removed (p - filename > 1) check but it means that c://tmp will be used as stream wrapper - you added check for !wrapper, but we don't need to do any filesystem search for filename with wrong wrapper I've removed this modifications. I also added proper assignment to opened_path. I've committed the patch into PHP_5_3 and HEAD. Thanks. Dmitry. Gregory Beaver wrote:

Marcus Boerger wrote:

Hello Gregory,

+               for (p = ptr; isalnum((int)*p) || *p == '+' || *p == '-' || *p == '.'; p++);
+               /* p - ptr > 1 allows us to skip things like "C:\whatever" */
+               if ((*p == ':') && (p - ptr > 1) && (path_len - (p - path) > 2) && (p[1] == '/') && (p[2] == '/')) {
+                       /* .:// or ..:// is not a stream wrapper */
+                       if (p[-1] != '.' || p[-2] != '.' || p - 2 != ptr) {
+                               p += 3;
+                               is_stream_wrapper = 1;
+                       }
+               }

You missed one part though. C stops execution of a boolean expression on the first one that decides on the result. So if p[1] is '\0' then p[2] will never be accessed. So there is no access violation at all.

good point (i.e. duh on my part). attached patch removes that unnecessary paranoia.

Analyzing the check for '..:', took a long time :-) And I like that we check for this common case without going to the wrapper list. And we do not need to check for the common case '.' either as you require two chars in front of the ':', cool!

I found a few minor optimizations of this code just now, attached patch should be even better.

However with the check below:

+       if ((*p == ':') && (filename_length - (p - filename) > 2) && (p[1] == '/') && (p[2] == '/')) {
+               wrapper = php_stream_locate_url_wrapper(filename, &actual_path, STREAM_OPEN_FOR_INCLUDE TSRMLS_CC);                     +               if (wrapper == &php_plain_files_wrapper) {
+                       if (tsrm_realpath(actual_path, resolved_path TSRMLS_CC)) {
+                               return estrdup(resolved_path);
+                       }
+               }
                return NULL;

Don't we need to check for wrapper being NULL as in: if (!wrapper || wrapper == &php_plain_files_wrapper) {

Probably, I've added that in too. Greg