- Fixed bug #61253: Wrappers opened with errors concurrency problem

#NOTE: There is a very small possibility that this will further break
#extensions that access wrapper->{err_stack, err_count}. On PECL SVN, rar is the
#only one and it may leak memory after this. I say "further break" because
#extensions that do that are already broken (will segfault) under ZTS, which is
#why this patch is necessary.
#There was what I deem as tacit acceptance from 5.3/5.4 RMs on this.

diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c
index ca882838ac7029322534d774739f74b3209db5c1..c036ad9fc85aa016673fcfe904de7510daf20571 100644 (file)
--- a/ext/standard/basic_functions.c
+++ b/ext/standard/basic_functions.c
@@ -3754,6 +3754,8 @@ PHP_RINIT_FUNCTION(basic) /* {{{ */
        /* Default to global filters only */
        FG(stream_filters) = NULL;
 
+       FG(wrapper_errors) = NULL;
+
        return SUCCESS;
 }
 /* }}} */

diff --git a/ext/standard/file.h b/ext/standard/file.h
index 0f0949d5de1ac19d1d347366931952bfca9a07a1..2d2406e0d545eba7ad28fac83ccba8a83cde4815 100644 (file)
--- a/ext/standard/file.h
+++ b/ext/standard/file.h
@@ -125,6 +125,7 @@ typedef struct {
        php_stream_context *default_context;
        HashTable *stream_wrappers;                     /* per-request copy of url_stream_wrappers_hash */
        HashTable *stream_filters;                      /* per-request copy of stream_filters_hash */
+       HashTable *wrapper_errors;                      /* key: wrapper address; value: linked list of char* */
 } php_file_globals;
 
 #ifdef ZTS

diff --git a/main/php_streams.h b/main/php_streams.h
index 1388169f1a90ee238b3f597a06602d48a90cb64c..291fd0d0ae32d04f11db36079e6a8e9f40ffd295 100755 (executable)
--- a/main/php_streams.h
+++ b/main/php_streams.h
@@ -162,10 +162,6 @@ struct _php_stream_wrapper {
        php_stream_wrapper_ops *wops;   /* operations the wrapper can perform */
        void *abstract;                                 /* context for the wrapper */
        int is_url;                                             /* so that PG(allow_url_fopen) can be respected */
-
-       /* support for wrappers to return (multiple) error messages to the stream opener */
-       int err_count;
-       char **err_stack;
 };
 
 #define PHP_STREAM_FLAG_NO_SEEK                                                1

diff --git a/main/streams/streams.c b/main/streams/streams.c
index d8bb9d3c7a63da14dd3f132b43a5598f3d9854df..9a95108bfeeadca0b3b1a937a02ab475e060cb9c 100755 (executable)
--- a/main/streams/streams.c
+++ b/main/streams/streams.c
@@ -157,20 +157,35 @@ PHPAPI int php_stream_from_persistent_id(const char *persistent_id, php_stream *
 
 /* }}} */
 
+static zend_llist *php_get_wrapper_errors_list(php_stream_wrapper *wrapper TSRMLS_DC)
+{
+    zend_llist *list = NULL;
+    if (!FG(wrapper_errors)) {
+        return NULL;
+    } else {
+        zend_hash_find(FG(wrapper_errors), (const char*)&wrapper,
+            sizeof wrapper, (void**)&list);
+        return list;
+    }
+}
+
 /* {{{ wrapper error reporting */
 void php_stream_display_wrapper_errors(php_stream_wrapper *wrapper, const char *path, const char *caption TSRMLS_DC)
 {
        char *tmp = estrdup(path);
        char *msg;
        int free_msg = 0;
-       php_stream_wrapper orig_wrapper;
 
        if (wrapper) {
-               if (wrapper->err_count > 0) {
-                       int i;
-                       size_t l;
+               zend_llist *err_list = php_get_wrapper_errors_list(wrapper TSRMLS_CC);
+               if (err_list) {
+                       size_t l = 0;
                        int brlen;
-                       char *br;
+                       int i;
+                       int count = zend_llist_count(err_list);
+                       const char *br;
+                       const char **err_buf_p;
+                       zend_llist_position pos;
 
                        if (PG(html_errors)) {
                                brlen = 7;
@@ -180,25 +195,29 @@ void php_stream_display_wrapper_errors(php_stream_wrapper *wrapper, const char *
                                br = "\n";
                        }
 
-                       for (i = 0, l = 0; i < wrapper->err_count; i++) {
-                               l += strlen(wrapper->err_stack[i]);
-                               if (i < wrapper->err_count - 1) {
+                       for (err_buf_p = zend_llist_get_first_ex(err_list, &pos), i = 0;
+                                       err_buf_p;
+                                       err_buf_p = zend_llist_get_next_ex(err_list, &pos), i++) {
+                               l += strlen(*err_buf_p);
+                               if (i < count - 1) {
                                        l += brlen;
                                }
                        }
                        msg = emalloc(l + 1);
                        msg[0] = '\0';
-                       for (i = 0; i < wrapper->err_count; i++) {
-                               strcat(msg, wrapper->err_stack[i]);
-                               if (i < wrapper->err_count - 1) {
-                                       strcat(msg, br);
+                       for (err_buf_p = zend_llist_get_first_ex(err_list, &pos), i = 0;
+                                       err_buf_p;
+                                       err_buf_p = zend_llist_get_next_ex(err_list, &pos), i++) {
+                               strcat(msg, *err_buf_p);
+                               if (i < count - 1) {
+                                       l += brlen;
                                }
                        }
 
                        free_msg = 1;
                } else {
                        if (wrapper == &php_plain_files_wrapper) {
-                               msg = strerror(errno);
+                               msg = strerror(errno); /* TODO: not ts on linux */
                        } else {
                                msg = "operation failed";
                        }
@@ -208,16 +227,7 @@ void php_stream_display_wrapper_errors(php_stream_wrapper *wrapper, const char *
        }
 
        php_strip_url_passwd(tmp);
-       if (wrapper) {
-               /* see bug #52935 */
-               orig_wrapper = *wrapper;
-               wrapper->err_stack = NULL;
-               wrapper->err_count = 0;
-       }
        php_error_docref1(NULL TSRMLS_CC, tmp, E_WARNING, "%s: %s", caption, msg);
-       if (wrapper) {
-               *wrapper = orig_wrapper;
-       }
        efree(tmp);
        if (free_msg) {
                efree(msg);
@@ -226,21 +236,16 @@ void php_stream_display_wrapper_errors(php_stream_wrapper *wrapper, const char *
 
 void php_stream_tidy_wrapper_error_log(php_stream_wrapper *wrapper TSRMLS_DC)
 {
-       if (wrapper) {
-               /* tidy up the error stack */
-               int i;
-
-               for (i = 0; i < wrapper->err_count; i++) {
-                       efree(wrapper->err_stack[i]);
-               }
-               if (wrapper->err_stack) {
-                       efree(wrapper->err_stack);
-               }
-               wrapper->err_stack = NULL;
-               wrapper->err_count = 0;
+       if (wrapper && FG(wrapper_errors)) {
+               zend_hash_del(FG(wrapper_errors), (const char*)&wrapper, sizeof wrapper);
        }
 }
 
+static void wrapper_error_dtor(void *error)
+{
+       efree(*(char**)error);
+}
+
 PHPAPI void php_stream_wrapper_log_error(php_stream_wrapper *wrapper, int options TSRMLS_DC, const char *fmt, ...)
 {
        va_list args;
@@ -254,11 +259,25 @@ PHPAPI void php_stream_wrapper_log_error(php_stream_wrapper *wrapper, int option
                php_error_docref(NULL TSRMLS_CC, E_WARNING, "%s", buffer);
                efree(buffer);
        } else {
-               /* append to stack */
-               wrapper->err_stack = erealloc(wrapper->err_stack, (wrapper->err_count + 1) * sizeof(char *));
-               if (wrapper->err_stack) {
-                       wrapper->err_stack[wrapper->err_count++] = buffer;
+               zend_llist *list = NULL;
+               if (!FG(wrapper_errors)) {
+                       ALLOC_HASHTABLE(FG(wrapper_errors));
+                       zend_hash_init(FG(wrapper_errors), 8, NULL,
+                                       (dtor_func_t)zend_llist_destroy, 0);
+               } else {
+                       zend_hash_find(FG(wrapper_errors), (const char*)&wrapper,
+                               sizeof wrapper, (void**)&list);
                }
+
+               if (!list) {
+                       zend_llist new_list;
+                       zend_llist_init(&new_list, sizeof buffer, wrapper_error_dtor, 0);
+                       zend_hash_update(FG(wrapper_errors), (const char*)&wrapper,
+                               sizeof wrapper, &new_list, sizeof new_list, (void**)&list);
+               }
+
+               /* append to linked list */
+               zend_llist_add_element(list, &buffer);
        }
 }
 
@@ -1599,6 +1618,12 @@ void php_shutdown_stream_hashes(TSRMLS_D)
                efree(FG(stream_filters));
                FG(stream_filters) = NULL;
        }
+    
+    if (FG(wrapper_errors)) {
+               zend_hash_destroy(FG(wrapper_errors));
+               efree(FG(wrapper_errors));
+               FG(wrapper_errors) = NULL;
+    }
 }
 
 int php_init_stream_wrappers(int module_number TSRMLS_DC)