From 3c0dba45109ade4b7de2938593ed2b9360d016c5 Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Wed, 23 Sep 2009 13:51:50 +0000 Subject: [PATCH] Fixed a bug in parameter passing/conversion API which caused tons of memleak and was the reason of ~1500 test failurs --- ext/standard/proc_open.c | 20 ++++++++++---------- ext/zip/php_zip.c | 24 ++++++++++++------------ main/php_streams.h | 36 ++++++++---------------------------- 3 files changed, 30 insertions(+), 50 deletions(-) diff --git a/ext/standard/proc_open.c b/ext/standard/proc_open.c index 82f0fdbcfc..1655b3e9f9 100644 --- a/ext/standard/proc_open.c +++ b/ext/standard/proc_open.c @@ -477,7 +477,7 @@ struct php_proc_open_descriptor_item { PHP_FUNCTION(proc_open) { zval **ppcommand, **ppcwd = NULL; - zval *command_with_args; + zval **command_with_args; char *command, *cwd=NULL; int command_len, cwd_len = 0; zval *descriptorspec; @@ -521,7 +521,7 @@ PHP_FUNCTION(proc_open) php_stream_context *context = FG(default_context); zend_uchar binary_pipes = 0; - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "zaz|Z!a!a!", &command_with_args, &descriptorspec, &pipes, &ppcwd, &environment, &other_options) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "Zaz|Z!a!a!", &command_with_args, &descriptorspec, &pipes, &ppcwd, &environment, &other_options) == FAILURE) { RETURN_FALSE; } @@ -562,17 +562,17 @@ PHP_FUNCTION(proc_open) if (bypass_shell) { zval **item; - if (Z_TYPE_P(command_with_args) != IS_ARRAY) { + if (Z_TYPE_PP(command_with_args) != IS_ARRAY) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "first parameter must be array when bypass_shell is on"); RETURN_FALSE; } - if (zend_hash_num_elements(Z_ARRVAL_P(command_with_args)) < 1) { + if (zend_hash_num_elements(Z_ARRVAL_PP(command_with_args)) < 1) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "arguments array must have at least one element"); RETURN_FALSE; } - zend_hash_internal_pointer_reset_ex(Z_ARRVAL_P(command_with_args), &pos); - if (zend_hash_get_current_data_ex(Z_ARRVAL_P(command_with_args), (void **)&item, &pos) == SUCCESS) { + zend_hash_internal_pointer_reset_ex(Z_ARRVAL_PP(command_with_args), &pos); + if (zend_hash_get_current_data_ex(Z_ARRVAL_PP(command_with_args), (void **)&item, &pos) == SUCCESS) { if (Z_TYPE_PP(item) == IS_STRING || Z_TYPE_PP(item) == IS_UNICODE) { ppcommand = item; } else { @@ -585,12 +585,12 @@ PHP_FUNCTION(proc_open) } } else { #endif - if (Z_TYPE_P(command_with_args) != IS_STRING && Z_TYPE_P(command_with_args) != IS_UNICODE) { + if (Z_TYPE_PP(command_with_args) != IS_STRING && Z_TYPE_PP(command_with_args) != IS_UNICODE) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "%s() expects parameter 1 to be string, %s given", get_active_function_name(TSRMLS_C), - zend_zval_type_name(command_with_args)); + zend_zval_type_name(*command_with_args)); RETURN_FALSE; } - ppcommand = &command_with_args; + ppcommand = command_with_args; /* command_len will be set below */ #if !defined(PHP_WIN32) && !defined(NETWARE) } @@ -602,7 +602,7 @@ PHP_FUNCTION(proc_open) #if !defined(PHP_WIN32) && !defined(NETWARE) if (bypass_shell) { - child_argv = _php_array_to_argv(command_with_args, is_persistent); + child_argv = _php_array_to_argv(*command_with_args, is_persistent); } #endif diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index 00c219e2ba..683b8287d4 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -2375,7 +2375,7 @@ static ZIPARCHIVE_METHOD(extractTo) struct zip *intern; zval *this = getThis(); - zval *zval_files = NULL; + zval **zval_files = NULL; zval **zval_file = NULL; php_stream_statbuf ssb; zval **pathto_zval; @@ -2391,7 +2391,7 @@ static ZIPARCHIVE_METHOD(extractTo) RETURN_FALSE; } - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "Z|z", &pathto_zval, &zval_files) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "Z|Z", &pathto_zval, &zval_files) == FAILURE) { return; } @@ -2411,16 +2411,16 @@ static ZIPARCHIVE_METHOD(extractTo) } ZIP_FROM_OBJECT(intern, this); - if (zval_files && (Z_TYPE_P(zval_files) != IS_NULL)) { - switch (Z_TYPE_P(zval_files)) { + if (zval_files && (Z_TYPE_PP(zval_files) != IS_NULL)) { + switch (Z_TYPE_PP(zval_files)) { case IS_UNICODE: - if (FAILURE == php_stream_path_param_encode(&zval_files, &file, &file_len, REPORT_ERRORS, FG(default_context))) { + if (FAILURE == php_stream_path_param_encode(zval_files, &file, &file_len, REPORT_ERRORS, FG(default_context))) { RETURN_FALSE; } case IS_STRING: - if (Z_TYPE_P(zval_files) != IS_UNICODE) { - file_len = Z_STRLEN_P(zval_files); - file = Z_STRVAL_P(zval_files); + if (Z_TYPE_PP(zval_files) != IS_UNICODE) { + file_len = Z_STRLEN_PP(zval_files); + file = Z_STRVAL_PP(zval_files); } if (file_len < 1) { efree(file); @@ -2428,19 +2428,19 @@ static ZIPARCHIVE_METHOD(extractTo) RETURN_FALSE; } - if (!php_zip_extract_file(intern, pathto, Z_STRVAL_P(zval_files), Z_STRLEN_P(zval_files) TSRMLS_CC)) { + if (!php_zip_extract_file(intern, pathto, Z_STRVAL_PP(zval_files), Z_STRLEN_PP(zval_files) TSRMLS_CC)) { efree(file); RETURN_FALSE; } break; case IS_ARRAY: - nelems = zend_hash_num_elements(Z_ARRVAL_P(zval_files)); + nelems = zend_hash_num_elements(Z_ARRVAL_PP(zval_files)); if (nelems == 0 ) { RETURN_FALSE; } for (i = 0; i < nelems; i++) { - if (zend_hash_index_find(Z_ARRVAL_P(zval_files), i, (void **) &zval_file) == SUCCESS) { + if (zend_hash_index_find(Z_ARRVAL_PP(zval_files), i, (void **) &zval_file) == SUCCESS) { switch (Z_TYPE_PP(zval_file)) { case IS_LONG: break; @@ -2449,7 +2449,7 @@ static ZIPARCHIVE_METHOD(extractTo) RETURN_FALSE; } case IS_STRING: - if (Z_TYPE_P(zval_files) != IS_UNICODE) { + if (Z_TYPE_PP(zval_files) != IS_UNICODE) { file_len = Z_STRLEN_PP(zval_file); file = Z_STRVAL_PP(zval_file); } diff --git a/main/php_streams.h b/main/php_streams.h index 472885a552..79200123b3 100755 --- a/main/php_streams.h +++ b/main/php_streams.h @@ -414,7 +414,6 @@ END_EXTERN_C() static inline int _php_stream_path_param_encode(zval **ppzval, char **ppath, int *ppath_len, int options, php_stream_context *context TSRMLS_DC) { if (Z_TYPE_PP(ppzval) == IS_UNICODE) { - zval *zpath; char *path; int path_len; @@ -422,35 +421,16 @@ static inline int _php_stream_path_param_encode(zval **ppzval, char **ppath, int if (FAILURE == php_stream_path_encode(NULL, &path, &path_len, Z_USTRVAL_PP(ppzval), Z_USTRLEN_PP(ppzval), options, context)) { return FAILURE; } - Z_ADDREF_PP(ppzval); /* the conversion removes a refcount */ - MAKE_STD_ZVAL(zpath); - ZVAL_STRINGL(zpath, path, path_len, 0); - Z_UNSET_ISREF_P(zpath); - Z_SET_REFCOUNT_P(zpath, 1); - - /* Replace the param stack with the new zval */ - zval_ptr_dtor(ppzval); - *ppzval = zpath; - } else if (Z_TYPE_PP(ppzval) != IS_STRING) { - if (Z_ISREF_PP(ppzval) || - Z_REFCOUNT_PP(ppzval) > 1) { - zval *zpath; - - /* Produce a new zval of type string */ - MAKE_STD_ZVAL(zpath); - *zpath = **ppzval; - zval_copy_ctor(zpath); - convert_to_string(zpath); - Z_UNSET_ISREF_P(zpath); - Z_SET_REFCOUNT_P(zpath, 1); - - /* Replace the param stack with it */ - zval_ptr_dtor(ppzval); - *ppzval = zpath; + if (Z_ISREF_PP(ppzval) || Z_REFCOUNT_PP(ppzval) == 1) { + zval_dtor(*ppzval); + ZVAL_STRINGL(*ppzval, path, path_len, 0); } else { - /* Convert the value on the param stack directly */ - convert_to_string(*ppzval); + zval_ptr_dtor(ppzval); + MAKE_STD_ZVAL(*ppzval); + ZVAL_STRINGL(*ppzval, path, path_len, 0); } + } else if (Z_TYPE_PP(ppzval) != IS_STRING) { + convert_to_string_ex(ppzval); } /* Populate convenience params if requested */ -- 2.50.1