diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index 9fc0af4fcaca4..66b14e2513f8c 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -1019,25 +1019,23 @@ static HashTable *php_zip_get_properties(zend_object *object)/* {{{ */ /* }}} */ #ifdef HAVE_PROGRESS_CALLBACK -static void _php_zip_progress_callback_free(void *ptr) +static void php_zip_progress_callback_free(void *ptr) { ze_zip_object *obj = ptr; - if (!Z_ISUNDEF(obj->progress_callback)) { - zval_ptr_dtor(&obj->progress_callback); - ZVAL_UNDEF(&obj->progress_callback); + if (ZEND_FCC_INITIALIZED(obj->progress_callback)) { + zend_fcc_dtor(&obj->progress_callback); } } #endif #ifdef HAVE_CANCEL_CALLBACK -static void _php_zip_cancel_callback_free(void *ptr) +static void php_zip_cancel_callback_free(void *ptr) { ze_zip_object *obj = ptr; - if (!Z_ISUNDEF(obj->cancel_callback)) { - zval_ptr_dtor(&obj->cancel_callback); - ZVAL_UNDEF(&obj->cancel_callback); + if (ZEND_FCC_INITIALIZED(obj->cancel_callback)) { + zend_fcc_dtor(&obj->cancel_callback); } } #endif @@ -1066,12 +1064,12 @@ static void php_zip_object_free_storage(zend_object *object) /* {{{ */ #ifdef HAVE_PROGRESS_CALLBACK /* if not properly called by libzip */ - _php_zip_progress_callback_free(intern); + php_zip_progress_callback_free(intern); #endif #ifdef HAVE_CANCEL_CALLBACK /* if not properly called by libzip */ - _php_zip_cancel_callback_free(intern); + php_zip_cancel_callback_free(intern); #endif intern->za = NULL; @@ -3018,42 +3016,43 @@ PHP_METHOD(ZipArchive, getStream) } #ifdef HAVE_PROGRESS_CALLBACK -static void _php_zip_progress_callback(zip_t *arch, double state, void *ptr) +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; 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)) { - zval_ptr_dtor(&cb_retval); - } + zend_call_known_fcc(&obj->progress_callback, NULL, 1, cb_args, NULL); } /* {{{ register a progression callback: void callback(double state); */ PHP_METHOD(ZipArchive, registerProgressCallback) { struct zip *intern; - zval *self = ZEND_THIS; double rate; - zend_fcall_info fci; + zend_fcall_info dummy_fci; zend_fcall_info_cache fcc; ze_zip_object *obj; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "df", &rate, &fci, &fcc) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "dF", &rate, &dummy_fci, &fcc) == FAILURE) { RETURN_THROWS(); } - ZIP_FROM_OBJECT(intern, self); - - obj = Z_ZIP_P(self); + /* Inline ZIP_FROM_OBJECT(intern, self); */ + obj = Z_ZIP_P(ZEND_THIS); + intern = obj->za; + if (!intern) { \ + zend_value_error("Invalid or uninitialized Zip object"); + zend_release_fcall_info_cache(&fcc); + RETURN_THROWS(); + } /* free if called twice */ - _php_zip_progress_callback_free(obj); + php_zip_progress_callback_free(obj); /* register */ - ZVAL_COPY(&obj->progress_callback, &fci.function_name); - if (zip_register_progress_callback_with_state(intern, rate, _php_zip_progress_callback, _php_zip_progress_callback_free, obj)) { + zend_fcc_dup(&obj->progress_callback, &fcc); + if (zip_register_progress_callback_with_state(intern, rate, php_zip_progress_callback, php_zip_progress_callback_free, obj)) { RETURN_FALSE; } @@ -3063,42 +3062,55 @@ PHP_METHOD(ZipArchive, registerProgressCallback) #endif #ifdef HAVE_CANCEL_CALLBACK -static int _php_zip_cancel_callback(zip_t *arch, void *ptr) +static int php_zip_cancel_callback(zip_t *arch, void *ptr) { zval cb_retval; - int retval = 0; ze_zip_object *obj = ptr; - if (call_user_function(EG(function_table), NULL, &obj->cancel_callback, &cb_retval, 0, NULL) == SUCCESS && !Z_ISUNDEF(cb_retval)) { - retval = zval_get_long(&cb_retval); + zend_call_known_fcc(&obj->cancel_callback, &cb_retval, 0, NULL, NULL); + if (Z_ISUNDEF(cb_retval)) { + /* Cancel if an exception has been thrown */ + return -1; + } + bool failed = false; + zend_long retval = zval_try_get_long(&cb_retval, &failed); + if (failed) { + zend_type_error("Return value of callback provided to ZipArchive::registerCancelCallback()" + " must be of type int, %s returned", zend_zval_value_name(&cb_retval)); zval_ptr_dtor(&cb_retval); + return -1; } + zval_ptr_dtor(&cb_retval); - return retval; + return (int) retval; } /* {{{ register a progression callback: int callback(double state); */ PHP_METHOD(ZipArchive, registerCancelCallback) { struct zip *intern; - zval *self = ZEND_THIS; - zend_fcall_info fci; + zend_fcall_info dummy_fci; zend_fcall_info_cache fcc; ze_zip_object *obj; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "f", &fci, &fcc) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "F", &dummy_fci, &fcc) == FAILURE) { RETURN_THROWS(); } - ZIP_FROM_OBJECT(intern, self); - - obj = Z_ZIP_P(self); + /* Inline ZIP_FROM_OBJECT(intern, self); */ + obj = Z_ZIP_P(ZEND_THIS); + intern = obj->za; + if (!intern) { \ + zend_value_error("Invalid or uninitialized Zip object"); + zend_release_fcall_info_cache(&fcc); + RETURN_THROWS(); + } /* free if called twice */ - _php_zip_cancel_callback_free(obj); + php_zip_cancel_callback_free(obj); /* register */ - ZVAL_COPY(&obj->cancel_callback, &fci.function_name); - if (zip_register_cancel_callback_with_state(intern, _php_zip_cancel_callback, _php_zip_cancel_callback_free, obj)) { + zend_fcc_dup(&obj->cancel_callback, &fcc); + if (zip_register_cancel_callback_with_state(intern, php_zip_cancel_callback, php_zip_cancel_callback_free, obj)) { RETURN_FALSE; } diff --git a/ext/zip/php_zip.h b/ext/zip/php_zip.h index 82f7b5e057a43..6431b73fc3072 100644 --- a/ext/zip/php_zip.h +++ b/ext/zip/php_zip.h @@ -74,10 +74,10 @@ typedef struct _ze_zip_object { int err_zip; int err_sys; #ifdef HAVE_PROGRESS_CALLBACK - zval progress_callback; + zend_fcall_info_cache progress_callback; #endif #ifdef HAVE_CANCEL_CALLBACK - zval cancel_callback; + zend_fcall_info_cache cancel_callback; #endif zend_object zo; } ze_zip_object; diff --git a/ext/zip/tests/oo_cancel.phpt b/ext/zip/tests/oo_cancel.phpt index fa5fe92dadaad..85f418414d5d8 100644 --- a/ext/zip/tests/oo_cancel.phpt +++ b/ext/zip/tests/oo_cancel.phpt @@ -1,5 +1,5 @@ --TEST-- -registerCancelCallback +ZipArchive::registerCancelCallback() with a normal callback --EXTENSIONS-- zip --SKIPIF-- @@ -14,8 +14,6 @@ date.timezone=UTC $dirname = dirname(__FILE__) . '/'; $file = $dirname . '__tmp_oo_cancel.zip'; -@unlink($file); - $zip = new ZipArchive; if (!$zip->open($file, ZIPARCHIVE::CREATE)) { exit('failed'); @@ -33,6 +31,13 @@ var_dump($zip->getStatusString()); @unlink($file); ?> Done +--CLEAN-- + --EXPECTF-- bool(true) bool(true) diff --git a/ext/zip/tests/oo_cancel_non_int_return.phpt b/ext/zip/tests/oo_cancel_non_int_return.phpt new file mode 100644 index 0000000000000..6068a0ac60dc0 --- /dev/null +++ b/ext/zip/tests/oo_cancel_non_int_return.phpt @@ -0,0 +1,52 @@ +--TEST-- +ZipArchive::registerCancelCallback() with a callback returning an incorrect type +--EXTENSIONS-- +zip +--SKIPIF-- + +--INI-- +date.timezone=UTC +--FILE-- +open($file, ZIPARCHIVE::CREATE)) { + exit('failed'); +} + +/* Register a bogus callback */ +var_dump($zip->registerCancelCallback(function () { + return []; +})); +var_dump($zip->addFromString(PHP_BINARY, 'entry #1')); +try { + var_dump($zip->close()); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +var_dump($zip->status == ZipArchive::ER_CANCELLED); +var_dump($zip->getStatusString()); +@unlink($file); +?> +Done +--CLEAN-- + +--EXPECTF-- +bool(true) +bool(true) + +Warning: ZipArchive::close(): Operation cancelled in %s on line %d +TypeError: Return value of callback provided to ZipArchive::registerCancelCallback() must be of type int, array returned +bool(true) +string(19) "Operation cancelled" +Done diff --git a/ext/zip/tests/oo_cancel_trampoline.phpt b/ext/zip/tests/oo_cancel_trampoline.phpt new file mode 100644 index 0000000000000..2ea26c2cd7ddf --- /dev/null +++ b/ext/zip/tests/oo_cancel_trampoline.phpt @@ -0,0 +1,99 @@ +--TEST-- +ZipArchive::registerCancelCallback() with a trampoline +--EXTENSIONS-- +zip +--SKIPIF-- + +--INI-- +date.timezone=UTC +--FILE-- +open($file, ZIPARCHIVE::CREATE)) { + exit('failed'); +} + +var_dump($zip->registerCancelCallback($callback)); +var_dump($zip->addFromString(PHP_BINARY, 'entry #1')); +var_dump($zip->close()); +var_dump($zip->status == ZipArchive::ER_CANCELLED); +var_dump($zip->getStatusString()); + +echo "Set trampoline after closed Archive:\n"; +try { + var_dump($zip->registerCancelCallback($callback)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +@unlink($file); +unset($zip); + +$zip = new ZipArchive; +if (!$zip->open($file, ZIPARCHIVE::CREATE)) { + exit('failed'); +} + +var_dump($zip->registerCancelCallback($callbackThrow)); +var_dump($zip->addFromString(PHP_BINARY, 'entry #1')); +try { + var_dump($zip->close()); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +var_dump($zip->status == ZipArchive::ER_CANCELLED); +var_dump($zip->getStatusString()); +@unlink($file); + +?> +Done +--CLEAN-- + +--EXPECTF-- +bool(true) +bool(true) +Trampoline for trampoline +array(0) { +} + +Warning: ZipArchive::close(): Operation cancelled in %s on line %d +bool(false) +bool(true) +string(19) "Operation cancelled" +Set trampoline after closed Archive: +ValueError: Invalid or uninitialized Zip object +bool(true) +bool(true) +Trampoline for trampolineThrow + +Warning: ZipArchive::close(): Operation cancelled in %s on line %d +Exception: boo +bool(true) +string(19) "Operation cancelled" +Done diff --git a/ext/zip/tests/oo_progress.phpt b/ext/zip/tests/oo_progress.phpt index 690ef58a8f9e7..103a8556c2d91 100644 --- a/ext/zip/tests/oo_progress.phpt +++ b/ext/zip/tests/oo_progress.phpt @@ -1,5 +1,5 @@ --TEST-- -registerProgressCallback +ZipArchive::registerProgressCallback() with a normal callback --EXTENSIONS-- zip --SKIPIF-- @@ -14,8 +14,6 @@ date.timezone=UTC $dirname = dirname(__FILE__) . '/'; $file = $dirname . '__tmp_oo_progress.zip'; -@unlink($file); - $zip = new ZipArchive; if (!$zip->open($file, ZIPARCHIVE::CREATE)) { exit('failed'); @@ -32,6 +30,13 @@ var_dump($zip->close()); unlink($file); ?> Done +--CLEAN-- + --EXPECT-- bool(true) bool(true) diff --git a/ext/zip/tests/oo_progress_trampoline.phpt b/ext/zip/tests/oo_progress_trampoline.phpt new file mode 100644 index 0000000000000..f37bc26182db6 --- /dev/null +++ b/ext/zip/tests/oo_progress_trampoline.phpt @@ -0,0 +1,97 @@ +--TEST-- +ZipArchive::registerProgressCallback() with a trampoline +--EXTENSIONS-- +zip +--SKIPIF-- + +--INI-- +date.timezone=UTC +--FILE-- +open($file, ZIPARCHIVE::CREATE)) { + exit('failed'); +} +var_dump($zip->registerProgressCallback(0.5, $callback)); +var_dump($zip->addFromString('foo', 'entry #1')); +var_dump($zip->close()); + +echo "Set trampoline after closed Archive:\n"; +try { + var_dump($zip->registerProgressCallback(0.5, $callback)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +unlink($file); +unset($zip); + +$zip = new ZipArchive; +if (!$zip->open($file, ZIPARCHIVE::CREATE)) { + exit('failed'); +} + +var_dump($zip->registerProgressCallback(0.5, $callbackThrow)); +var_dump($zip->addFromString('foo', 'entry #1')); +try { + var_dump($zip->close()); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +unlink($file); +unset($zip); +?> +Done +--CLEAN-- + +--EXPECT-- +bool(true) +bool(true) +Trampoline for trampoline +array(1) { + [0]=> + float(0) +} +start +Trampoline for trampoline +array(1) { + [0]=> + float(1) +} +end +bool(true) +Set trampoline after closed Archive: +ValueError: Invalid or uninitialized Zip object +bool(true) +bool(true) +Trampoline for trampolineThrow +Exception: boo +Done