Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:36431
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain chiaraquartet.net from 38.99.98.18 cause and error)
Message-ID: <47E7BD9C.3040506@chiaraquartet.net>
Date: Mon, 24 Mar 2008 09:41:32 -0500
User-Agent: Thunderbird 2.0.0.12 (X11/20080227)
MIME-Version: 1.0
To: Dmitry Stogov <dmitry@zend.com>
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>
In-Reply-To: <47E7B89C.7060708@zend.com>
Content-Type: multipart/mixed;
 boundary="------------060007060100070708070800"
Subject: Re: [PHP-DEV] REMINDER - stream wrappers in include_path
From: greg@chiaraquartet.net (Gregory Beaver)

--------------060007060100070708070800
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

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.  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

This bug only happens when the last item in include_path is a stream
wrapper and the file is not found in include_path.

Greg

--------------060007060100070708070800
Content-Type: text/plain;
 name="wrapper5.patch.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="wrapper5.patch.txt"

Index: main/fopen_wrappers.c
===================================================================
RCS file: /repository/php-src/main/fopen_wrappers.c,v
retrieving revision 1.175.2.3.2.13.2.9
diff -u -r1.175.2.3.2.13.2.9 fopen_wrappers.c
--- main/fopen_wrappers.c	24 Mar 2008 09:30:41 -0000	1.175.2.3.2.13.2.9
+++ main/fopen_wrappers.c	24 Mar 2008 14:39:45 -0000
@@ -473,7 +473,15 @@
 
 	ptr = path;
 	while (ptr && *ptr) {
-		end = strchr(ptr, DEFAULT_DIR_SEPARATOR);
+		/* Check for stream wrapper */
+		int is_stream_wrapper = 0;
+
+		for (p = ptr; isalnum((int)*p) || *p == '+' || *p == '-' || *p == '.'; p++);
+		if ((*p == ':') && (p - ptr > 1) && (p[1] == '/') && (p[2] == '/')) {
+			p += 2;
+			is_stream_wrapper = 1;
+		}
+		end = strchr(p, DEFAULT_DIR_SEPARATOR);
 		if (end) {
 			if ((end-ptr) + 1 + filename_length + 1 >= MAXPATHLEN) {
 				ptr = end + 1;
@@ -494,6 +502,25 @@
 			memcpy(trypath+len+1, filename, filename_length+1);
 			ptr = NULL;
 		}
+		if (is_stream_wrapper) {
+			char *actual;
+			php_stream_wrapper *wrapper = php_stream_locate_url_wrapper(trypath, &actual, STREAM_OPEN_FOR_INCLUDE TSRMLS_CC);
+			
+			if (!wrapper) {
+				continue;
+			} else if (wrapper == &php_plain_files_wrapper) {
+				strncpy(trypath, actual, MAXPATHLEN);
+			} else {
+				if (wrapper->wops->url_stat) {
+					php_stream_statbuf ssb;
+
+					if (SUCCESS == wrapper->wops->url_stat(wrapper, trypath, 0, &ssb, NULL TSRMLS_CC)) {
+						return estrdup(trypath);
+					}
+				}
+				continue;
+			}
+		}
 		if (tsrm_realpath(trypath, resolved_path TSRMLS_CC)) {
 			return estrdup(resolved_path);
 		}
@@ -511,6 +538,29 @@
 		    exec_fname_length + 1 + filename_length + 1 < MAXPATHLEN) {
 			memcpy(trypath, exec_fname, exec_fname_length + 1);
 			memcpy(trypath+exec_fname_length + 1, filename, filename_length+1);
+
+			/* Check for stream wrapper */
+			for (p = trypath; isalnum((int)*p) || *p == '+' || *p == '-' || *p == '.'; p++);
+			if ((*p == ':') && (p - trypath > 1) && (p[1] == '/') && (p[2] == '/')) {
+				char *actual;
+				php_stream_wrapper *wrapper = php_stream_locate_url_wrapper(trypath, &actual, STREAM_OPEN_FOR_INCLUDE TSRMLS_CC);
+			
+				if (!wrapper) {
+					return NULL;
+				} else if (wrapper == &php_plain_files_wrapper) {
+					strncpy(trypath, actual, MAXPATHLEN);
+				} else {
+					if (wrapper->wops->url_stat) {
+						php_stream_statbuf ssb;
+
+						if (SUCCESS == wrapper->wops->url_stat(wrapper, trypath, 0, &ssb, NULL TSRMLS_CC)) {
+							return estrdup(trypath);
+						}
+					}
+					return NULL;
+				}
+			}
+
 			if (tsrm_realpath(trypath, resolved_path TSRMLS_CC)) {
 				return estrdup(resolved_path);
 			}
Index: main/php_streams.h
===================================================================
RCS file: /repository/php-src/main/php_streams.h,v
retrieving revision 1.103.2.1.2.4.2.2
diff -u -r1.103.2.1.2.4.2.2 php_streams.h
--- main/php_streams.h	31 Dec 2007 07:17:17 -0000	1.103.2.1.2.4.2.2
+++ main/php_streams.h	24 Mar 2008 14:39:46 -0000
@@ -511,6 +511,9 @@
 /* don't check allow_url_fopen and allow_url_include */
 #define STREAM_DISABLE_URL_PROTECTION   0x00002000
 
+/* assume the path passed in exists and is fully expanded, avoiding syscalls */
+#define STREAM_ASSUME_REALPATH          0x00004000
+
 /* Antique - no longer has meaning */
 #define IGNORE_URL_WIN 0
 
Index: main/streams/plain_wrapper.c
===================================================================
RCS file: /repository/php-src/main/streams/plain_wrapper.c,v
retrieving revision 1.52.2.6.2.23.2.5
diff -u -r1.52.2.6.2.23.2.5 plain_wrapper.c
--- main/streams/plain_wrapper.c	31 Dec 2007 07:17:17 -0000	1.52.2.6.2.23.2.5
+++ main/streams/plain_wrapper.c	24 Mar 2008 14:39:46 -0000
@@ -892,9 +892,13 @@
 		}
 		return NULL;
 	}
-	
-	if ((realpath = expand_filepath(filename, NULL TSRMLS_CC)) == NULL) {
-		return NULL;
+
+	if (options & STREAM_ASSUME_REALPATH) {
+		realpath = estrdup(filename);
+	} else {
+		if ((realpath = expand_filepath(filename, NULL TSRMLS_CC)) == NULL) {
+			return NULL;
+		}
 	}
 
 	if (persistent) {
Index: main/streams/streams.c
===================================================================
RCS file: /repository/php-src/main/streams/streams.c,v
retrieving revision 1.82.2.6.2.18.2.5
diff -u -r1.82.2.6.2.18.2.5 streams.c
--- main/streams/streams.c	12 Jan 2008 15:50:57 -0000	1.82.2.6.2.18.2.5
+++ main/streams/streams.c	24 Mar 2008 14:39:47 -0000
@@ -1773,7 +1773,7 @@
 	php_stream *stream = NULL;
 	php_stream_wrapper *wrapper = NULL;
 	char *path_to_open;
-	int persistent = options & STREAM_OPEN_PERSISTENT;
+	int persistent = options & STREAM_OPEN_PERSISTENT, free_path = 0;
 	char *copy_of_path = NULL;
 
 	
@@ -1787,9 +1787,25 @@
 
 	path_to_open = path;
 
+	if (options & USE_PATH) {
+		path_to_open = zend_resolve_path(path, strlen(path) TSRMLS_CC);
+		if (path_to_open) {
+			path = path_to_open;
+			free_path = 1;
+			/* we've found this file, don't re-check include_path or run realpath */
+			options |= STREAM_ASSUME_REALPATH;
+			options &= ~USE_PATH;
+		} else {
+			path_to_open = path;
+		}
+	}
+
 	wrapper = php_stream_locate_url_wrapper(path, &path_to_open, options TSRMLS_CC);
 	if (options & STREAM_USE_URL && (!wrapper || !wrapper->is_url)) {
 		php_error_docref(NULL TSRMLS_CC, E_WARNING, "This function may only be used against URLs");
+		if (free_path) {
+			efree(path);
+		}
 		return NULL;
 	}
 
@@ -1809,6 +1825,9 @@
 			php_stream_wrapper_log_error(wrapper, options ^ REPORT_ERRORS TSRMLS_CC,
 					"wrapper does not support persistent streams");
 			php_stream_close(stream);
+			if (free_path) {
+				efree(path);
+			}
 			stream = NULL;
 		}
 		
@@ -1836,12 +1855,18 @@
 					(options & STREAM_WILL_CAST)
 						? PHP_STREAM_PREFER_STDIO : PHP_STREAM_NO_PREFERENCE)) {
 			case PHP_STREAM_UNCHANGED:
+				if (free_path) {
+					efree(path);
+				}
 				return stream;
 			case PHP_STREAM_RELEASED:
 				if (newstream->orig_path) {
 					pefree(newstream->orig_path, persistent);
 				}
 				newstream->orig_path = pestrdup(path, persistent);
+				if (free_path) {
+					efree(path);
+				}
 				return newstream;
 			default:
 				php_stream_close(stream);
@@ -1880,6 +1905,9 @@
 		pefree(copy_of_path, persistent);
 	}
 #endif
+	if (free_path) {
+		efree(path);
+	}
 	return stream;
 }
 /* }}} */

--------------060007060100070708070800--