Commit ac7fddfaccc for php.net
commit ac7fddfaccc7069ada009804143e77478290c589
Author: David CARLIER <devnexen@gmail.com>
Date: Sun Nov 16 14:35:37 2025 +0000
ext/zip: further micro optimisations. (#20362)
- earlier return on invalid inputs from some ZipArchive methods.
- apply comp_flags type check for setCompressionName()/setCompressionIndex().
and throw exception for these
- add comp_flags check when passed as userland array option.
- adding a wrapper for zip_file_set_encryption workaround.
diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c
index 06bd41d602c..49506a19862 100644
--- a/ext/zip/php_zip.c
+++ b/ext/zip/php_zip.c
@@ -74,6 +74,17 @@ static int le_zip_entry;
# define add_ascii_assoc_string add_assoc_string
# define add_ascii_assoc_long add_assoc_long
+static bool php_zip_file_set_encryption(struct zip *intern, zend_long index, zend_long method, char *password) {
+ // FIXME: is a workaround to reset/free the password in case of consecutive calls.
+ // when libzip 1.11.5 is available, we can save this call in this case.
+ if (UNEXPECTED(zip_file_set_encryption(intern, (zip_uint64_t)index, ZIP_EM_NONE, NULL) < 0)) {
+ php_error_docref(NULL, E_WARNING, "password reset failed");
+ return false;
+ }
+
+ return (zip_file_set_encryption(intern, (zip_uint64_t)index, (zip_uint16_t)method, password) == 0);
+}
+
/* Flatten a path by making a relative path (to .)*/
static char * php_zip_make_relative_path(char *path, size_t path_len) /* {{{ */
{
@@ -367,14 +378,22 @@ static zend_result php_zip_parse_options(HashTable *options, zip_options *opts)
php_error_docref(NULL, E_WARNING, "Option \"comp_method\" must be of type int, %s given",
zend_zval_value_name(option));
}
- opts->comp_method = zval_get_long(option);
+ zend_long comp_method = zval_get_long(option);
+ if (comp_method < 0 || comp_method > INT_MAX) {
+ php_error_docref(NULL, E_WARNING, "Option \"comp_method\" must be between 0 and %d", INT_MAX);
+ }
+ opts->comp_method = (zip_int32_t)comp_method;
if ((option = zend_hash_str_find(options, "comp_flags", sizeof("comp_flags") - 1)) != NULL) {
if (Z_TYPE_P(option) != IS_LONG) {
php_error_docref(NULL, E_WARNING, "Option \"comp_flags\" must be of type int, %s given",
zend_zval_value_name(option));
}
- opts->comp_flags = zval_get_long(option);
+ zend_long comp_flags = zval_get_long(option);
+ if (comp_flags < 0 || comp_flags > USHRT_MAX) {
+ php_error_docref(NULL, E_WARNING, "Option \"comp_flags\" must be between 0 and %u", USHRT_MAX);
+ }
+ opts->comp_flags = (zip_uint32_t)comp_flags;
}
}
@@ -1752,12 +1771,7 @@ static void php_zip_add_from_pattern(INTERNAL_FUNCTION_PARAMETERS, int type) /*
}
#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)) {
- 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 (!php_zip_file_set_encryption(ze_obj->za, ze_obj->last_id, opts.enc_method, opts.enc_password)) {
zend_array_destroy(Z_ARR_P(return_value));
RETURN_FALSE;
}
@@ -2279,15 +2293,7 @@ PHP_METHOD(ZipArchive, setEncryptionName)
RETURN_FALSE;
}
- if (UNEXPECTED(zip_file_set_encryption(intern, idx, ZIP_EM_NONE, NULL) < 0)) {
- php_error_docref(NULL, E_WARNING, "password reset failed");
- RETURN_FALSE;
- }
-
- if (zip_file_set_encryption(intern, idx, (zip_uint16_t)method, password)) {
- RETURN_FALSE;
- }
- RETURN_TRUE;
+ RETURN_BOOL(php_zip_file_set_encryption(intern, idx, method, password));
}
/* }}} */
@@ -2307,12 +2313,7 @@ PHP_METHOD(ZipArchive, setEncryptionIndex)
ZIP_FROM_OBJECT(intern, self);
- if (UNEXPECTED(zip_file_set_encryption(intern, index, ZIP_EM_NONE, NULL) < 0)) {
- php_error_docref(NULL, E_WARNING, "password reset failed");
- RETURN_FALSE;
- }
-
- RETURN_BOOL(zip_file_set_encryption(intern, index, (zip_uint16_t)method, password) == 0);
+ RETURN_BOOL(php_zip_file_set_encryption(intern, index, method, password));
}
/* }}} */
#endif
@@ -2390,13 +2391,23 @@ PHP_METHOD(ZipArchive, setCompressionName)
RETURN_THROWS();
}
- ZIP_FROM_OBJECT(intern, this);
-
if (name_len == 0) {
zend_argument_must_not_be_empty_error(1);
RETURN_THROWS();
}
+ if (comp_method < -1 || comp_method > INT_MAX) {
+ zend_argument_value_error(2, "must be between -1 and %d", INT_MAX);
+ RETURN_THROWS();
+ }
+
+ if (comp_flags < 0 || comp_flags > USHRT_MAX) {
+ // comp_flags is cast down accordingly in libzip, zip_entry_t compression_level is of zip_uint16_t
+ zend_argument_value_error(3, "must be between 0 and %u", USHRT_MAX);
+ RETURN_THROWS();
+ }
+
+ ZIP_FROM_OBJECT(intern, this);
idx = zip_name_locate(intern, name, 0);
if (idx < 0) {
@@ -2421,6 +2432,21 @@ PHP_METHOD(ZipArchive, setCompressionIndex)
RETURN_THROWS();
}
+ if (index < 0) {
+ RETURN_FALSE;
+ }
+
+ if (comp_method < -1 || comp_method > INT_MAX) {
+ zend_argument_value_error(2, "must be between -1 and %d", INT_MAX);
+ RETURN_THROWS();
+ }
+
+ if (comp_flags < 0 || comp_flags > USHRT_MAX) {
+ // comp_flags is cast down accordingly in libzip, zip_entry_t compression_level is of zip_uint16_t
+ zend_argument_value_error(3, "must be between 0 and %u", USHRT_MAX);
+ RETURN_THROWS();
+ }
+
ZIP_FROM_OBJECT(intern, this);
RETURN_BOOL(zip_set_file_compression(intern, (zip_uint64_t)index,
@@ -2443,13 +2469,13 @@ PHP_METHOD(ZipArchive, setMtimeName)
RETURN_THROWS();
}
- ZIP_FROM_OBJECT(intern, this);
-
if (name_len == 0) {
zend_argument_must_not_be_empty_error(1);
RETURN_THROWS();
}
+ ZIP_FROM_OBJECT(intern, this);
+
idx = zip_name_locate(intern, name, 0);
if (idx < 0) {
@@ -2515,17 +2541,15 @@ PHP_METHOD(ZipArchive, deleteName)
RETURN_THROWS();
}
- ZIP_FROM_OBJECT(intern, self);
-
if (name_len < 1) {
RETURN_FALSE;
}
+ ZIP_FROM_OBJECT(intern, self);
+
PHP_ZIP_STAT_PATH(intern, name, name_len, 0, sb);
- if (zip_delete(intern, sb.index)) {
- RETURN_FALSE;
- }
- RETURN_TRUE;
+
+ RETURN_BOOL(zip_delete(intern, sb.index) == 0);
}
/* }}} */
@@ -2546,13 +2570,13 @@ PHP_METHOD(ZipArchive, renameIndex)
RETURN_FALSE;
}
- ZIP_FROM_OBJECT(intern, self);
-
if (new_name_len == 0) {
zend_argument_must_not_be_empty_error(2);
RETURN_THROWS();
}
+ ZIP_FROM_OBJECT(intern, self);
+
RETURN_BOOL(zip_file_rename(intern, index, (const char *)new_name, 0) == 0);
}
/* }}} */
@@ -2594,12 +2618,12 @@ PHP_METHOD(ZipArchive, unchangeIndex)
RETURN_THROWS();
}
- ZIP_FROM_OBJECT(intern, self);
-
if (index < 0) {
RETURN_FALSE;
}
+ ZIP_FROM_OBJECT(intern, self);
+
RETURN_BOOL(zip_unchange(intern, index) == 0);
}
/* }}} */
@@ -2617,12 +2641,12 @@ PHP_METHOD(ZipArchive, unchangeName)
RETURN_THROWS();
}
- ZIP_FROM_OBJECT(intern, self);
-
if (name_len < 1) {
RETURN_FALSE;
}
+ ZIP_FROM_OBJECT(intern, self);
+
PHP_ZIP_STAT_PATH(intern, name, name_len, 0, sb);
RETURN_BOOL(zip_unchange(intern, sb.index) == 0);
@@ -2686,8 +2710,6 @@ PHP_METHOD(ZipArchive, extractTo)
Z_PARAM_ARRAY_HT_OR_STR_OR_NULL(files_ht, files_str)
ZEND_PARSE_PARAMETERS_END();
- ZIP_FROM_OBJECT(intern, self);
-
if (pathto_len < 1) {
RETURN_FALSE;
}
@@ -2700,6 +2722,7 @@ PHP_METHOD(ZipArchive, extractTo)
}
uint32_t nelems, i;
+ ZIP_FROM_OBJECT(intern, self);
if (files_str) {
if (!php_zip_extract_file(intern, pathto, ZSTR_VAL(files_str), ZSTR_LEN(files_str), -1)) {
@@ -2796,7 +2819,7 @@ static void php_zip_get_from(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */
RETURN_FALSE;
}
- buffer = zend_string_safe_alloc(1, len, 0, 0);
+ buffer = zend_string_safe_alloc(1, len, 0, false);
n = zip_fread(zf, ZSTR_VAL(buffer), ZSTR_LEN(buffer));
if (n < 1) {
zend_string_efree(buffer);
@@ -3005,6 +3028,10 @@ PHP_METHOD(ZipArchive, isCompressionMethodSupported)
if (zend_parse_parameters(ZEND_NUM_ARGS(), "l|b", &method, &enc) == FAILURE) {
return;
}
+ if (method < -1 || method > INT_MAX) {
+ zend_argument_value_error(1, "must be between -1 and %d", INT_MAX);
+ RETURN_THROWS();
+ }
RETVAL_BOOL(zip_compression_method_supported((zip_int32_t)method, enc));
}
/* }}} */
@@ -3018,7 +3045,11 @@ PHP_METHOD(ZipArchive, isEncryptionMethodSupported)
if (zend_parse_parameters(ZEND_NUM_ARGS(), "l|b", &method, &enc) == FAILURE) {
return;
}
- RETVAL_BOOL(zip_encryption_method_supported((zip_uint16_t)method, enc));
+ if (method < 0 || method > USHRT_MAX) {
+ zend_argument_value_error(1, "must be between 0 and %u", USHRT_MAX);
+ RETURN_THROWS();
+ }
+ RETURN_BOOL(zip_encryption_method_supported((zip_uint16_t)method, enc));
}
/* }}} */
#endif
diff --git a/ext/zip/tests/oo_addglob2.phpt b/ext/zip/tests/oo_addglob2.phpt
index 517c0b7fd7f..a98a2227120 100644
--- a/ext/zip/tests/oo_addglob2.phpt
+++ b/ext/zip/tests/oo_addglob2.phpt
@@ -33,10 +33,26 @@
$options = [
'remove_all_path' => true,
'comp_method' => ZipArchive::CM_STORE,
- 'comp_flags' => 5,
+ 'comp_flags' => PHP_INT_MIN,
'enc_method' => ZipArchive::EM_AES_256,
'enc_password' => 'secret',
];
+
+try {
+ $zip->addGlob($dirname. 'bar.*', GLOB_BRACE, $options);
+} catch (\ValueError $e) {
+ echo $e->getMessage(), PHP_EOL;
+}
+
+$options['comp_flags'] = 65536;
+
+try {
+ $zip->addGlob($dirname. 'bar.*', GLOB_BRACE, $options);
+} catch (\ValueError $e) {
+ echo $e->getMessage(), PHP_EOL;
+}
+
+$options['comp_flags'] = 5;
if (!$zip->addGlob($dirname . 'bar.*', GLOB_BRACE, $options)) {
echo "failed 2\n";
}
@@ -61,6 +77,10 @@
include $dirname . 'utils.inc';
rmdir_rf(__DIR__ . '/__tmp_oo_addglob2/');
?>
---EXPECT--
+--EXPECTF--
+
+Warning: ZipArchive::addGlob(): Option "comp_flags" must be between 0 and 65535 in %s on line %d
+
+Warning: ZipArchive::addGlob(): Option "comp_flags" must be between 0 and 65535 in %s on line %d
0: foo.txt, comp=8, enc=0
1: bar.txt, comp=0, enc=259
diff --git a/ext/zip/tests/oo_setcompression.phpt b/ext/zip/tests/oo_setcompression.phpt
index 6b329146365..3d90d1e985c 100644
--- a/ext/zip/tests/oo_setcompression.phpt
+++ b/ext/zip/tests/oo_setcompression.phpt
@@ -28,6 +28,30 @@
var_dump($zip->setCompressionName('dir/entry3.txt', ZipArchive::CM_STORE));
var_dump($zip->setCompressionName('entry4.txt', ZipArchive::CM_DEFLATE));
+try {
+ $zip->setCompressionName('entry5.txt', PHP_INT_MIN);
+} catch (\ValueError $e) {
+ echo $e->getMessage(), PHP_EOL;
+}
+
+try {
+ $zip->setCompressionName('entry5.txt', PHP_INT_MAX);
+} catch (\ValueError $e) {
+ echo $e->getMessage(), PHP_EOL;
+}
+
+try {
+ $zip->setCompressionIndex(4, PHP_INT_MIN);
+} catch (\ValueError $e) {
+ echo $e->getMessage(), PHP_EOL;
+}
+
+try {
+ $zip->setCompressionIndex(4, PHP_INT_MAX);
+} catch (\ValueError $e) {
+ echo $e->getMessage(), PHP_EOL;
+}
+
var_dump($zip->setCompressionIndex(4, ZipArchive::CM_STORE));
var_dump($zip->setCompressionIndex(5, ZipArchive::CM_DEFLATE));
var_dump($zip->setCompressionIndex(6, ZipArchive::CM_DEFAULT));
@@ -57,6 +81,10 @@
bool(true)
bool(true)
bool(true)
+ZipArchive::setCompressionName(): Argument #2 ($method) must be between -1 and %d
+ZipArchive::setCompressionName(): Argument #2 ($method) must be between -1 and %d
+ZipArchive::setCompressionIndex(): Argument #2 ($method) must be between -1 and %d
+ZipArchive::setCompressionIndex(): Argument #2 ($method) must be between -1 and %d
bool(true)
bool(true)
bool(true)
diff --git a/ext/zip/zip_stream.c b/ext/zip/zip_stream.c
index 76e4588f249..50f097de3c8 100644
--- a/ext/zip/zip_stream.c
+++ b/ext/zip/zip_stream.c
@@ -150,7 +150,7 @@ static int php_zip_ops_stat(php_stream *stream, php_stream_statbuf *ssb) /* {{{
fragment++;
if (ZIP_OPENBASEDIR_CHECKPATH(file_dirname)) {
- zend_string_release_ex(file_basename, 0);
+ zend_string_release_ex(file_basename, false);
return -1;
}
@@ -159,7 +159,7 @@ static int php_zip_ops_stat(php_stream *stream, php_stream_statbuf *ssb) /* {{{
memset(ssb, 0, sizeof(php_stream_statbuf));
if (zip_stat(za, fragment, ZIP_FL_NOCASE, &sb) != 0) {
zip_close(za);
- zend_string_release_ex(file_basename, 0);
+ zend_string_release_ex(file_basename, false);
return -1;
}
zip_close(za);
@@ -183,7 +183,7 @@ static int php_zip_ops_stat(php_stream *stream, php_stream_statbuf *ssb) /* {{{
#endif
ssb->sb.st_ino = -1;
}
- zend_string_release_ex(file_basename, 0);
+ zend_string_release_ex(file_basename, false);
return 0;
}
/* }}} */
@@ -312,7 +312,7 @@ php_stream *php_stream_zip_opener(php_stream_wrapper *wrapper,
fragment++;
if (ZIP_OPENBASEDIR_CHECKPATH(file_dirname)) {
- zend_string_release_ex(file_basename, 0);
+ zend_string_release_ex(file_basename, false);
return NULL;
}
@@ -344,14 +344,14 @@ php_stream *php_stream_zip_opener(php_stream_wrapper *wrapper,
}
if (opened_path) {
- *opened_path = zend_string_init(path, strlen(path), 0);
+ *opened_path = zend_string_init(path, path_len, false);
}
} else {
zip_close(za);
}
}
- zend_string_release_ex(file_basename, 0);
+ zend_string_release_ex(file_basename, false);
if (!stream) {
return NULL;