Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:36423
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: <47E7AC60.1040705@chiaraquartet.net>
Date: Mon, 24 Mar 2008 08:28:00 -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>
In-Reply-To: <47E792E1.9070803@zend.com>
Content-Type: multipart/mixed;
 boundary="------------010402070402000600030705"
Subject: Re: [PHP-DEV] REMINDER - stream wrappers in include_path
From: greg@chiaraquartet.net (Gregory Beaver)

--------------010402070402000600030705
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Dmitry Stogov wrote:
> Hi Greg,
>
> In your second patch you forgot "break", so it couldn't work.
> I don't see any reason in second patch at all, because you call
> php_resolve_patch() from _php_stream_open_wrapper_ex() anyway.
>
> The bad thing with the first part that it calls realpath() function
> twice for each include("relative_path.php") and it makes several
> file-system accesses (look into `strace ... 2>&1| grep lstat`). With
> your patch I see 6 syscalls more.
>
> So I would prefer my patch that is more efficient for regular files.
> The same patch with fixed white-spaces is attached. 

Your patch is broken, it causes an endless loop in
tests/include_path.phpt when running phar tests.

Incidentally, I was unaware of the strace command (I've seen it bandied
about on irc, but thought it was something else), so thank you for
edumacatin me there.

The attached patch has no additional syscalls over the current case for
both existing and non-existing files, and actually works for phar as
well :).  It's also simpler (I removed the include stuff from
zend_vm_def.h), although you may want to add it back in and bench it to
see if it makes any difference in performance, since I found such a
dramatic reduction in operations here with it.

Greg

--------------010402070402000600030705
Content-Type: text/plain;
 name="wrapper3.patch.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="wrapper3.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 13:24:34 -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 13:24:34 -0000
@@ -511,6 +511,12 @@
 /* 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 ASSUME_REALPATH                 0x00004000
+
+/* assume the path passed in does not exist, and fail (plain wrapper only) */
+#define REALPATH_FAILED                 0x00008000
+
 /* 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 13:24:35 -0000
@@ -892,9 +892,16 @@
 		}
 		return NULL;
 	}
-	
-	if ((realpath = expand_filepath(filename, NULL TSRMLS_CC)) == NULL) {
-		return NULL;
+
+	if (options & ASSUME_REALPATH) {
+		realpath = estrdup(filename);
+	} else {
+		if (options & REALPATH_FAILED) {
+			return NULL;
+		}
+		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 13:24:36 -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,26 @@
 
 	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 |= ASSUME_REALPATH;
+			options &= ~USE_PATH;
+		} else {
+			options |= REALPATH_FAILED;
+			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 +1826,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 +1856,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 +1906,9 @@
 		pefree(copy_of_path, persistent);
 	}
 #endif
+	if (free_path) {
+		efree(path);
+	}
 	return stream;
 }
 /* }}} */

--------------010402070402000600030705--