diff --git a/NEWS b/NEWS index a2c65685b4ce..e4b12cd41535 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,10 @@ PHP NEWS . Fixed bug GH-23301 (Nested "yield from" yields a value twice when the middle generator delegates again). (Lazizbek Ergashev) +- CLI: + . Fixed bug GH-23425 (sapi_cli_server_send_headers() does not check the + return value of php_cli_server_client_send_through()). (Lazizbek Ergashev) + - DOM: . Fixed a use-after-free when cloning a DOMNameSpaceNode after DOMDocument::xinclude(). (iliaal) @@ -28,6 +32,8 @@ PHP NEWS . Fixed Locale::parseLocale() reading past a trailing '-' or '_'. (iliaal, Xuyang Zhang) . Fixed grapheme_str_split() treating UBRK_DONE as a byte index. (iliaal) + . Fixed a leak in Locale::getKeywords() when a keyword value cannot be + read. (iliaal) - Opcache: . Fixed opcache.protect_memory race under ZTS. (realFlowControl) @@ -42,10 +48,14 @@ PHP NEWS with no other live PDO handle. (iliaal) - Standard: + . Fixed an out-of-bounds read when following a redirect response with an + empty Location header. (iliaal) . Fixed a memory leak in array_merge_recursive() when the recursive merge of an object converted to an array fails. (David Carlier) - Zip: + . Fixed bug GH-23276 (ZipArchive subclass storing its own stream cannot be + garbage collected). (Weilin Du, ndossche) . Fixed ZipArchive::extractTo() and ZipArchive::getFrom*() reporting success on corrupted entries. (David Carlier) diff --git a/ext/intl/locale/locale_methods.c b/ext/intl/locale/locale_methods.c index e3894b6f28fb..2643f4920878 100644 --- a/ext/intl/locale/locale_methods.c +++ b/ext/intl/locale/locale_methods.c @@ -776,6 +776,7 @@ PHP_FUNCTION( locale_get_keywords ) zend_string_efree( kw_value_str ); } zend_array_destroy(Z_ARR_P(return_value)); + uenum_close( e ); RETURN_FALSE; } diff --git a/ext/intl/tests/locale_get_keywords_failure.phpt b/ext/intl/tests/locale_get_keywords_failure.phpt new file mode 100644 index 000000000000..823da63ca541 --- /dev/null +++ b/ext/intl/tests/locale_get_keywords_failure.phpt @@ -0,0 +1,18 @@ +--TEST-- +Locale::getKeywords() closes the keyword enumeration on failure +--EXTENSIONS-- +intl +--SKIPIF-- += 59.1'); +} +?> +--FILE-- + +--EXPECT-- +bool(false) +bool(true) diff --git a/ext/standard/http_fopen_wrapper.c b/ext/standard/http_fopen_wrapper.c index 89125ed0765e..7173ada46711 100644 --- a/ext/standard/http_fopen_wrapper.c +++ b/ext/standard/http_fopen_wrapper.c @@ -1060,7 +1060,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, { char *loc_path = NULL; if (*header_info.location != '/') { - if (*(header_info.location+1) != '\0' && resource->path) { + if (header_info.location_len > 0 && *(header_info.location+1) != '\0' && resource->path) { char *s = strrchr(ZSTR_VAL(resource->path), '/'); if (!s) { s = ZSTR_VAL(resource->path); diff --git a/ext/standard/tests/http/http_empty_location_redirect.phpt b/ext/standard/tests/http/http_empty_location_redirect.phpt new file mode 100644 index 000000000000..a7f99bf1e249 --- /dev/null +++ b/ext/standard/tests/http/http_empty_location_redirect.phpt @@ -0,0 +1,36 @@ +--TEST-- +Empty Location header must not over-read when building the redirect target +--FILE-- + ['follow_location' => 1]]); +echo @file_get_contents("http://{{ ADDR }}/a/b", false, $ctx), "\n"; +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECT-- +uri=/ diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index 5330d78b36b0..5e640df9a102 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -340,6 +340,7 @@ static int php_zip_add_file(ze_zip_object *obj, const char *filename, size_t fil zip_flags_t flags ) /* {{{ */ { + struct zip *za = php_zip_object_za(obj); struct zip_source *zs; char resolved_path[MAXPATHLEN]; php_stream_statbuf ssb; @@ -365,33 +366,33 @@ static int php_zip_add_file(ze_zip_object *obj, const char *filename, size_t fil return -1; } flags ^= ZIP_FL_OPEN_FILE_NOW; - zs = zip_source_filep(obj->za, fd, offset_start, offset_len); + zs = zip_source_filep(za, fd, offset_start, offset_len); if (!zs) { fclose(fd); return FAILURE; } } else { - zs = zip_source_file(obj->za, resolved_path, offset_start, offset_len); + zs = zip_source_file(za, resolved_path, offset_start, offset_len); if (!zs) { return FAILURE; } } /* Replace */ if (replace >= 0) { - if (zip_file_replace(obj->za, replace, zs, flags) < 0) { + if (zip_file_replace(za, replace, zs, flags) < 0) { zip_source_free(zs); return -1; } - zip_error_clear(obj->za); + zip_error_clear(za); return 1; } /* Add */ - obj->last_id = zip_file_add(obj->za, entry_name, zs, flags); + obj->last_id = zip_file_add(za, entry_name, zs, flags); if (obj->last_id < 0) { zip_source_free(zs); return -1; } - zip_error_clear(obj->za); + zip_error_clear(za); return 1; } /* }}} */ @@ -517,7 +518,7 @@ static int php_zip_parse_options(HashTable *options, zip_options *opts) #define ZIP_FROM_OBJECT(intern, object) \ { \ ze_zip_object *obj = Z_ZIP_P(object); \ - intern = obj->za; \ + intern = php_zip_object_za(obj); \ if (!intern) { \ zend_value_error("Invalid or uninitialized Zip object"); \ RETURN_THROWS(); \ @@ -556,17 +557,18 @@ static int php_zip_parse_options(HashTable *options, zip_options *opts) static zend_long php_zip_status(ze_zip_object *obj) /* {{{ */ { + struct zip *za = php_zip_object_za(obj); int zep = obj->err_zip; /* saved err if closed */ - if (obj->za) { + if (za) { #if LIBZIP_VERSION_MAJOR < 1 int syp; - zip_error_get(obj->za, &zep, &syp); + zip_error_get(za, &zep, &syp); #else zip_error_t *err; - err = zip_get_error(obj->za); + err = zip_get_error(za); zep = zip_error_code_zip(err); zip_error_fini(err); #endif @@ -583,17 +585,18 @@ static zend_long php_zip_last_id(ze_zip_object *obj) /* {{{ */ static zend_long php_zip_status_sys(ze_zip_object *obj) /* {{{ */ { + struct zip *za = php_zip_object_za(obj); int syp = obj->err_sys; /* saved err if closed */ - if (obj->za) { + if (za) { #if LIBZIP_VERSION_MAJOR < 1 int zep; - zip_error_get(obj->za, &zep, &syp); + zip_error_get(za, &zep, &syp); #else zip_error_t *err; - err = zip_get_error(obj->za); + err = zip_get_error(za); syp = zip_error_code_system(err); zip_error_fini(err); #endif @@ -604,8 +607,10 @@ static zend_long php_zip_status_sys(ze_zip_object *obj) /* {{{ */ static zend_long php_zip_get_num_files(ze_zip_object *obj) /* {{{ */ { - if (obj->za) { - zip_int64_t num = zip_get_num_entries(obj->za, 0); + struct zip *za = php_zip_object_za(obj); + + if (za) { + zip_int64_t num = zip_get_num_entries(za, 0); return MIN(num, ZEND_LONG_MAX); } return 0; @@ -624,8 +629,10 @@ static char * php_zipobj_get_filename(ze_zip_object *obj, int *len) /* {{{ */ static char * php_zipobj_get_zip_comment(ze_zip_object *obj, int *len) /* {{{ */ { - if (obj->za) { - return (char *)zip_get_archive_comment(obj->za, len, 0); + struct zip *za = php_zip_object_za(obj); + + if (za) { + return (char *)zip_get_archive_comment(za, len, 0); } return NULL; } @@ -1080,11 +1087,11 @@ static HashTable *php_zip_get_properties(zend_object *object)/* {{{ */ #ifdef HAVE_PROGRESS_CALLBACK static void _php_zip_progress_callback_free(void *ptr) { - ze_zip_object *obj = ptr; + php_zip_archive *archive = ptr; - if (!Z_ISUNDEF(obj->progress_callback)) { - zval_ptr_dtor(&obj->progress_callback); - ZVAL_UNDEF(&obj->progress_callback); + if (!Z_ISUNDEF(archive->progress_callback)) { + zval_ptr_dtor(&archive->progress_callback); + ZVAL_UNDEF(&archive->progress_callback); } } #endif @@ -1092,48 +1099,83 @@ static void _php_zip_progress_callback_free(void *ptr) #ifdef HAVE_CANCEL_CALLBACK static void _php_zip_cancel_callback_free(void *ptr) { - ze_zip_object *obj = ptr; + php_zip_archive *archive = ptr; - if (!Z_ISUNDEF(obj->cancel_callback)) { - zval_ptr_dtor(&obj->cancel_callback); - ZVAL_UNDEF(&obj->cancel_callback); + if (!Z_ISUNDEF(archive->cancel_callback)) { + zval_ptr_dtor(&archive->cancel_callback); + ZVAL_UNDEF(&archive->cancel_callback); } } #endif -static void php_zip_object_free_storage(zend_object *object) /* {{{ */ +static php_zip_archive *php_zip_archive_create(struct zip *za) { - ze_zip_object * intern = php_zip_fetch_object(object); - int i; + php_zip_archive *archive = ecalloc(1, sizeof(php_zip_archive)); + + archive->za = za; + archive->refcount = 1; + + return archive; +} - if (!intern) { +void php_zip_archive_addref(php_zip_archive *archive) +{ + ZEND_ASSERT(archive->refcount > 0); + archive->refcount++; +} + +void php_zip_archive_release(php_zip_archive *archive) +{ + ZEND_ASSERT(archive->refcount > 0); + if (--archive->refcount != 0) { return; } - if (intern->za) { - if (zip_close(intern->za) != 0) { - php_error_docref(NULL, E_WARNING, "Cannot destroy the zip context: %s", zip_strerror(intern->za)); - zip_discard(intern->za); - } - } - if (intern->buffers_cnt>0) { - for (i=0; ibuffers_cnt; i++) { - efree(intern->buffers[i]); + if (archive->za) { + if (zip_close(archive->za) != 0) { + php_error_docref(NULL, E_WARNING, "Cannot destroy the zip context: %s", zip_strerror(archive->za)); + zip_discard(archive->za); } - efree(intern->buffers); } #ifdef HAVE_PROGRESS_CALLBACK - /* if not properly called by libzip */ - _php_zip_progress_callback_free(intern); + /* In case libzip did not invoke the callback state destructor. */ + _php_zip_progress_callback_free(archive); #endif #ifdef HAVE_CANCEL_CALLBACK - /* if not properly called by libzip */ - _php_zip_cancel_callback_free(intern); + /* In case libzip did not invoke the callback state destructor. */ + _php_zip_cancel_callback_free(archive); #endif - intern->za = NULL; + if (archive->buffers) { + for (int i = 0; i < archive->buffers_cnt; i++) { + efree(archive->buffers[i]); + } + efree(archive->buffers); + } + + efree(archive); +} + +/* The caller must close or discard released_za before detaching it. */ +static void php_zip_object_detach_archive(ze_zip_object *ze_obj, struct zip *released_za) +{ + ZEND_ASSERT(ze_obj->archive != NULL); + ZEND_ASSERT(ze_obj->archive->za == released_za); + ze_obj->archive->za = NULL; + php_zip_archive_release(ze_obj->archive); + ze_obj->archive = NULL; +} + +static void php_zip_object_free_storage(zend_object *object) /* {{{ */ +{ + ze_zip_object * intern = php_zip_fetch_object(object); + + if (intern->archive) { + php_zip_archive_release(intern->archive); + intern->archive = NULL; + } zend_object_std_dtor(&intern->zo); if (intern->filename) { @@ -1538,14 +1580,15 @@ PHP_METHOD(ZipArchive, open) RETURN_FALSE; } - if (ze_obj->za) { + if (ze_obj->archive) { /* we already have an opened zip, free it */ - if (zip_close(ze_obj->za) != 0) { + intern = ze_obj->archive->za; + if (zip_close(intern) != 0) { php_error_docref(NULL, E_WARNING, "Empty string as source"); efree(resolved_path); RETURN_FALSE; } - ze_obj->za = NULL; + php_zip_object_detach_archive(ze_obj, intern); } if (ze_obj->filename) { efree(ze_obj->filename); @@ -1577,7 +1620,7 @@ PHP_METHOD(ZipArchive, open) } ze_obj->filename = resolved_path; ze_obj->filename_len = strlen(resolved_path); - ze_obj->za = intern; + ze_obj->archive = php_zip_archive_create(intern); RETURN_TRUE; } /* }}} */ @@ -1653,7 +1696,7 @@ PHP_METHOD(ZipArchive, close) efree(ze_obj->filename); ze_obj->filename = NULL; ze_obj->filename_len = 0; - ze_obj->za = NULL; + php_zip_object_detach_archive(ze_obj, intern); if (!err) { RETURN_TRUE; @@ -1686,14 +1729,16 @@ PHP_METHOD(ZipArchive, clearError) { zval *self = ZEND_THIS; ze_zip_object *ze_obj; + struct zip *za; if (zend_parse_parameters_none() == FAILURE) { RETURN_THROWS(); } ze_obj = Z_ZIP_P(self); /* not ZIP_FROM_OBJECT as we can use saved error after close */ - if (ze_obj->za) { - zip_error_clear(ze_obj->za); + za = php_zip_object_za(ze_obj); + if (za) { + zip_error_clear(za); } else { ze_obj->err_zip = 0; ze_obj->err_sys = 0; @@ -1710,26 +1755,28 @@ PHP_METHOD(ZipArchive, getStatusString) char error_string[128]; #endif ze_zip_object *ze_obj; + struct zip *za; if (zend_parse_parameters_none() == FAILURE) { RETURN_THROWS(); } ze_obj = Z_ZIP_P(self); /* not ZIP_FROM_OBJECT as we can use saved error after close */ + za = php_zip_object_za(ze_obj); #if LIBZIP_VERSION_MAJOR < 1 - if (ze_obj->za) { - zip_error_get(ze_obj->za, &zep, &syp); + if (za) { + zip_error_get(za, &zep, &syp); len = zip_error_to_str(error_string, 128, zep, syp); } else { len = zip_error_to_str(error_string, 128, ze_obj->err_zip, ze_obj->err_sys); } RETVAL_STRINGL(error_string, len); #else - if (ze_obj->za) { + if (za) { zip_error_t *err; - err = zip_get_error(ze_obj->za); + err = zip_get_error(za); RETVAL_STRING(zip_error_strerror(err)); zip_error_fini(err); } else { @@ -1829,8 +1876,10 @@ static void php_zip_add_from_pattern(INTERNAL_FUNCTION_PARAMETERS, int type) /* int i; zval *zval_file; ze_zip_object *ze_obj; + struct zip *za; ze_obj = Z_ZIP_P(self); + za = php_zip_object_za(ze_obj); for (i = 0; i < found; i++) { char *file_stripped, *entry_name; @@ -1884,19 +1933,19 @@ static void php_zip_add_from_pattern(INTERNAL_FUNCTION_PARAMETERS, int type) /* RETURN_FALSE; } if (opts.comp_method >= 0) { - if (zip_set_file_compression(ze_obj->za, ze_obj->last_id, opts.comp_method, opts.comp_flags)) { + if (zip_set_file_compression(za, ze_obj->last_id, opts.comp_method, opts.comp_flags)) { zend_array_destroy(Z_ARR_P(return_value)); RETURN_FALSE; } } #ifdef HAVE_ENCRYPTION if (opts.enc_method >= 0) { - if (UNEXPECTED(zip_file_set_encryption(ze_obj->za, ze_obj->last_id, ZIP_EM_NONE, NULL) < 0)) { + if (UNEXPECTED(zip_file_set_encryption(za, ze_obj->last_id, ZIP_EM_NONE, NULL) < 0)) { zend_array_destroy(Z_ARR_P(return_value)); php_error_docref(NULL, E_WARNING, "password reset failed"); RETURN_FALSE; } - if (zip_file_set_encryption(ze_obj->za, ze_obj->last_id, opts.enc_method, opts.enc_password)) { + if (zip_file_set_encryption(za, ze_obj->last_id, opts.enc_method, opts.enc_password)) { zend_array_destroy(Z_ARR_P(return_value)); RETURN_FALSE; } @@ -2002,6 +2051,7 @@ PHP_METHOD(ZipArchive, addFromString) char *name; size_t name_len; ze_zip_object *ze_obj; + php_zip_archive *archive; struct zip_source *zs; int pos = 0; zend_long flags = ZIP_FL_OVERWRITE; @@ -2014,18 +2064,19 @@ PHP_METHOD(ZipArchive, addFromString) ZIP_FROM_OBJECT(intern, self); ze_obj = Z_ZIP_P(self); - if (ze_obj->buffers_cnt) { - ze_obj->buffers = (char **)safe_erealloc(ze_obj->buffers, sizeof(char *), (ze_obj->buffers_cnt+1), 0); - pos = ze_obj->buffers_cnt++; + archive = ze_obj->archive; + if (archive->buffers_cnt) { + archive->buffers = (char **)safe_erealloc(archive->buffers, sizeof(char *), (archive->buffers_cnt+1), 0); + pos = archive->buffers_cnt++; } else { - ze_obj->buffers = (char **)emalloc(sizeof(char *)); - ze_obj->buffers_cnt++; + archive->buffers = (char **)emalloc(sizeof(char *)); + archive->buffers_cnt++; pos = 0; } - ze_obj->buffers[pos] = (char *)safe_emalloc(ZSTR_LEN(buffer), 1, 1); - memcpy(ze_obj->buffers[pos], ZSTR_VAL(buffer), ZSTR_LEN(buffer) + 1); + archive->buffers[pos] = (char *)safe_emalloc(ZSTR_LEN(buffer), 1, 1); + memcpy(archive->buffers[pos], ZSTR_VAL(buffer), ZSTR_LEN(buffer) + 1); - zs = zip_source_buffer(intern, ze_obj->buffers[pos], ZSTR_LEN(buffer), 0); + zs = zip_source_buffer(intern, archive->buffers[pos], ZSTR_LEN(buffer), 0); if (zs == NULL) { RETURN_FALSE; @@ -3133,10 +3184,10 @@ static void _php_zip_progress_callback(zip_t *arch, double state, void *ptr) { zval cb_args[1]; zval cb_retval; - ze_zip_object *obj = ptr; + php_zip_archive *archive = ptr; ZVAL_DOUBLE(&cb_args[0], state); - if (call_user_function(EG(function_table), NULL, &obj->progress_callback, &cb_retval, 1, cb_args) == SUCCESS && !Z_ISUNDEF(cb_retval)) { + if (call_user_function(EG(function_table), NULL, &archive->progress_callback, &cb_retval, 1, cb_args) == SUCCESS && !Z_ISUNDEF(cb_retval)) { zval_ptr_dtor(&cb_retval); } } @@ -3149,7 +3200,7 @@ PHP_METHOD(ZipArchive, registerProgressCallback) double rate; zend_fcall_info fci; zend_fcall_info_cache fcc; - ze_zip_object *obj; + php_zip_archive *archive; if (zend_parse_parameters(ZEND_NUM_ARGS(), "df", &rate, &fci, &fcc) == FAILURE) { RETURN_THROWS(); @@ -3157,13 +3208,13 @@ PHP_METHOD(ZipArchive, registerProgressCallback) ZIP_FROM_OBJECT(intern, self); - obj = Z_ZIP_P(self); + archive = Z_ZIP_P(self)->archive; /* register */ - if (zip_register_progress_callback_with_state(intern, rate, _php_zip_progress_callback, _php_zip_progress_callback_free, obj)) { + if (zip_register_progress_callback_with_state(intern, rate, _php_zip_progress_callback, _php_zip_progress_callback_free, archive)) { RETURN_FALSE; } - ZVAL_COPY(&obj->progress_callback, &fci.function_name); + ZVAL_COPY(&archive->progress_callback, &fci.function_name); RETURN_TRUE; } @@ -3175,9 +3226,9 @@ static int _php_zip_cancel_callback(zip_t *arch, void *ptr) { zval cb_retval; int retval = 0; - ze_zip_object *obj = ptr; + php_zip_archive *archive = ptr; - if (call_user_function(EG(function_table), NULL, &obj->cancel_callback, &cb_retval, 0, NULL) == SUCCESS && !Z_ISUNDEF(cb_retval)) { + if (call_user_function(EG(function_table), NULL, &archive->cancel_callback, &cb_retval, 0, NULL) == SUCCESS && !Z_ISUNDEF(cb_retval)) { retval = zval_get_long(&cb_retval); zval_ptr_dtor(&cb_retval); } @@ -3192,20 +3243,20 @@ PHP_METHOD(ZipArchive, registerCancelCallback) zval *self = ZEND_THIS; zend_fcall_info fci; zend_fcall_info_cache fcc; - ze_zip_object *obj; + php_zip_archive *archive; if (zend_parse_parameters(ZEND_NUM_ARGS(), "f", &fci, &fcc) == FAILURE) { RETURN_THROWS(); } ZIP_FROM_OBJECT(intern, self); - obj = Z_ZIP_P(self); + archive = Z_ZIP_P(self)->archive; /* register */ - if (zip_register_cancel_callback_with_state(intern, _php_zip_cancel_callback, _php_zip_cancel_callback_free, obj)) { + if (zip_register_cancel_callback_with_state(intern, _php_zip_cancel_callback, _php_zip_cancel_callback_free, archive)) { RETURN_FALSE; } - ZVAL_COPY(&obj->cancel_callback, &fci.function_name); + ZVAL_COPY(&archive->cancel_callback, &fci.function_name); RETURN_TRUE; } diff --git a/ext/zip/php_zip.h b/ext/zip/php_zip.h index 84fdd21e3476..8385674a1cf0 100644 --- a/ext/zip/php_zip.h +++ b/ext/zip/php_zip.h @@ -65,23 +65,33 @@ typedef struct _ze_zip_read_rsrc { zend_long zip_rsrc_handle; } zip_read_rsrc; -/* Extends zend object */ -typedef struct _ze_zip_object { +/* Refcounted holder for the native archive state. + * Owned by a ZipArchive object and streams opened from it. */ +typedef struct _php_zip_archive { struct zip *za; + uint32_t refcount; + /* libzip reads buffers until the archive is closed, can outlive the object. */ char **buffers; - HashTable *prop_handler; - char *filename; - int filename_len; int buffers_cnt; - zip_int64_t last_id; - int err_zip; - int err_sys; #ifdef HAVE_PROGRESS_CALLBACK zval progress_callback; #endif #ifdef HAVE_CANCEL_CALLBACK zval cancel_callback; #endif +} php_zip_archive; + +/* Extends zend object */ +typedef struct _ze_zip_object { + /* NULL when there is no open archive, non-NULL otherwise. + * Owns one ref to the struct. */ + php_zip_archive *archive; + HashTable *prop_handler; + char *filename; + int filename_len; + zip_int64_t last_id; + int err_zip; + int err_sys; zend_object zo; } ze_zip_object; @@ -89,11 +99,19 @@ static inline ze_zip_object *php_zip_fetch_object(zend_object *obj) { return (ze_zip_object *)((char*)(obj) - XtOffsetOf(ze_zip_object, zo)); } +/* The archive an object currently has open, or NULL. */ +static zend_always_inline struct zip *php_zip_object_za(const ze_zip_object *obj) { + return obj->archive ? obj->archive->za : NULL; +} + #define Z_ZIP_P(zv) php_zip_fetch_object(Z_OBJ_P((zv))) php_stream *php_stream_zip_opener(php_stream_wrapper *wrapper, const char *path, const char *mode, int options, zend_string **opened_path, php_stream_context *context STREAMS_DC); php_stream *php_stream_zip_open(ze_zip_object *obj, struct zip_stat *sb, const char *mode, zip_flags_t flags STREAMS_DC); +void php_zip_archive_addref(php_zip_archive *archive); +void php_zip_archive_release(php_zip_archive *archive); + extern const php_stream_wrapper php_stream_zip_wrapper; #define LIBZIP_ATLEAST(m,n,p) (((m<<16) + (n<<8) + p) <= ((LIBZIP_VERSION_MAJOR<<16) + (LIBZIP_VERSION_MINOR<<8) + LIBZIP_VERSION_MICRO)) diff --git a/ext/zip/tests/gh23276.phpt b/ext/zip/tests/gh23276.phpt new file mode 100644 index 000000000000..ac005cd86354 --- /dev/null +++ b/ext/zip/tests/gh23276.phpt @@ -0,0 +1,95 @@ +--TEST-- +GH-23276 (ZipArchive subclass storing its own stream is collectable) +--CREDITS-- +Eyüp Can Akman +--EXTENSIONS-- +zip +--FILE-- +getStream('entry.txt'); + if (!is_resource($stream)) { + throw new Exception('Failed to open entry stream'); + } + return $stream; +} + +$filename = __DIR__ . '/gh23276.zip'; + +$zip = new Holder; +$zip->open($filename, ZipArchive::CREATE | ZipArchive::OVERWRITE); +$zip->addFromString('entry.txt', 'contents'); +$zip->close(); + +// An archive holding its own stream in a property must be destroyed. +$zip->open($filename, ZipArchive::RDONLY); +$zip->stream = getEntryStream($zip); +$weakRef = WeakReference::create($zip); +unset($zip); +var_dump($weakRef->get()); + +// Same through an indirect edge (property -> array -> resource). +$zip = new Holder; +$zip->open($filename, ZipArchive::RDONLY); +$zip->bag[] = getEntryStream($zip); +$weakRef = WeakReference::create($zip); +unset($zip); +var_dump($weakRef->get()); + +// Two archives cross-holding each other's streams. +$a = new Holder; +$b = new Holder; +$a->open($filename, ZipArchive::RDONLY); +$b->open($filename, ZipArchive::RDONLY); +$a->stream = getEntryStream($b); +$b->stream = getEntryStream($a); +$weakRef = WeakReference::create($a); +unset($a, $b); +var_dump($weakRef->get()); + +// A resurrected object must retain a usable stream. +$zip = new ResurrectingHolder; +$zip->open($filename, ZipArchive::RDONLY); +$zip->stream = getEntryStream($zip); +unset($zip); +var_dump($resurrected instanceof ResurrectingHolder); +var_dump(stream_get_contents($resurrected->stream)); +fclose($resurrected->stream); +unset($resurrected); + +// Externally held streams no longer keep the object alive. Closing one stream +// must not close the archive while another stream still uses it. +$zip = new Holder; +$zip->open($filename, ZipArchive::RDONLY); +$stream1 = getEntryStream($zip); +$stream2 = getEntryStream($zip); +$weakRef = WeakReference::create($zip); +unset($zip); +var_dump($weakRef->get()); +fclose($stream1); +var_dump(stream_get_contents($stream2)); +fclose($stream2); +?> +--CLEAN-- + +--EXPECT-- +NULL +NULL +NULL +bool(true) +string(8) "contents" +NULL +string(8) "contents" diff --git a/ext/zip/tests/gh23276_cancel_callback.phpt b/ext/zip/tests/gh23276_cancel_callback.phpt new file mode 100644 index 000000000000..5aa9057e6f39 --- /dev/null +++ b/ext/zip/tests/gh23276_cancel_callback.phpt @@ -0,0 +1,49 @@ +--TEST-- +GH-23276 (ZipArchive cancel callback outlives the object while a stream holds the archive) +--EXTENSIONS-- +zip +--SKIPIF-- + +--FILE-- +open($filename, ZipArchive::CREATE | ZipArchive::OVERWRITE); +$zip->addFromString('entry.txt', 'contents'); +$zip->close(); + +$zip->open($filename); +$callbackState = new stdClass; +$callbackStateRef = WeakReference::create($callbackState); +var_dump($zip->registerCancelCallback( + static function () use ($callbackState): int { return 0; }, +)); +$zip->addFromString('cancel.txt', 'late'); +$stream = $zip->getStream('entry.txt'); +if (!is_resource($stream)) { + throw new Exception('Failed to open entry stream'); +} +$weakRef = WeakReference::create($zip); +unset($callbackState, $zip); + +var_dump($weakRef->get()); +var_dump($callbackStateRef->get() !== null); +var_dump(stream_get_contents($stream)); +fclose($stream); +var_dump($callbackStateRef->get()); +?> +--CLEAN-- + +--EXPECT-- +bool(true) +NULL +bool(true) +string(8) "contents" +NULL diff --git a/ext/zip/tests/gh23276_close_with_open_stream.phpt b/ext/zip/tests/gh23276_close_with_open_stream.phpt new file mode 100644 index 000000000000..e216c95f4eb9 --- /dev/null +++ b/ext/zip/tests/gh23276_close_with_open_stream.phpt @@ -0,0 +1,47 @@ +--TEST-- +GH-23276 (ZipArchive dropping its archive while a stream is open leaves the object collectable) +--EXTENSIONS-- +zip +--FILE-- +open($filename, ZipArchive::CREATE | ZipArchive::OVERWRITE); +$zip->addFromString('entry.txt', 'contents'); +$zip->close(); + +$zip->open($filename, ZipArchive::RDONLY); +$stream = $zip->getStream('entry.txt'); +var_dump($zip->close()); +$weakRef = WeakReference::create($zip); +unset($zip); +var_dump($weakRef->get()); +var_dump(stream_get_contents($stream)); +fclose($stream); + +$zip = new ZipArchive; +$zip->open($filename, ZipArchive::RDONLY); +$stream = $zip->getStream('entry.txt'); +var_dump($zip->open($filename, ZipArchive::RDONLY)); +$weakRef = WeakReference::create($zip); +unset($zip); +var_dump($weakRef->get()); +var_dump(stream_get_contents($stream)); +fclose($stream); +?> +--CLEAN-- + +--EXPECTF-- +bool(true) +NULL + +Warning: stream_get_contents(): Zip stream error: %s in %s on line %d +string(0) "" +bool(true) +NULL + +Warning: stream_get_contents(): Zip stream error: %s in %s on line %d +string(0) "" diff --git a/ext/zip/tests/gh23276_progress_callback.phpt b/ext/zip/tests/gh23276_progress_callback.phpt new file mode 100644 index 000000000000..ff37b6233e5a --- /dev/null +++ b/ext/zip/tests/gh23276_progress_callback.phpt @@ -0,0 +1,50 @@ +--TEST-- +GH-23276 (ZipArchive progress callback outlives the object while a stream holds the archive) +--EXTENSIONS-- +zip +--SKIPIF-- + +--FILE-- +open($filename, ZipArchive::CREATE | ZipArchive::OVERWRITE); +$zip->addFromString('entry.txt', 'contents'); +$zip->close(); + +$zip->open($filename); +$callbackState = new stdClass; +$callbackStateRef = WeakReference::create($callbackState); +var_dump($zip->registerProgressCallback( + 0.5, + static function (float $rate) use ($callbackState): void {}, +)); +$zip->addFromString('progress.txt', 'late'); +$stream = $zip->getStream('entry.txt'); +if (!is_resource($stream)) { + throw new Exception('Failed to open entry stream'); +} +$weakRef = WeakReference::create($zip); +unset($callbackState, $zip); + +var_dump($weakRef->get()); +var_dump($callbackStateRef->get() !== null); +var_dump(stream_get_contents($stream)); +fclose($stream); +var_dump($callbackStateRef->get()); +?> +--CLEAN-- + +--EXPECT-- +bool(true) +NULL +bool(true) +string(8) "contents" +NULL diff --git a/ext/zip/tests/oo_addfromstring_reopen_memory.phpt b/ext/zip/tests/oo_addfromstring_reopen_memory.phpt new file mode 100644 index 000000000000..fb721c2d6273 --- /dev/null +++ b/ext/zip/tests/oo_addfromstring_reopen_memory.phpt @@ -0,0 +1,26 @@ +--TEST-- +ZipArchive::addFromString() buffers are released when the archive is closed +--EXTENSIONS-- +zip +--FILE-- +open($filename, ZipArchive::CREATE | ZipArchive::OVERWRITE); + $zip->addFromString('entry.txt', $blob); + $zip->close(); +} + +var_dump(memory_get_usage() - $start < 1000000); +?> +--CLEAN-- + +--EXPECT-- +bool(true) diff --git a/ext/zip/zip_stream.c b/ext/zip/zip_stream.c index 7f6990962d00..0356863ef7ce 100644 --- a/ext/zip/zip_stream.c +++ b/ext/zip/zip_stream.c @@ -34,7 +34,7 @@ struct php_zip_stream_data_t { struct zip_file *zf; size_t cursor; php_stream *stream; - ze_zip_object *owner; + php_zip_archive *archive; }; #define STREAM_DATA_FROM_STREAM() \ @@ -103,10 +103,10 @@ static int php_zip_ops_close(php_stream *stream, int close_handle) } } - /* the pinned object ref is tied to self, so release it regardless of close_handle */ - if (self->owner) { - OBJ_RELEASE(&self->owner->zo); - self->owner = NULL; + /* the archive ref is tied to self, so release it regardless of close_handle */ + if (self->archive) { + php_zip_archive_release(self->archive); + self->archive = NULL; } efree(self); stream->abstract = NULL; @@ -243,7 +243,7 @@ const php_stream_ops php_stream_zipio_ops = { /* {{{ php_stream_zip_open */ php_stream *php_stream_zip_open(ze_zip_object *obj, struct zip_stat *sb, const char *mode, zip_flags_t flags STREAMS_DC) { - struct zip *arch = obj->za; + struct zip *arch = php_zip_object_za(obj); struct zip_file *zf = NULL; php_stream *stream = NULL; @@ -262,9 +262,9 @@ php_stream *php_stream_zip_open(ze_zip_object *obj, struct zip_stat *sb, const c self->zf = zf; self->stream = NULL; self->cursor = 0; - /* keep the archive object alive while the stream borrows its zip_t */ - self->owner = obj; - GC_ADDREF(&obj->zo); + /* keep the zip_t alive while the stream borrows it */ + self->archive = obj->archive; + php_zip_archive_addref(self->archive); #if LIBZIP_ATLEAST(1,9,1) if (zip_file_is_seekable(zf) > 0) { stream = php_stream_alloc(&php_stream_zipio_seek_ops, self, NULL, mode); @@ -350,7 +350,7 @@ php_stream *php_stream_zip_opener(php_stream_wrapper *wrapper, self->zf = zf; self->stream = NULL; self->cursor = 0; - self->owner = NULL; + self->archive = NULL; #if LIBZIP_ATLEAST(1,9,1) if (zip_file_is_seekable(zf) > 0) { stream = php_stream_alloc(&php_stream_zipio_seek_ops, self, NULL, mode); diff --git a/run-tests.php b/run-tests.php index e4a4563f93fc..dfb07a465bd5 100755 --- a/run-tests.php +++ b/run-tests.php @@ -3386,6 +3386,7 @@ public function markTestAs( $time = $time ?? $this->getTimer($file_name); $this->record($suite, 'execution_time', $time); + $formatted_time = number_format($time, 4, '.', ''); $escaped_details = htmlspecialchars($details, ENT_QUOTES, 'UTF-8'); $escaped_details = preg_replace_callback('/[\0-\x08\x0B\x0C\x0E-\x1F]/', function ($c) { @@ -3394,7 +3395,7 @@ public function markTestAs( $escaped_message = htmlspecialchars($message, ENT_QUOTES, 'UTF-8'); $escaped_test_name = htmlspecialchars($file_name . ' (' . $test_name . ')', ENT_QUOTES); - $this->rootSuite['files'][$file_name]['xml'] = "\n"; + $this->rootSuite['files'][$file_name]['xml'] = "\n"; if (is_array($type)) { $output_type = $type[0] . 'ED'; @@ -3439,7 +3440,7 @@ private function getTimer(string $file_name) } if (isset($this->rootSuite['files'][$file_name]['total'])) { - return number_format($this->rootSuite['files'][$file_name]['total'], 4); + return $this->rootSuite['files'][$file_name]['total']; } return 0; diff --git a/sapi/cli/php_cli_server.c b/sapi/cli/php_cli_server.c index f4d29ce56855..36187aaeb035 100644 --- a/sapi/cli/php_cli_server.c +++ b/sapi/cli/php_cli_server.c @@ -177,6 +177,7 @@ typedef struct php_cli_server_client { zend_string *addr_str; php_http_parser parser; bool request_read; + bool headers_written; zend_string *current_header_name; zend_string *current_header_value; enum { HEADER_NONE=0, HEADER_FIELD, HEADER_VALUE } last_header_element; @@ -555,7 +556,7 @@ static int sapi_cli_server_send_headers(sapi_headers_struct *sapi_headers) /* {{ sapi_header_struct *h; zend_llist_position pos; - if (client == NULL || SG(request_info).no_headers) { + if (client == NULL || SG(request_info).no_headers || client->headers_written) { return SAPI_HEADER_SENT_SUCCESSFULLY; } @@ -578,10 +579,12 @@ static int sapi_cli_server_send_headers(sapi_headers_struct *sapi_headers) /* {{ } smart_str_appendl(&buffer, "\r\n", 2); - php_cli_server_client_send_through(client, ZSTR_VAL(buffer.s), ZSTR_LEN(buffer.s)); + size_t buffer_len = ZSTR_LEN(buffer.s); + bool sent = php_cli_server_client_send_through(client, ZSTR_VAL(buffer.s), buffer_len) == buffer_len; + client->headers_written = true; smart_str_free(&buffer); - return SAPI_HEADER_SENT_SUCCESSFULLY; + return sent ? SAPI_HEADER_SENT_SUCCESSFULLY : SAPI_HEADER_SEND_FAILED; } /* }}} */ @@ -1920,11 +1923,11 @@ static size_t php_cli_server_client_send_through(php_cli_server_client *client, } else { /* error or timeout */ php_handle_aborted_connection(); - return nbytes_left; + return str_len - nbytes_left; } } else { php_handle_aborted_connection(); - return nbytes_left; + return str_len - nbytes_left; } } nbytes_left -= nbytes_sent; @@ -1973,6 +1976,7 @@ static void php_cli_server_client_ctor(php_cli_server_client *client, php_cli_se php_http_parser_init(&client->parser, PHP_HTTP_REQUEST); client->request_read = false; + client->headers_written = false; client->last_header_element = HEADER_NONE; client->current_header_name = NULL; diff --git a/sapi/cli/tests/gh23425.phpt b/sapi/cli/tests/gh23425.phpt new file mode 100644 index 000000000000..98ed07859174 --- /dev/null +++ b/sapi/cli/tests/gh23425.phpt @@ -0,0 +1,40 @@ +--TEST-- +GH-23425 (sapi_cli_server_send_headers() does not check the return value of php_cli_server_client_send_through()) +--EXTENSIONS-- +sockets +--SKIPIF-- + +--FILE-- + 1, 'l_linger' => 0]); +socket_close($sock); + +$result_file = $info->docRoot . '/result.txt'; +for ($i = 0; $i < 40 && !file_exists($result_file); $i++) { + usleep(50000); +} + +echo file_get_contents($result_file), "\n"; +?> +--EXPECT-- +not-sent