From fd3341846f32281f73125bc4fd2d320730d3feb2 Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Tue, 2 Jun 2026 03:01:56 +1000 Subject: [PATCH 1/6] Add ZipArchive::closeString() (#21497) - Add a $flags parameter to ZipArchive::openString(), by analogy with ZipArchive::open(). This allows the string to be opened read/write. - Have the $data parameter to ZipArchive::openString() default to an empty string, for convenience of callers that want to create an empty archive. This works on all versions of libzip since the change in 1.6.0 only applied to files, it's opt-in for generic sources. - Add ZipArchive::closeString() which closes the archive and returns the resulting string. For consistency with openString(), return an empty string if the archive is empty. --- ext/zip/php_zip.c | 72 +++++++++++++++--- ext/zip/php_zip.h | 2 + ext/zip/php_zip.stub.php | 4 +- ext/zip/php_zip_arginfo.h | 12 ++- .../tests/ZipArchive_closeString_basic.phpt | 25 ++++++ .../tests/ZipArchive_closeString_error.phpt | 47 ++++++++++++ .../tests/ZipArchive_closeString_false.phpt | 22 ++++++ .../ZipArchive_closeString_variation.phpt | 39 ++++++++++ ext/zip/tests/ZipArchive_openString.phpt | 25 +++++- ext/zip/tests/checkcons.zip | Bin 0 -> 181 bytes ext/zip/tests/wrong-file-size.zip | Bin 0 -> 110 bytes ext/zip/zip_source.c | 6 ++ 12 files changed, 238 insertions(+), 16 deletions(-) create mode 100644 ext/zip/tests/ZipArchive_closeString_basic.phpt create mode 100644 ext/zip/tests/ZipArchive_closeString_error.phpt create mode 100644 ext/zip/tests/ZipArchive_closeString_false.phpt create mode 100644 ext/zip/tests/ZipArchive_closeString_variation.phpt create mode 100644 ext/zip/tests/checkcons.zip create mode 100644 ext/zip/tests/wrong-file-size.zip diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index a8289a41a304..cb2389ca4370 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -573,8 +573,12 @@ static char * php_zipobj_get_zip_comment(ze_zip_object *obj, int *len) /* {{{ */ } /* }}} */ -/* Close and free the zip_t */ -static bool php_zipobj_close(ze_zip_object *obj) /* {{{ */ +/* Close and free the zip_t. If the archive was opened as a string, the + * final contents of the archive will be assigned to *out_str and that + * string will afterwards be owned by the caller. + * + * If out_str is NULL, the final string contents, if any, will be discarded. */ +static bool php_zipobj_close(ze_zip_object *obj, zend_string **out_str) /* {{{ */ { struct zip *intern = obj->za; bool success = false; @@ -606,7 +610,19 @@ static bool php_zipobj_close(ze_zip_object *obj) /* {{{ */ obj->filename_len = 0; } + if (obj->out_str) { + if (out_str) { + *out_str = obj->out_str; + } else { + zend_string_release(obj->out_str); + } + obj->out_str = NULL; + } else { + ZEND_ASSERT(!out_str); + } + obj->za = NULL; + obj->from_string = false; return success; } /* }}} */ @@ -1060,7 +1076,7 @@ static void php_zip_object_free_storage(zend_object *object) /* {{{ */ { ze_zip_object * intern = php_zip_fetch_object(object); - php_zipobj_close(intern); + php_zipobj_close(intern, NULL); #ifdef HAVE_PROGRESS_CALLBACK /* if not properly called by libzip */ @@ -1467,7 +1483,7 @@ PHP_METHOD(ZipArchive, open) } /* If we already have an opened zip, free it */ - php_zipobj_close(ze_obj); + php_zipobj_close(ze_obj, NULL); /* open for write without option to empty the archive */ if ((flags & (ZIP_TRUNCATE | ZIP_RDONLY)) == 0) { @@ -1491,28 +1507,34 @@ PHP_METHOD(ZipArchive, open) ze_obj->filename = resolved_path; ze_obj->filename_len = strlen(resolved_path); ze_obj->za = intern; + ze_obj->from_string = false; RETURN_TRUE; } /* }}} */ -/* {{{ Create new read-only zip using given string */ +/* {{{ Create new zip from a string, or a create an empty zip to be saved to a string */ PHP_METHOD(ZipArchive, openString) { - zend_string *buffer; + zend_string *buffer = NULL; + zend_long flags = 0; zval *self = ZEND_THIS; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "S", &buffer) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "|Sl", &buffer, &flags) == FAILURE) { RETURN_THROWS(); } + if (!buffer) { + buffer = ZSTR_EMPTY_ALLOC(); + } + ze_zip_object *ze_obj = Z_ZIP_P(self); - php_zipobj_close(ze_obj); + php_zipobj_close(ze_obj, NULL); zip_error_t err; zip_error_init(&err); - zip_source_t * zip_source = php_zip_create_string_source(buffer, NULL, &err); + zip_source_t * zip_source = php_zip_create_string_source(buffer, &ze_obj->out_str, &err); if (!zip_source) { ze_obj->err_zip = zip_error_code_zip(&err); @@ -1521,7 +1543,7 @@ PHP_METHOD(ZipArchive, openString) RETURN_LONG(ze_obj->err_zip); } - struct zip *intern = zip_open_from_source(zip_source, ZIP_RDONLY, &err); + struct zip *intern = zip_open_from_source(zip_source, flags, &err); if (!intern) { ze_obj->err_zip = zip_error_code_zip(&err); ze_obj->err_sys = zip_error_code_system(&err); @@ -1530,6 +1552,7 @@ PHP_METHOD(ZipArchive, openString) RETURN_LONG(ze_obj->err_zip); } + ze_obj->from_string = true; ze_obj->za = intern; zip_error_fini(&err); RETURN_TRUE; @@ -1568,7 +1591,34 @@ PHP_METHOD(ZipArchive, close) ZIP_FROM_OBJECT(intern, self); - RETURN_BOOL(php_zipobj_close(Z_ZIP_P(self))); + RETURN_BOOL(php_zipobj_close(Z_ZIP_P(self), NULL)); +} +/* }}} */ + +/* {{{ close the zip archive and get the result as a string */ +PHP_METHOD(ZipArchive, closeString) +{ + struct zip *intern; + zval *self = ZEND_THIS; + + ZEND_PARSE_PARAMETERS_NONE(); + + ZIP_FROM_OBJECT(intern, self); + + if (!Z_ZIP_P(self)->from_string) { + zend_throw_error(NULL, "ZipArchive::closeString can only be called on " + "an archive opened with ZipArchive::openString"); + RETURN_THROWS(); + } + + zend_string *ret = NULL; + bool success = php_zipobj_close(Z_ZIP_P(self), &ret); + ZEND_ASSERT(ret); + if (success) { + RETURN_STR(ret); + } + zend_string_release(ret); + RETURN_FALSE; } /* }}} */ diff --git a/ext/zip/php_zip.h b/ext/zip/php_zip.h index 74c776ddb3db..9e57a54de1cc 100644 --- a/ext/zip/php_zip.h +++ b/ext/zip/php_zip.h @@ -69,6 +69,8 @@ typedef struct _ze_zip_object { HashTable *prop_handler; char *filename; size_t filename_len; + zend_string *out_str; + bool from_string; zip_int64_t last_id; int err_zip; int err_sys; diff --git a/ext/zip/php_zip.stub.php b/ext/zip/php_zip.stub.php index 19ea67e07fba..49dd19e53553 100644 --- a/ext/zip/php_zip.stub.php +++ b/ext/zip/php_zip.stub.php @@ -646,7 +646,7 @@ class ZipArchive implements Countable /** @tentative-return-type */ public function open(string $filename, int $flags = 0): bool|int {} - public function openString(string $data): bool|int {} + public function openString(string $data = '', int $flags = 0): bool|int {} /** * @tentative-return-type @@ -656,6 +656,8 @@ public function setPassword(#[\SensitiveParameter] string $password): bool {} /** @tentative-return-type */ public function close(): bool {} + public function closeString(): string|false {} + /** @tentative-return-type */ public function count(): int {} diff --git a/ext/zip/php_zip_arginfo.h b/ext/zip/php_zip_arginfo.h index ae2569400efe..faa6feb1cb1e 100644 --- a/ext/zip/php_zip_arginfo.h +++ b/ext/zip/php_zip_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit php_zip.stub.php instead. - * Stub hash: bf6706496639628a3287d0026f68f57ecebc4a55 */ + * Stub hash: d623efdfe5ac46f07aebf8fb120050c818f3d793 */ ZEND_BEGIN_ARG_INFO_EX(arginfo_zip_open, 0, 0, 1) ZEND_ARG_TYPE_INFO(0, filename, IS_STRING, 0) @@ -45,8 +45,9 @@ ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_MASK_EX(arginfo_class_ZipArchive_open, ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, flags, IS_LONG, 0, "0") ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_class_ZipArchive_openString, 0, 1, MAY_BE_BOOL|MAY_BE_LONG) - ZEND_ARG_TYPE_INFO(0, data, IS_STRING, 0) +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_class_ZipArchive_openString, 0, 0, MAY_BE_BOOL|MAY_BE_LONG) + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, data, IS_STRING, 0, "\'\'") + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, flags, IS_LONG, 0, "0") ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_ZipArchive_setPassword, 0, 1, _IS_BOOL, 0) @@ -56,6 +57,9 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_ZipArchive_close, 0, 0, _IS_BOOL, 0) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_class_ZipArchive_closeString, 0, 0, MAY_BE_STRING|MAY_BE_FALSE) +ZEND_END_ARG_INFO() + ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_ZipArchive_count, 0, 0, IS_LONG, 0) ZEND_END_ARG_INFO() @@ -320,6 +324,7 @@ ZEND_METHOD(ZipArchive, open); ZEND_METHOD(ZipArchive, openString); ZEND_METHOD(ZipArchive, setPassword); ZEND_METHOD(ZipArchive, close); +ZEND_METHOD(ZipArchive, closeString); ZEND_METHOD(ZipArchive, count); ZEND_METHOD(ZipArchive, getStatusString); ZEND_METHOD(ZipArchive, clearError); @@ -399,6 +404,7 @@ static const zend_function_entry class_ZipArchive_methods[] = { ZEND_ME(ZipArchive, openString, arginfo_class_ZipArchive_openString, ZEND_ACC_PUBLIC) ZEND_ME(ZipArchive, setPassword, arginfo_class_ZipArchive_setPassword, ZEND_ACC_PUBLIC) ZEND_ME(ZipArchive, close, arginfo_class_ZipArchive_close, ZEND_ACC_PUBLIC) + ZEND_ME(ZipArchive, closeString, arginfo_class_ZipArchive_closeString, ZEND_ACC_PUBLIC) ZEND_ME(ZipArchive, count, arginfo_class_ZipArchive_count, ZEND_ACC_PUBLIC) ZEND_ME(ZipArchive, getStatusString, arginfo_class_ZipArchive_getStatusString, ZEND_ACC_PUBLIC) ZEND_ME(ZipArchive, clearError, arginfo_class_ZipArchive_clearError, ZEND_ACC_PUBLIC) diff --git a/ext/zip/tests/ZipArchive_closeString_basic.phpt b/ext/zip/tests/ZipArchive_closeString_basic.phpt new file mode 100644 index 000000000000..852a7ebf53f3 --- /dev/null +++ b/ext/zip/tests/ZipArchive_closeString_basic.phpt @@ -0,0 +1,25 @@ +--TEST-- +ZipArchive::closeString() basic +--EXTENSIONS-- +zip +--FILE-- +openString(); +$zip->addFromString('test1', '1'); +$zip->addFromString('test2', '2'); +$contents = $zip->closeString(); +echo $contents ? "OK\n" : "FAILED\n"; + +$zip = new ZipArchive(); +$zip->openString($contents); +var_dump($zip->getFromName('test1')); +var_dump($zip->getFromName('test2')); +var_dump($zip->getFromName('nonexistent')); + +?> +--EXPECT-- +OK +string(1) "1" +string(1) "2" +bool(false) diff --git a/ext/zip/tests/ZipArchive_closeString_error.phpt b/ext/zip/tests/ZipArchive_closeString_error.phpt new file mode 100644 index 000000000000..d5dca14a97fc --- /dev/null +++ b/ext/zip/tests/ZipArchive_closeString_error.phpt @@ -0,0 +1,47 @@ +--TEST-- +ZipArchive::closeString() error cases +--EXTENSIONS-- +zip +--FILE-- +openString(); +var_dump($zip->open(__DIR__ . '/test.zip')); +try { + $zip->closeString(); +} catch (Error $e) { + echo $e->getMessage() . "\n"; +} + +echo "2.\n"; +$zip = new ZipArchive(); +$zip->openString('...'); +echo $zip->getStatusString() . "\n"; +try { + $zip->closeString(); +} catch (Error $e) { + echo $e->getMessage() . "\n"; +} + +echo "3.\n"; +$zip = new ZipArchive(); +$zip->openString(file_get_contents(__DIR__ . '/test.zip')); +echo gettype($zip->closeString()) . "\n"; +try { + $zip->closeString(); +} catch (Error $e) { + echo $e->getMessage() . "\n"; +} + +?> +--EXPECT-- +1. +bool(true) +ZipArchive::closeString can only be called on an archive opened with ZipArchive::openString +2. +Not a zip archive +Invalid or uninitialized Zip object +3. +string +Invalid or uninitialized Zip object diff --git a/ext/zip/tests/ZipArchive_closeString_false.phpt b/ext/zip/tests/ZipArchive_closeString_false.phpt new file mode 100644 index 000000000000..8a7d942527f6 --- /dev/null +++ b/ext/zip/tests/ZipArchive_closeString_false.phpt @@ -0,0 +1,22 @@ +--TEST-- +ZipArchive::closeString() false return +--EXTENSIONS-- +zip +--FILE-- +openString($input)); +$zip->setCompressionIndex(0, ZipArchive::CM_DEFLATE); +var_dump($zip->closeString()); +echo $zip->getStatusString() . "\n"; +?> +--EXPECTREGEX-- +bool\(true\) + +Warning: ZipArchive::closeString\(\): (Zip archive inconsistent|Unexpected length of data).* +bool\(false\) +(Zip archive inconsistent|Unexpected length of data) diff --git a/ext/zip/tests/ZipArchive_closeString_variation.phpt b/ext/zip/tests/ZipArchive_closeString_variation.phpt new file mode 100644 index 000000000000..a1922ebd8f1f --- /dev/null +++ b/ext/zip/tests/ZipArchive_closeString_variation.phpt @@ -0,0 +1,39 @@ +--TEST-- +ZipArchive::closeString() variations +--EXTENSIONS-- +zip +--FILE-- +openString(); +var_dump($zip->closeString()); +echo $zip->getStatusString() . "\n"; + +echo "2. Update existing archive\n"; +$input = file_get_contents(__DIR__ . '/test.zip'); +$zip = new ZipArchive(); +$zip->openString($input); +$zip->addFromString('entry1.txt', ''); +$result = $zip->closeString(); +echo gettype($result) . "\n"; +var_dump($input !== $result); + +echo "3. Unchanged existing archive\n"; +$zip = new ZipArchive(); +$zip->openString($input); +$result = $zip->closeString(); +echo gettype($result) . "\n"; +var_dump($input === $result); + +?> +--EXPECT-- +1. Empty archive creation +string(0) "" +No error +2. Update existing archive +string +bool(true) +3. Unchanged existing archive +string +bool(true) diff --git a/ext/zip/tests/ZipArchive_openString.phpt b/ext/zip/tests/ZipArchive_openString.phpt index f787b4a84933..2a11505fcf69 100644 --- a/ext/zip/tests/ZipArchive_openString.phpt +++ b/ext/zip/tests/ZipArchive_openString.phpt @@ -4,8 +4,10 @@ ZipArchive::openString() method zip --FILE-- openString(file_get_contents(__DIR__."/test_procedural.zip")); +$zip->openString($input, ZipArchive::RDONLY); for ($i = 0; $i < $zip->numFiles; $i++) { $stat = $zip->statIndex($i); @@ -17,8 +19,22 @@ var_dump($zip->addFromString("foobar/baz", "baz")); var_dump($zip->addEmptyDir("blub")); var_dump($zip->close()); + +echo "2. CREATE and EXCL flags\n"; +$zip = new ZipArchive(); +var_dump($zip->openString($input, ZipArchive::CREATE)); +var_dump($zip->openString($input, ZipArchive::EXCL)); +echo $zip->getStatusString() . "\n"; + +echo "3. CHECKCONS flag\n"; +$inconsistent = file_get_contents(__DIR__ . '/checkcons.zip'); +$zip = new ZipArchive(); +var_dump($zip->openString($inconsistent)); +var_dump($zip->openString($inconsistent, ZipArchive::CHECKCONS)); + ?> --EXPECTF-- +1. Open read-only and read directory foo bar foobar/ @@ -26,3 +42,10 @@ foobar/baz bool(false) bool(false) bool(true) +2. CREATE and EXCL flags +bool(true) +int(10) +File already exists +3. CHECKCONS flag +bool(true) +int(%d) diff --git a/ext/zip/tests/checkcons.zip b/ext/zip/tests/checkcons.zip new file mode 100644 index 0000000000000000000000000000000000000000..50bcea128a46a40417700a9594c9cdbecb38a55e GIT binary patch literal 181 zcmWIWW@h1HfB=21$!yn+7=auR=4Oy#aL!3AF4jv1k)a`+49pi6C0c=SX$3a}Bg+eB z1_m&}72wUtB*%=)90{=TATyUVf|w|#vO-KnGcmxMl?|kn5eNf;v>S-S5K@$wnWLwt F2LP)EAYK3f literal 0 HcmV?d00001 diff --git a/ext/zip/tests/wrong-file-size.zip b/ext/zip/tests/wrong-file-size.zip new file mode 100644 index 0000000000000000000000000000000000000000..fc9fa1a434c7ddb3188117f5b125cc3cef92e56b GIT binary patch literal 110 zcmWIWW@Zs#0DP1W^zjtZX1Q NBM=$^X*Cds0RYQL62Jfe literal 0 HcmV?d00001 diff --git a/ext/zip/zip_source.c b/ext/zip/zip_source.c index 48b35862b903..03fa50867d56 100644 --- a/ext/zip/zip_source.c +++ b/ext/zip/zip_source.c @@ -155,6 +155,7 @@ static zip_int64_t php_zip_string_cb(void *userdata, void *data, zip_uint64_t le ctx->in_str = ctx->out_str; ctx->out_str = ZSTR_EMPTY_ALLOC(); if (ctx->dest) { + zend_string_release(*(ctx->dest)); *(ctx->dest) = zend_string_copy(ctx->in_str); } return 0; @@ -200,5 +201,10 @@ zip_source_t * php_zip_create_string_source(zend_string *str, zend_string **dest ctx->out_str = ZSTR_EMPTY_ALLOC(); ctx->dest = dest; ctx->mtime = time(NULL); + + if (dest) { + *dest = zend_string_copy(str); + } + return zip_source_function_create(php_zip_string_cb, (void*)ctx, err); } From 56d1ffca58543a59fcef21edc4d9157110db8ef1 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Mon, 1 Jun 2026 10:04:02 -0700 Subject: [PATCH 2/6] UPGRADING: document `ZipArchive::closeString()` addition Follow-up to #21497 [skip ci] --- UPGRADING | 1 + 1 file changed, 1 insertion(+) diff --git a/UPGRADING b/UPGRADING index c996f963a34f..2a32c2a5a363 100644 --- a/UPGRADING +++ b/UPGRADING @@ -318,6 +318,7 @@ PHP 8.6 UPGRADE NOTES - Zip: . Added ZipArchive::openString() method. + . Added ZipArchive::closeString() method. ======================================== 7. New Classes and Interfaces From 4c8dabf698785644e8bc602633831485899687cb Mon Sep 17 00:00:00 2001 From: Nora Dossche <7771979+ndossche@users.noreply.github.com> Date: Mon, 1 Jun 2026 20:21:41 +0200 Subject: [PATCH 3/6] Fix -Werror compile error in `zend_dval_to_lval_cap()` (#22196) This makes imagick CI fail on PHP 8.5: ``` /usr/include/php/20250925/Zend/zend_operators.h: In function 'zend_dval_to_lval_cap': /usr/include/php/20250925/Zend/zend_operators.h:149:88: error: unused parameter 's' [-Werror=unused-parameter] 149 | static zend_always_inline zend_long zend_dval_to_lval_cap(double d, const zend_string *s) | ~~~~~~~~~~~~~~~~~~~^ cc1: all warnings being treated as errors ``` --- Zend/zend_operators.h | 1 + 1 file changed, 1 insertion(+) diff --git a/Zend/zend_operators.h b/Zend/zend_operators.h index 57bcdd8d6ae4..5478e164c178 100644 --- a/Zend/zend_operators.h +++ b/Zend/zend_operators.h @@ -148,6 +148,7 @@ static zend_always_inline zend_long zend_dval_to_lval_silent(double d) /* Used to convert a string float to integer during an (int) cast */ static zend_always_inline zend_long zend_dval_to_lval_cap(double d, const zend_string *s) { + ZEND_IGNORE_VALUE(s); if (UNEXPECTED(!zend_finite(d))) { return 0; } else if (!ZEND_DOUBLE_FITS_LONG(d)) { From 8172b7e5073d816e7831758d9b256f829327574f Mon Sep 17 00:00:00 2001 From: ndossche <7771979+ndossche@users.noreply.github.com> Date: Mon, 1 Jun 2026 20:41:49 +0200 Subject: [PATCH 4/6] Fix merge --- Zend/zend_operators.h | 1 - 1 file changed, 1 deletion(-) diff --git a/Zend/zend_operators.h b/Zend/zend_operators.h index 530fb22806a9..2dc6dbfbfc6d 100644 --- a/Zend/zend_operators.h +++ b/Zend/zend_operators.h @@ -147,7 +147,6 @@ static zend_always_inline zend_long zend_dval_to_lval_silent(double d) /* Used to convert a string float to integer during an (int) cast */ static zend_always_inline zend_long zend_dval_to_lval_cap(double d) { - ZEND_IGNORE_VALUE(s); if (UNEXPECTED(!zend_finite(d))) { return 0; } else if (!ZEND_DOUBLE_FITS_LONG(d)) { From 7de451fc3a5811cad4639c5eca36218e40832ce2 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Mon, 1 Jun 2026 15:28:20 -0400 Subject: [PATCH 5/6] uri: Do not copy and normalize already-normalized URIs for uri_parser_rfc3986 (#21726) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Uri\Rfc3986\Uri::parse() produces a URI already in canonical form (the common case: http/https URLs with no uppercase host, no percent-encoding in unreserved ranges, no ".." path segments), get_normalized_uri() no longer deep-copies the parsed struct and runs a full normalization pass. It calls uriNormalizeSyntaxMaskRequiredExA once to compute the dirty mask; a zero mask means we alias the raw uri. The struct caches the dirty mask, so multiple non-raw reads on the same instance only run the scan once. Fallback: when the mask is nonzero, we copy and normalize as before, but only for the flagged components (uriNormalizeSyntaxExMmA(..., dirty_mask, ...) instead of (..., -1, ...)). Measurements on a 17-URL mix with a realistic parse-and-read workload (10 runs of 1.7M parses each, CPU pinned via taskset, same-session stash-pop A/B so both builds share machine state): baseline mean optimized mean delta parse only 0.3992s (4.26M/s) 0.4083s (4.16M/s) noise parse + 1 read 0.6687s (2.54M/s) 0.5464s (3.11M/s) -18.3% parse + 7 reads 0.8510s (2.00M/s) 0.7305s (2.33M/s) -14.2% The "parse + 1 read" row isolates the first-read cost where this change lands. The "parse + 7 reads" row shows the amortized effect under a realistic user pattern: the first getter pays the reduced normalization cost, and the remaining six getters hit the cached normalized uri and cost the same as before. hyperfine cross-check on the whole benchmark script, 15 runs each: baseline 20.471 s +/- 1.052 s [19.535 .. 22.985] optimized 17.240 s +/- 0.540 s [16.556 .. 18.190] optimized runs 1.19 +/- 0.07 times faster. All 309 tests in ext/uri/tests pass. I checked that URIs needing normalization (http://EXAMPLE.com/A/%2e%2e/c resolving to /c) still hit the full normalize path through the nonzero dirty mask. Co-authored-by: Tim Düsterhus --- UPGRADING | 5 ++++- ext/uri/uri_parser_rfc3986.c | 16 +++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/UPGRADING b/UPGRADING index 2a32c2a5a363..95299dd5117f 100644 --- a/UPGRADING +++ b/UPGRADING @@ -448,7 +448,10 @@ PHP 8.6 UPGRADE NOTES . Improved performance of str_split(). - URI: - . Reduced allocations when reading RFC3986 IPv6/IPFuture hosts and paths. + . Reduced allocations when reading IPv6/IPFuture hosts and paths with + Uri\Rfc3986\Uri. + . Improved performance and memory consumption when using normalizing + (non-raw) getters on already-normalized URIs with Uri\Rfc3986\Uri. - Zip: . Avoid string copies in ZipArchive::addFromString(). diff --git a/ext/uri/uri_parser_rfc3986.c b/ext/uri/uri_parser_rfc3986.c index ad47aa1946c7..4e2c5656aa77 100644 --- a/ext/uri/uri_parser_rfc3986.c +++ b/ext/uri/uri_parser_rfc3986.c @@ -24,6 +24,7 @@ struct php_uri_parser_rfc3986_uris { UriUriA uri; UriUriA normalized_uri; + unsigned int normalization_mask; bool normalized_uri_initialized; }; @@ -84,12 +85,21 @@ ZEND_ATTRIBUTE_NONNULL static void copy_uri(UriUriA *new_uriparser_uri, const Ur ZEND_ATTRIBUTE_NONNULL static UriUriA *get_normalized_uri(php_uri_parser_rfc3986_uris *uriparser_uris) { if (!uriparser_uris->normalized_uri_initialized) { - copy_uri(&uriparser_uris->normalized_uri, &uriparser_uris->uri); - int result = uriNormalizeSyntaxExMmA(&uriparser_uris->normalized_uri, (unsigned int)-1, mm); - ZEND_ASSERT(result == URI_SUCCESS); + int mask_result = uriNormalizeSyntaxMaskRequiredExA(&uriparser_uris->uri, &uriparser_uris->normalization_mask); + ZEND_ASSERT(mask_result == URI_SUCCESS); + + if (uriparser_uris->normalization_mask != URI_NORMALIZED) { + copy_uri(&uriparser_uris->normalized_uri, &uriparser_uris->uri); + int result = uriNormalizeSyntaxExMmA(&uriparser_uris->normalized_uri, uriparser_uris->normalization_mask, mm); + ZEND_ASSERT(result == URI_SUCCESS); + } uriparser_uris->normalized_uri_initialized = true; } + if (uriparser_uris->normalization_mask == URI_NORMALIZED) { + return &uriparser_uris->uri; + } + return &uriparser_uris->normalized_uri; } From a22c56c969f1fc34bfc9195dc2cfc70a15186148 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Mon, 1 Jun 2026 18:01:09 -0300 Subject: [PATCH 6/6] Add `error_include_args` INI option to display function args in docref (#12276) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Displays arguments for errors from built-in PHP functions using docref API. RFC: https://wiki.php.net/rfc/display_error_function_args Co-authored-by: Tim Düsterhus --- Zend/tests/display_error_function_args.phpt | 30 +++++++ Zend/zend_exceptions.c | 87 ++++++++++++++------- Zend/zend_exceptions.h | 2 + ext/openssl/tests/ServerClientTestCase.inc | 3 +- main/main.c | 11 ++- main/php_globals.h | 1 + php.ini-development | 6 ++ php.ini-production | 6 ++ run-tests.php | 1 + sapi/cli/tests/php_cli_server.inc | 3 +- 10 files changed, 120 insertions(+), 30 deletions(-) create mode 100644 Zend/tests/display_error_function_args.phpt diff --git a/Zend/tests/display_error_function_args.phpt b/Zend/tests/display_error_function_args.phpt new file mode 100644 index 000000000000..c28a4a2808b4 --- /dev/null +++ b/Zend/tests/display_error_function_args.phpt @@ -0,0 +1,30 @@ +--TEST-- +Displaying function arguments in errors +--INI-- +error_include_args=On +--FILE-- + "123456789012345678901" . chr(0), "cost" => 4]; +password_hash("test", PASSWORD_BCRYPT, $flags); + +ini_set("error_include_args", "Off"); + +unlink('/'); +password_hash("test", PASSWORD_BCRYPT, $flags); + +?> +--EXPECTF-- +Warning: unlink('/'): %s in %s on line %d + +Warning: password_hash(Object(SensitiveParameterValue), '2y', Array): The "salt" option has been ignored, since providing a custom salt is no longer supported in %s on line %d + +Warning: unlink(/): %s in %s on line %d + +Warning: password_hash(): The "salt" option has been ignored, since providing a custom salt is no longer supported in %s on line %d diff --git a/Zend/zend_exceptions.c b/Zend/zend_exceptions.c index d23fb647af9d..a1301b8c20b2 100644 --- a/Zend/zend_exceptions.c +++ b/Zend/zend_exceptions.c @@ -506,7 +506,7 @@ ZEND_METHOD(ErrorException, getSeverity) } \ } while (0) -static void _build_trace_args(zval *arg, smart_str *str) /* {{{ */ +static void build_trace_args(zval *arg, smart_str *str) /* {{{ */ { /* the trivial way would be to do * convert_to_string(arg); @@ -516,24 +516,21 @@ static void _build_trace_args(zval *arg, smart_str *str) /* {{{ */ ZVAL_DEREF(arg); - if (smart_str_append_zval(str, arg, EG(exception_string_param_max_len)) == SUCCESS) { - smart_str_appends(str, ", "); - } else { + if (smart_str_append_zval(str, arg, EG(exception_string_param_max_len)) != SUCCESS) { switch (Z_TYPE_P(arg)) { case IS_RESOURCE: smart_str_appends(str, "Resource id #"); smart_str_append_long(str, Z_RES_HANDLE_P(arg)); - smart_str_appends(str, ", "); break; case IS_ARRAY: - smart_str_appends(str, "Array, "); + smart_str_appends(str, "Array"); break; case IS_OBJECT: { zend_string *class_name = Z_OBJ_HANDLER_P(arg, get_class_name)(Z_OBJ_P(arg)); smart_str_appends(str, "Object("); /* cut off on NULL byte ... class@anonymous */ smart_str_appends(str, ZSTR_VAL(class_name)); - smart_str_appends(str, "), "); + smart_str_appends(str, ")"); zend_string_release_ex(class_name, 0); break; } @@ -542,7 +539,30 @@ static void _build_trace_args(zval *arg, smart_str *str) /* {{{ */ } /* }}} */ -static void _build_trace_string(smart_str *str, const HashTable *ht, uint32_t num) /* {{{ */ +static void build_trace_args_list(zval *tmp, smart_str *str) /* {{{ */ +{ + if (UNEXPECTED(Z_TYPE_P(tmp) != IS_ARRAY)) { + /* only happens w/ reflection abuse (Zend/tests/bug63762.phpt) */ + zend_error(E_WARNING, "args element is not an array"); + return; + } + + bool first = true; + ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(tmp), zend_string *name, zval *arg) { + if (!first) { + smart_str_appends(str, ", "); + } + first = false; + if (name) { + smart_str_append(str, name); + smart_str_appends(str, ": "); + } + build_trace_args(arg, str); + } ZEND_HASH_FOREACH_END(); +} +/* }}} */ + +static void build_trace_string(smart_str *str, const HashTable *ht, uint32_t num) /* {{{ */ { zval *file, *tmp; @@ -588,27 +608,40 @@ static void _build_trace_string(smart_str *str, const HashTable *ht, uint32_t nu smart_str_appendc(str, '('); tmp = zend_hash_find_known_hash(ht, ZSTR_KNOWN(ZEND_STR_ARGS)); if (tmp) { - if (EXPECTED(Z_TYPE_P(tmp) == IS_ARRAY)) { - size_t last_len = ZSTR_LEN(str->s); - zend_string *name; - zval *arg; - - ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(tmp), name, arg) { - if (name) { - smart_str_append(str, name); - smart_str_appends(str, ": "); - } - _build_trace_args(arg, str); - } ZEND_HASH_FOREACH_END(); + build_trace_args_list(tmp, str); + } + smart_str_appends(str, ")\n"); +} +/* }}} */ - if (last_len != ZSTR_LEN(str->s)) { - ZSTR_LEN(str->s) -= 2; /* remove last ', ' */ - } - } else { - zend_error(E_WARNING, "args element is not an array"); +/* {{{ Gets the function arguments printed as a string from a backtrace frame. */ +ZEND_API zend_string *zend_trace_function_args_to_string(const HashTable *frame) { + smart_str str = {0}; + + zval *tmp = zend_hash_find_known_hash(frame, ZSTR_KNOWN(ZEND_STR_ARGS)); + if (tmp) { + build_trace_args_list(tmp, &str); + } + + return smart_str_extract(&str); +} +/* }}} */ + +/* {{{ Gets the currently executing function's arguments as a string. Used by php_verror. */ +ZEND_API zend_string *zend_trace_current_function_args_string(void) { + zend_string *dynamic_params = NULL; + /* get a backtrace to snarf function args */ + zval backtrace; + zend_fetch_debug_backtrace(&backtrace, /* skip_last */ 0, /* options */ 0, /* limit */ 1); + /* can fail esp if low memory condition */ + if (Z_TYPE(backtrace) == IS_ARRAY) { + zval *first_frame = zend_hash_index_find(Z_ARRVAL(backtrace), 0); + if (first_frame) { + dynamic_params = zend_trace_function_args_to_string(Z_ARRVAL_P(first_frame)); } } - smart_str_appends(str, ")\n"); + zval_ptr_dtor(&backtrace); + return dynamic_params; } /* }}} */ @@ -624,7 +657,7 @@ ZEND_API zend_string *zend_trace_to_string(const HashTable *trace, bool include_ continue; } - _build_trace_string(&str, Z_ARRVAL_P(frame), num++); + build_trace_string(&str, Z_ARRVAL_P(frame), num++); } ZEND_HASH_FOREACH_END(); if (include_main) { diff --git a/Zend/zend_exceptions.h b/Zend/zend_exceptions.h index f9b472598012..7ef9ef016393 100644 --- a/Zend/zend_exceptions.h +++ b/Zend/zend_exceptions.h @@ -65,6 +65,8 @@ ZEND_API zend_result zend_update_exception_properties(zend_execute_data *execute /* show an exception using zend_error(severity,...), severity should be E_ERROR */ ZEND_API ZEND_COLD zend_result zend_exception_error(zend_object *exception, int severity); ZEND_NORETURN void zend_exception_uncaught_error(const char *prefix, ...) ZEND_ATTRIBUTE_FORMAT(printf, 1, 2); +ZEND_API zend_string *zend_trace_function_args_to_string(const HashTable *frame); +ZEND_API zend_string *zend_trace_current_function_args_string(void); ZEND_API zend_string *zend_trace_to_string(const HashTable *trace, bool include_main); ZEND_API ZEND_COLD zend_object *zend_create_unwind_exit(void); diff --git a/ext/openssl/tests/ServerClientTestCase.inc b/ext/openssl/tests/ServerClientTestCase.inc index f0336fdd3921..c5db41d48417 100644 --- a/ext/openssl/tests/ServerClientTestCase.inc +++ b/ext/openssl/tests/ServerClientTestCase.inc @@ -100,7 +100,8 @@ class ServerClientTestCase $ini = php_ini_loaded_file(); $cmd = sprintf( '%s %s "%s" %s', - PHP_BINARY, $ini ? "-n -c $ini" : "", + // XXX: TEST_PHP_EXTRA_ARGS for run-test values won't work here? + PHP_BINARY, $ini ? "-n -c $ini -d error_include_args=0" : "", __FILE__, WORKER_ARGV_VALUE ); diff --git a/main/main.c b/main/main.c index cc3f1cae2586..6bda55ac8746 100644 --- a/main/main.c +++ b/main/main.c @@ -62,6 +62,7 @@ #include "win32/php_registry.h" #include "ext/standard/flock_compat.h" #endif +#include "Zend/zend_builtin_functions.h" #include "Zend/zend_exceptions.h" #if PHP_SIGCHILD @@ -801,6 +802,7 @@ PHP_INI_BEGIN() STD_PHP_INI_ENTRY_EX("display_errors", "1", PHP_INI_ALL, OnUpdateDisplayErrors, display_errors, php_core_globals, core_globals, display_errors_mode) STD_PHP_INI_BOOLEAN("display_startup_errors", "1", PHP_INI_ALL, OnUpdateBool, display_startup_errors, php_core_globals, core_globals) STD_PHP_INI_BOOLEAN("enable_dl", "1", PHP_INI_SYSTEM, OnUpdateBool, enable_dl, php_core_globals, core_globals) + STD_PHP_INI_BOOLEAN("error_include_args", "0", PHP_INI_ALL, OnUpdateBool, error_include_args, php_core_globals, core_globals) STD_PHP_INI_BOOLEAN("expose_php", "1", PHP_INI_SYSTEM, OnUpdateBool, expose_php, php_core_globals, core_globals) STD_PHP_INI_ENTRY("docref_root", "", PHP_INI_ALL, OnUpdateString, docref_root, php_core_globals, core_globals) STD_PHP_INI_ENTRY("docref_ext", "", PHP_INI_ALL, OnUpdateString, docref_ext, php_core_globals, core_globals) @@ -1132,7 +1134,14 @@ PHPAPI ZEND_COLD void php_verror(const char *docref, const char *params, int typ /* if we still have memory then format the origin */ if (is_function) { - origin_len = spprintf(&origin, 0, "%s%s%s(%s)", class_name, space, function, params); + zend_string *dynamic_params = NULL; + if (PG(error_include_args)) { + dynamic_params = zend_trace_current_function_args_string(); + } + origin_len = spprintf(&origin, 0, "%s%s%s(%s)", class_name, space, function, dynamic_params ? ZSTR_VAL(dynamic_params) : params); + if (dynamic_params) { + zend_string_release(dynamic_params); + } } else { origin_len = strlen(function); origin = estrndup(function, origin_len); diff --git a/main/php_globals.h b/main/php_globals.h index f6f57e0045c8..8a032e9edb13 100644 --- a/main/php_globals.h +++ b/main/php_globals.h @@ -59,6 +59,7 @@ struct _php_core_globals { uint8_t display_errors; bool display_startup_errors; + bool error_include_args; bool log_errors; bool ignore_repeated_errors; bool ignore_repeated_source; diff --git a/php.ini-development b/php.ini-development index 78ae50708d5a..afabe74ba0e4 100644 --- a/php.ini-development +++ b/php.ini-development @@ -611,6 +611,12 @@ ignore_repeated_source = Off ; Production Value: On ;fatal_error_backtraces = On +; This directive controls whether PHP will print the actual arguments of a +; function upon an error. If this is off (or there was an error fetching the +; arguments), the function providing the error may optionally provide some +; additional information after the problem function's name. +;error_include_args = Off + ;;;;;;;;;;;;;;;;; ; Data Handling ; ;;;;;;;;;;;;;;;;; diff --git a/php.ini-production b/php.ini-production index eb6880fe75d6..04a7b699dadd 100644 --- a/php.ini-production +++ b/php.ini-production @@ -613,6 +613,12 @@ ignore_repeated_source = Off ; Production Value: On ;fatal_error_backtraces = On +; This directive controls whether PHP will print the actual arguments of a +; function upon an error. If this is off (or there was an error fetching the +; arguments), the function providing the error may optionally provide some +; additional information after the problem function's name. +;error_include_args = Off + ;;;;;;;;;;;;;;;;; ; Data Handling ; ;;;;;;;;;;;;;;;;; diff --git a/run-tests.php b/run-tests.php index c08d07cdd7c1..f5c7be8b4f45 100755 --- a/run-tests.php +++ b/run-tests.php @@ -273,6 +273,7 @@ function main(): void 'fatal_error_backtraces=Off', 'display_errors=1', 'display_startup_errors=1', + 'error_include_args=0', 'log_errors=0', 'html_errors=0', 'track_errors=0', diff --git a/sapi/cli/tests/php_cli_server.inc b/sapi/cli/tests/php_cli_server.inc index 3022022f894e..3ad6ced5cb44 100644 --- a/sapi/cli/tests/php_cli_server.inc +++ b/sapi/cli/tests/php_cli_server.inc @@ -24,7 +24,8 @@ function php_cli_server_start( file_put_contents($doc_root . '/' . ($router ?: 'index.php'), ''); } - $cmd = [$php_executable, '-t', $doc_root, '-n', ...$cmd_args, '-S', 'localhost:0']; + // XXX: This should ideally use the same INI overrides as run-tests + $cmd = [$php_executable, '-d', 'error_include_args=0', '-t', $doc_root, '-n', ...$cmd_args, '-S', 'localhost:0']; if (!is_null($router)) { $cmd[] = $router; }