From 45a50d0d6889274d1ad4f94750572dc159ef7266 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 20 Oct 2025 20:31:01 +0200 Subject: [PATCH 01/17] phar: Remove pointless efree() (#20232) `error` is NULL at this point, otherwise we couldn't have gotten and *shouldn't* have gotten here in the first place. --- ext/phar/dirstream.c | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/phar/dirstream.c b/ext/phar/dirstream.c index 81fa62c854df..71420c11e62d 100644 --- a/ext/phar/dirstream.c +++ b/ext/phar/dirstream.c @@ -450,7 +450,6 @@ int phar_wrapper_mkdir(php_stream_wrapper *wrapper, const char *url_from, int mo if (NULL == zend_hash_add_mem(&phar->manifest, entry.filename, &entry, sizeof(phar_entry_info))) { php_stream_wrapper_log_error(wrapper, options, "phar error: cannot create directory \"%s\" in phar \"%s\", adding to manifest failed", ZSTR_VAL(entry.filename), phar->fname); - efree(error); zend_string_efree(entry.filename); return 0; } From 40b3f884260b5b3a686d8bba569317be0e2d0e2d Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 20 Oct 2025 20:31:16 +0200 Subject: [PATCH 02/17] phar: Avoid string duplication just for error message, use truncation formatting string (#20229) --- ext/phar/phar_object.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c index 9c09417e077b..fba332aa21a0 100644 --- a/ext/phar/phar_object.c +++ b/ext/phar/phar_object.c @@ -4315,10 +4315,7 @@ PHP_METHOD(Phar, extractTo) } if (ZSTR_LEN(path_to) >= MAXPATHLEN) { - char *tmp = estrndup(ZSTR_VAL(path_to), 50); - /* truncate for error message */ - zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0, "Cannot extract to \"%s...\", destination directory is too long for filesystem", tmp); - efree(tmp); + zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0, "Cannot extract to \"%.50s...\", destination directory is too long for filesystem", ZSTR_VAL(path_to)); RETURN_THROWS(); } From 0805953bb0b7b2a7bc646442a49953071556a344 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 19 Oct 2025 22:55:24 +0200 Subject: [PATCH 03/17] phar: Restructure code to get rid of a goto --- ext/phar/zip.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/ext/phar/zip.c b/ext/phar/zip.c index dff893c221b9..10301fc48e7d 100644 --- a/ext/phar/zip.c +++ b/ext/phar/zip.c @@ -234,7 +234,7 @@ zend_result phar_parse_zipfile(php_stream *fp, char *fname, size_t fname_len, ch uint16_t i; phar_archive_data *mydata = NULL; phar_entry_info entry = {0}; - char *p = buf, *ext, *actual_alias = NULL; + char *ext, *actual_alias = NULL; char *metadata = NULL; size = php_stream_tell(fp); @@ -261,7 +261,16 @@ zend_result phar_parse_zipfile(php_stream *fp, char *fname, size_t fname_len, ch return FAILURE; } - if ((p = phar_find_eocd(buf, size)) != NULL) { + char *p = phar_find_eocd(buf, size); + if (!p) { + php_stream_close(fp); + if (error) { + spprintf(error, 4096, "phar error: end of central directory not found in zip-based phar \"%s\"", fname); + } + return FAILURE; + } + + { memcpy((void *)&locator, (void *) p, sizeof(locator)); if (PHAR_GET_16(locator.centraldisk) != 0 || PHAR_GET_16(locator.disknumber) != 0) { /* split archives not handled */ @@ -301,18 +310,8 @@ zend_result phar_parse_zipfile(php_stream *fp, char *fname, size_t fname_len, ch } else { ZVAL_UNDEF(&mydata->metadata_tracker.val); } - - goto foundit; - } - - php_stream_close(fp); - - if (error) { - spprintf(error, 4096, "phar error: end of central directory not found in zip-based phar \"%s\"", fname); } - return FAILURE; -foundit: mydata->fname = pestrndup(fname, fname_len, mydata->is_persistent); #ifdef PHP_WIN32 phar_unixify_path_separators(mydata->fname, fname_len); From 0098f3efbce2311027ad558ce4f5eb5f02634101 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 19 Oct 2025 22:55:35 +0200 Subject: [PATCH 04/17] phar: De-indent code --- ext/phar/zip.c | 60 ++++++++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/ext/phar/zip.c b/ext/phar/zip.c index 10301fc48e7d..5f025d75e341 100644 --- a/ext/phar/zip.c +++ b/ext/phar/zip.c @@ -270,46 +270,44 @@ zend_result phar_parse_zipfile(php_stream *fp, char *fname, size_t fname_len, ch return FAILURE; } - { - memcpy((void *)&locator, (void *) p, sizeof(locator)); - if (PHAR_GET_16(locator.centraldisk) != 0 || PHAR_GET_16(locator.disknumber) != 0) { - /* split archives not handled */ - php_stream_close(fp); - if (error) { - spprintf(error, 4096, "phar error: split archives spanning multiple zips cannot be processed in zip-based phar \"%s\"", fname); - } - return FAILURE; + memcpy((void *)&locator, (void *) p, sizeof(locator)); + if (PHAR_GET_16(locator.centraldisk) != 0 || PHAR_GET_16(locator.disknumber) != 0) { + /* split archives not handled */ + php_stream_close(fp); + if (error) { + spprintf(error, 4096, "phar error: split archives spanning multiple zips cannot be processed in zip-based phar \"%s\"", fname); } + return FAILURE; + } - if (PHAR_GET_16(locator.counthere) != PHAR_GET_16(locator.count)) { - if (error) { - spprintf(error, 4096, "phar error: corrupt zip archive, conflicting file count in end of central directory record in zip-based phar \"%s\"", fname); - } - php_stream_close(fp); - return FAILURE; + if (PHAR_GET_16(locator.counthere) != PHAR_GET_16(locator.count)) { + if (error) { + spprintf(error, 4096, "phar error: corrupt zip archive, conflicting file count in end of central directory record in zip-based phar \"%s\"", fname); } + php_stream_close(fp); + return FAILURE; + } - mydata = pecalloc(1, sizeof(phar_archive_data), PHAR_G(persist)); - mydata->is_persistent = PHAR_G(persist); + mydata = pecalloc(1, sizeof(phar_archive_data), PHAR_G(persist)); + mydata->is_persistent = PHAR_G(persist); - /* read in archive comment, if any */ - if (PHAR_GET_16(locator.comment_len)) { + /* read in archive comment, if any */ + if (PHAR_GET_16(locator.comment_len)) { - metadata = p + sizeof(locator); + metadata = p + sizeof(locator); - if (PHAR_GET_16(locator.comment_len) != size - (metadata - buf)) { - if (error) { - spprintf(error, 4096, "phar error: corrupt zip archive, zip file comment truncated in zip-based phar \"%s\"", fname); - } - php_stream_close(fp); - pefree(mydata, mydata->is_persistent); - return FAILURE; + if (PHAR_GET_16(locator.comment_len) != size - (metadata - buf)) { + if (error) { + spprintf(error, 4096, "phar error: corrupt zip archive, zip file comment truncated in zip-based phar \"%s\"", fname); } - - phar_parse_metadata_lazy(metadata, &mydata->metadata_tracker, PHAR_GET_16(locator.comment_len), mydata->is_persistent); - } else { - ZVAL_UNDEF(&mydata->metadata_tracker.val); + php_stream_close(fp); + pefree(mydata, mydata->is_persistent); + return FAILURE; } + + phar_parse_metadata_lazy(metadata, &mydata->metadata_tracker, PHAR_GET_16(locator.comment_len), mydata->is_persistent); + } else { + ZVAL_UNDEF(&mydata->metadata_tracker.val); } mydata->fname = pestrndup(fname, fname_len, mydata->is_persistent); From 6291f97c149677ce9a9bb249ca16e5ffec00cafe Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 19 Oct 2025 23:03:51 +0200 Subject: [PATCH 05/17] phar: Move code to avoid goto --- ext/phar/dirstream.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ext/phar/dirstream.c b/ext/phar/dirstream.c index 71420c11e62d..6b5033659ce3 100644 --- a/ext/phar/dirstream.c +++ b/ext/phar/dirstream.c @@ -188,8 +188,6 @@ static php_stream *phar_make_dirstream(const char *dir, size_t dirlen, const Has entry = safe_emalloc(keylen, 1, 1); memcpy(entry, ZSTR_VAL(str_key), keylen); entry[keylen] = '\0'; - - goto PHAR_ADD_ENTRY; } else { if (0 != memcmp(ZSTR_VAL(str_key), dir, dirlen)) { /* entry in directory not found */ @@ -199,7 +197,6 @@ static php_stream *phar_make_dirstream(const char *dir, size_t dirlen, const Has continue; } } - } const char *save = ZSTR_VAL(str_key); save += dirlen + 1; /* seek to just past the path separator */ @@ -220,7 +217,8 @@ static php_stream *phar_make_dirstream(const char *dir, size_t dirlen, const Has entry[keylen - dirlen - 1] = '\0'; keylen = keylen - dirlen - 1; } -PHAR_ADD_ENTRY: + } + if (keylen) { /** * Add an empty element to avoid duplicates From a69b35328d02282f18c843b10df4d719f7465c08 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 19 Oct 2025 23:03:59 +0200 Subject: [PATCH 06/17] phar: Re-indent code --- ext/phar/dirstream.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/ext/phar/dirstream.c b/ext/phar/dirstream.c index 6b5033659ce3..f37599e7db11 100644 --- a/ext/phar/dirstream.c +++ b/ext/phar/dirstream.c @@ -198,25 +198,25 @@ static php_stream *phar_make_dirstream(const char *dir, size_t dirlen, const Has } } - const char *save = ZSTR_VAL(str_key); - save += dirlen + 1; /* seek to just past the path separator */ - - const char *has_slash = memchr(save, '/', keylen - dirlen - 1); - if (has_slash) { - /* is subdirectory */ - save -= dirlen + 1; - entry = safe_emalloc(has_slash - save + dirlen, 1, 1); - memcpy(entry, save + dirlen + 1, has_slash - save - dirlen - 1); - keylen = has_slash - save - dirlen - 1; - entry[keylen] = '\0'; - } else { - /* is file */ - save -= dirlen + 1; - entry = safe_emalloc(keylen - dirlen, 1, 1); - memcpy(entry, save + dirlen + 1, keylen - dirlen - 1); - entry[keylen - dirlen - 1] = '\0'; - keylen = keylen - dirlen - 1; - } + const char *save = ZSTR_VAL(str_key); + save += dirlen + 1; /* seek to just past the path separator */ + + const char *has_slash = memchr(save, '/', keylen - dirlen - 1); + if (has_slash) { + /* is subdirectory */ + save -= dirlen + 1; + entry = safe_emalloc(has_slash - save + dirlen, 1, 1); + memcpy(entry, save + dirlen + 1, has_slash - save - dirlen - 1); + keylen = has_slash - save - dirlen - 1; + entry[keylen] = '\0'; + } else { + /* is file */ + save -= dirlen + 1; + entry = safe_emalloc(keylen - dirlen, 1, 1); + memcpy(entry, save + dirlen + 1, keylen - dirlen - 1); + entry[keylen - dirlen - 1] = '\0'; + keylen = keylen - dirlen - 1; + } } if (keylen) { From 9c90e16e188d5fd8cca8cd77fd8c65d29308428e Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 20 Oct 2025 20:34:04 +0200 Subject: [PATCH 07/17] phar: Avoid an error goto in phar_zip_flush() (#20233) --- ext/phar/zip.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/ext/phar/zip.c b/ext/phar/zip.c index 5f025d75e341..5abbc49d3c2c 100644 --- a/ext/phar/zip.c +++ b/ext/phar/zip.c @@ -1392,8 +1392,8 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(1, 4) void phar_zip_flush(phar_archive_data *phar, z zend_hash_apply_with_argument(&phar->manifest, phar_zip_changed_apply, (void *) &pass); phar_metadata_tracker_try_ensure_has_serialized_data(&phar->metadata_tracker, phar->is_persistent); - if (pass_error) { -has_pass_error: + if (pass_error + || FAILURE == phar_zip_applysignature(phar, &pass)) { spprintf(error, 4096, "phar zip flush of \"%s\" failed: %s", phar->fname, pass_error); efree(pass_error); nopasserror: @@ -1406,11 +1406,6 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(1, 4) void phar_zip_flush(phar_archive_data *phar, z return; } - if (FAILURE == phar_zip_applysignature(phar, &pass)) { - ZEND_ASSERT(pass_error != NULL); - goto has_pass_error; - } - /* save zip */ cdir_size = php_stream_tell(pass.centralfp); cdir_offset = php_stream_tell(pass.filefp); From df8ce6ddbd713d7e9163dd60d19648e2e4a8bb80 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 20 Oct 2025 00:10:40 +0200 Subject: [PATCH 08/17] Zend: remove zval_dtor() compatibility macro This is an alias for zval_ptr_dtor_nogc(). I've seen people make mistakes against this and use zval_dtor() instead of zval_ptr_dtor(). The crucial detail here is that the former won't root possible GC cycles while the latter will. We can avoid the confusion by just retiring this compatibility macro. Closes GH-20235. --- UPGRADING.INTERNALS | 2 ++ Zend/zend_variables.h | 3 --- ext/pdo/pdo_stmt.c | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index d269bd107885..42e52480bbb9 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -32,6 +32,8 @@ PHP 8.6 INTERNALS UPGRADE NOTES . ZEND_LTOA() (and ZEND_LTOA_BUF_LEN) has been removed, as it was unsafe. Directly use ZEND_LONG_FMT with a function from the printf family. + . The zval_dtor() alias of zval_ptr_dtor_nogc() has been removed. + Call zval_ptr_dtor_nogc() directly instead. ======================== 2. Build system changes diff --git a/Zend/zend_variables.h b/Zend/zend_variables.h index 1cb745ca1b1d..d90ad9951782 100644 --- a/Zend/zend_variables.h +++ b/Zend/zend_variables.h @@ -81,9 +81,6 @@ ZEND_API void zval_ptr_dtor(zval *zval_ptr); ZEND_API void zval_ptr_safe_dtor(zval *zval_ptr); ZEND_API void zval_internal_ptr_dtor(zval *zvalue); -/* Kept for compatibility */ -#define zval_dtor(zvalue) zval_ptr_dtor_nogc(zvalue) - ZEND_API void zval_add_ref(zval *p); END_EXTERN_C() diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index e2723c703f0a..e87af66c7586 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1052,7 +1052,7 @@ PHP_METHOD(PDOStatement, fetch) array_init_size(return_value, 1); bool success = pdo_do_key_pair_fetch(stmt, ori, off, Z_ARRVAL_P(return_value)); if (!success) { - zval_dtor(return_value); + zval_ptr_dtor_nogc(return_value); PDO_HANDLE_STMT_ERR(); RETURN_FALSE; } From 8e84e9a5514e6566ed8bbcaebfacbd32dc42f74b Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 18 Oct 2025 11:16:55 +0200 Subject: [PATCH 09/17] pgsql: Use cheaper string conversion functions --- ext/pgsql/pgsql.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index 3f511d21456b..b59c9ef8f3c8 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -3409,7 +3409,8 @@ PHP_FUNCTION(pg_copy_to) static zend_result pgsql_copy_from_query(PGconn *pgsql, PGresult *pgsql_result, zval *value) { - zend_string *tmp = zval_try_get_string(value); + zend_string *tmp_tmp; + zend_string *tmp = zval_try_get_tmp_string(value, &tmp_tmp); if (UNEXPECTED(!tmp)) { return FAILURE; } @@ -3423,11 +3424,11 @@ static zend_result pgsql_copy_from_query(PGconn *pgsql, PGresult *pgsql_result, } if (PQputCopyData(pgsql, ZSTR_VAL(zquery), ZSTR_LEN(zquery)) != 1) { zend_string_release_ex(zquery, false); - zend_string_release(tmp); + zend_tmp_string_release(tmp_tmp); return FAILURE; } zend_string_release_ex(zquery, false); - zend_string_release(tmp); + zend_tmp_string_release(tmp_tmp); return SUCCESS; } From e2396a6ffb60ff3d426f3b66182f60ab4dffdc7c Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 18 Oct 2025 11:19:34 +0200 Subject: [PATCH 10/17] pgsql: Simplify pgsql_copy_from_query() by using character buffers This also avoids some unnecessary zend_string overhead. --- ext/pgsql/pgsql.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index b59c9ef8f3c8..b0e2f53cc8d9 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -3414,21 +3414,20 @@ static zend_result pgsql_copy_from_query(PGconn *pgsql, PGresult *pgsql_result, if (UNEXPECTED(!tmp)) { return FAILURE; } - zend_string *zquery = zend_string_alloc(ZSTR_LEN(tmp) + 2, false); - memcpy(ZSTR_VAL(zquery), ZSTR_VAL(tmp), ZSTR_LEN(tmp) + 1); - ZSTR_LEN(zquery) = ZSTR_LEN(tmp); - if (ZSTR_LEN(tmp) > 0 && ZSTR_VAL(zquery)[ZSTR_LEN(tmp) - 1] != '\n') { - ZSTR_VAL(zquery)[ZSTR_LEN(tmp)] = '\n'; - ZSTR_VAL(zquery)[ZSTR_LEN(tmp) + 1] = '\0'; - ZSTR_LEN(zquery) ++; - } - if (PQputCopyData(pgsql, ZSTR_VAL(zquery), ZSTR_LEN(zquery)) != 1) { - zend_string_release_ex(zquery, false); - zend_tmp_string_release(tmp_tmp); - return FAILURE; + char *zquery = emalloc(ZSTR_LEN(tmp) + 2); + memcpy(zquery, ZSTR_VAL(tmp), ZSTR_LEN(tmp) + 1); + size_t zquery_len = ZSTR_LEN(tmp); + if (ZSTR_LEN(tmp) > 0 && zquery[ZSTR_LEN(tmp) - 1] != '\n') { + zquery[ZSTR_LEN(tmp)] = '\n'; + zquery[ZSTR_LEN(tmp) + 1] = '\0'; + zquery_len++; } - zend_string_release_ex(zquery, false); zend_tmp_string_release(tmp_tmp); + if (PQputCopyData(pgsql, zquery, zquery_len) != 1) { + efree(zquery); + return FAILURE; + } + efree(zquery); return SUCCESS; } From da9638298c8a578970286106e0e0ee8a031492ab Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 18 Oct 2025 11:23:47 +0200 Subject: [PATCH 11/17] pgsql: Avoid unnecessary work in pgsql_copy_from_query() if the input already ends in a newline --- ext/pgsql/pgsql.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index b0e2f53cc8d9..1212acdd1476 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -3414,21 +3414,22 @@ static zend_result pgsql_copy_from_query(PGconn *pgsql, PGresult *pgsql_result, if (UNEXPECTED(!tmp)) { return FAILURE; } - char *zquery = emalloc(ZSTR_LEN(tmp) + 2); - memcpy(zquery, ZSTR_VAL(tmp), ZSTR_LEN(tmp) + 1); - size_t zquery_len = ZSTR_LEN(tmp); - if (ZSTR_LEN(tmp) > 0 && zquery[ZSTR_LEN(tmp) - 1] != '\n') { + + int result; + if (ZSTR_LEN(tmp) > 0 && ZSTR_VAL(tmp)[ZSTR_LEN(tmp) - 1] != '\n') { + char *zquery = emalloc(ZSTR_LEN(tmp) + 2); + memcpy(zquery, ZSTR_VAL(tmp), ZSTR_LEN(tmp) + 1); zquery[ZSTR_LEN(tmp)] = '\n'; zquery[ZSTR_LEN(tmp) + 1] = '\0'; - zquery_len++; - } - zend_tmp_string_release(tmp_tmp); - if (PQputCopyData(pgsql, zquery, zquery_len) != 1) { + result = PQputCopyData(pgsql, zquery, ZSTR_LEN(tmp) + 1); efree(zquery); - return FAILURE; + } else { + result = PQputCopyData(pgsql, ZSTR_VAL(tmp), ZSTR_LEN(tmp)); } - efree(zquery); - return SUCCESS; + + zend_tmp_string_release(tmp_tmp); + + return result != 1 ? FAILURE : SUCCESS; } /* {{{ Copy table from array */ From 5eec4d8001ca97f54ce52e7be4bd131e0f4ef0d6 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 18 Oct 2025 11:36:08 +0200 Subject: [PATCH 12/17] pgsql: Avoid duplicating strings and factor out parameter building code --- ext/pgsql/pgsql.c | 144 +++++++++++++++++----------------------------- 1 file changed, 53 insertions(+), 91 deletions(-) diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index 1212acdd1476..89c99cd84132 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -1223,26 +1223,55 @@ PHP_FUNCTION(pg_query) } } -static void _php_pgsql_free_params(char **params, int num_params) +/* The char pointer MUST refer to the char* of a zend_string struct */ +static void php_pgsql_zend_string_release_from_char_pointer(char *ptr) { + zend_string_release((zend_string*) (ptr - XtOffsetOf(zend_string, val))); +} + +static void _php_pgsql_free_params(char **params, uint32_t num_params) { - int i; - for (i = 0; i < num_params; i++) { + for (uint32_t i = 0; i < num_params; i++) { if (params[i]) { - efree(params[i]); + php_pgsql_zend_string_release_from_char_pointer(params[i]); } } efree(params); } +static char **php_pgsql_make_arguments(const HashTable *param_arr, int *num_params) +{ + /* This conversion is safe because of the limit of number of elements in a table. */ + *num_params = (int) zend_hash_num_elements(param_arr); + char **params = safe_emalloc(sizeof(char *), *num_params, 0); + uint32_t i = 0; + + ZEND_HASH_FOREACH_VAL(param_arr, zval *tmp) { + ZVAL_DEREF(tmp); + if (Z_TYPE_P(tmp) == IS_NULL) { + params[i] = NULL; + } else { + zend_string *param_str = zval_try_get_string(tmp); + if (!param_str) { + _php_pgsql_free_params(params, i); + return NULL; + } + params[i] = ZSTR_VAL(param_str); + } + i++; + } ZEND_HASH_FOREACH_END(); + + return params; +} + /* Execute a query */ PHP_FUNCTION(pg_query_params) { zval *pgsql_link = NULL; - zval *pv_param_arr, *tmp; + zval *pv_param_arr; char *query; size_t query_len; bool leftover = false; - int num_params = 0; + int num_params; char **params = NULL; pgsql_link_handle *link; PGconn *pgsql; @@ -1286,26 +1315,9 @@ PHP_FUNCTION(pg_query_params) php_error_docref(NULL, E_NOTICE, "Found results on this connection. Use pg_get_result() to get these results first"); } - num_params = zend_hash_num_elements(Z_ARRVAL_P(pv_param_arr)); - if (num_params > 0) { - int i = 0; - params = (char **)safe_emalloc(sizeof(char *), num_params, 0); - - ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(pv_param_arr), tmp) { - ZVAL_DEREF(tmp); - if (Z_TYPE_P(tmp) == IS_NULL) { - params[i] = NULL; - } else { - zend_string *param_str = zval_try_get_string(tmp); - if (!param_str) { - _php_pgsql_free_params(params, i); - RETURN_THROWS(); - } - params[i] = estrndup(ZSTR_VAL(param_str), ZSTR_LEN(param_str)); - zend_string_release(param_str); - } - i++; - } ZEND_HASH_FOREACH_END(); + params = php_pgsql_make_arguments(Z_ARRVAL_P(pv_param_arr), &num_params); + if (UNEXPECTED(!params)) { + RETURN_THROWS(); } pgsql_result = PQexecParams(pgsql, query, num_params, @@ -1440,11 +1452,11 @@ PHP_FUNCTION(pg_prepare) PHP_FUNCTION(pg_execute) { zval *pgsql_link = NULL; - zval *pv_param_arr, *tmp; + zval *pv_param_arr; char *stmtname; size_t stmtname_len; bool leftover = false; - int num_params = 0; + int num_params; char **params = NULL; PGconn *pgsql; pgsql_link_handle *link; @@ -1488,25 +1500,9 @@ PHP_FUNCTION(pg_execute) php_error_docref(NULL, E_NOTICE, "Found results on this connection. Use pg_get_result() to get these results first"); } - num_params = zend_hash_num_elements(Z_ARRVAL_P(pv_param_arr)); - if (num_params > 0) { - int i = 0; - params = (char **)safe_emalloc(sizeof(char *), num_params, 0); - - ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(pv_param_arr), tmp) { - ZVAL_DEREF(tmp); - if (Z_TYPE_P(tmp) == IS_NULL) { - params[i] = NULL; - } else { - zend_string *tmp_str; - zend_string *str = zval_get_tmp_string(tmp, &tmp_str); - - params[i] = estrndup(ZSTR_VAL(str), ZSTR_LEN(str)); - zend_tmp_string_release(tmp_str); - } - - i++; - } ZEND_HASH_FOREACH_END(); + params = php_pgsql_make_arguments(Z_ARRVAL_P(pv_param_arr), &num_params); + if (UNEXPECTED(!params)) { + RETURN_THROWS(); } pgsql_result = PQexecPrepared(pgsql, stmtname, num_params, @@ -4034,9 +4030,9 @@ PHP_FUNCTION(pg_send_query) /* {{{ Send asynchronous parameterized query */ PHP_FUNCTION(pg_send_query_params) { - zval *pgsql_link, *pv_param_arr, *tmp; + zval *pgsql_link, *pv_param_arr; pgsql_link_handle *link; - int num_params = 0; + int num_params; char **params = NULL; char *query; size_t query_len; @@ -4066,25 +4062,9 @@ PHP_FUNCTION(pg_send_query_params) "There are results on this connection. Call pg_get_result() until it returns FALSE"); } - num_params = zend_hash_num_elements(Z_ARRVAL_P(pv_param_arr)); - if (num_params > 0) { - int i = 0; - params = (char **)safe_emalloc(sizeof(char *), num_params, 0); - - ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(pv_param_arr), tmp) { - ZVAL_DEREF(tmp); - if (Z_TYPE_P(tmp) == IS_NULL) { - params[i] = NULL; - } else { - zend_string *tmp_str; - zend_string *str = zval_get_tmp_string(tmp, &tmp_str); - - params[i] = estrndup(ZSTR_VAL(str), ZSTR_LEN(str)); - zend_tmp_string_release(tmp_str); - } - - i++; - } ZEND_HASH_FOREACH_END(); + params = php_pgsql_make_arguments(Z_ARRVAL_P(pv_param_arr), &num_params); + if (UNEXPECTED(!params)) { + RETURN_THROWS(); } if (PQsendQueryParams(pgsql, query, num_params, NULL, (const char * const *)params, NULL, NULL, 0)) { @@ -4206,8 +4186,8 @@ PHP_FUNCTION(pg_send_execute) { zval *pgsql_link; pgsql_link_handle *link; - zval *pv_param_arr, *tmp; - int num_params = 0; + zval *pv_param_arr; + int num_params; char **params = NULL; char *stmtname; size_t stmtname_len; @@ -4237,27 +4217,9 @@ PHP_FUNCTION(pg_send_execute) "There are results on this connection. Call pg_get_result() until it returns FALSE"); } - num_params = zend_hash_num_elements(Z_ARRVAL_P(pv_param_arr)); - if (num_params > 0) { - int i = 0; - params = (char **)safe_emalloc(sizeof(char *), num_params, 0); - - ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(pv_param_arr), tmp) { - ZVAL_DEREF(tmp); - if (Z_TYPE_P(tmp) == IS_NULL) { - params[i] = NULL; - } else { - zend_string *tmp_str = zval_try_get_string(tmp); - if (UNEXPECTED(!tmp_str)) { - _php_pgsql_free_params(params, i); - return; - } - params[i] = estrndup(ZSTR_VAL(tmp_str), ZSTR_LEN(tmp_str)); - zend_string_release(tmp_str); - } - - i++; - } ZEND_HASH_FOREACH_END(); + params = php_pgsql_make_arguments(Z_ARRVAL_P(pv_param_arr), &num_params); + if (UNEXPECTED(!params)) { + RETURN_THROWS(); } if (PQsendQueryPrepared(pgsql, stmtname, num_params, (const char * const *)params, NULL, NULL, 0)) { From 020bbea8fde8b83b0a6c3c0416d0a648466acb68 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 17 Oct 2025 23:18:04 +0200 Subject: [PATCH 13/17] phar: Fix memory leak when openssl polyfill returns garbage Closes GH-20210. --- NEWS | 1 + ...sl_sign_invalid_polyfill_return_value.phpt | 34 +++++++++++++++++++ ext/phar/util.c | 4 ++- 3 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 ext/phar/tests/openssl_sign_invalid_polyfill_return_value.phpt diff --git a/NEWS b/NEWS index 3bc4e05761f8..020927bb5ff6 100644 --- a/NEWS +++ b/NEWS @@ -55,6 +55,7 @@ PHP NEWS (nielsdos) . Fix potential buffer length truncation due to usage of type int instead of type size_t. (Girgias) + . Fix memory leak when openssl polyfill returns garbage. (nielsdos) - Random: . Fix Randomizer::__serialize() w.r.t. INDIRECTs. (nielsdos) diff --git a/ext/phar/tests/openssl_sign_invalid_polyfill_return_value.phpt b/ext/phar/tests/openssl_sign_invalid_polyfill_return_value.phpt new file mode 100644 index 000000000000..37c14188edf9 --- /dev/null +++ b/ext/phar/tests/openssl_sign_invalid_polyfill_return_value.phpt @@ -0,0 +1,34 @@ +--TEST-- +openssl_sign() polyfill with wrong return value +--EXTENSIONS-- +phar +--SKIPIF-- + +--INI-- +phar.require_hash=0 +--FILE-- +setSignatureAlgorithm(Phar::OPENSSL, "randomcrap"); +try { + $phar->addEmptyDir('blah'); +} catch (PharException $e) { + echo $e->getMessage(); +} + +?> +--CLEAN-- + +--EXPECTF-- +phar error: unable to write signature to tar-based phar: unable to write phar "%s" with requested openssl signature diff --git a/ext/phar/util.c b/ext/phar/util.c index 416aa1dcd7b0..c69f830e6280 100644 --- a/ext/phar/util.c +++ b/ext/phar/util.c @@ -1471,7 +1471,6 @@ static int phar_call_openssl_signverify(int is_sign, php_stream *fp, zend_off_t zval_ptr_dtor_str(&zp[2]); switch (Z_TYPE(retval)) { - default: case IS_LONG: zval_ptr_dtor(&zp[1]); if (1 == Z_LVAL(retval)) { @@ -1483,6 +1482,9 @@ static int phar_call_openssl_signverify(int is_sign, php_stream *fp, zend_off_t *signature_len = Z_STRLEN(zp[1]); zval_ptr_dtor(&zp[1]); return SUCCESS; + default: + zval_ptr_dtor(&retval); + ZEND_FALLTHROUGH; case IS_FALSE: zval_ptr_dtor(&zp[1]); return FAILURE; From fba06d836ae5b8bb1ccb244096d102b38bd155c0 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 20 Oct 2025 22:35:47 +0200 Subject: [PATCH 14/17] Fix merge issue --- ext/phar/util.c | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/phar/util.c b/ext/phar/util.c index 7c94ba2e63fd..7daeb450c3d7 100644 --- a/ext/phar/util.c +++ b/ext/phar/util.c @@ -1504,7 +1504,6 @@ static zend_result phar_call_openssl_signverify(bool is_sign, php_stream *fp, ze zval_ptr_dtor(&retval); ZEND_FALLTHROUGH; case IS_FALSE: - default: zval_ptr_dtor(&zp[1]); return FAILURE; } From 90bc40ecc0f0d2bf62e97c42256bd10015db3b5e Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Mon, 20 Oct 2025 17:53:00 +0200 Subject: [PATCH 15/17] Create separate workflow for nightly slack notification Closes GH-20248 --- .github/actions/notify-slack/action.yml | 10 ----- .github/workflows/nightly-results.yml | 16 ++++++++ .github/workflows/nightly.yml | 51 ------------------------- .github/workflows/root.yml | 5 --- 4 files changed, 16 insertions(+), 66 deletions(-) delete mode 100644 .github/actions/notify-slack/action.yml create mode 100644 .github/workflows/nightly-results.yml diff --git a/.github/actions/notify-slack/action.yml b/.github/actions/notify-slack/action.yml deleted file mode 100644 index 1ff425b51c6a..000000000000 --- a/.github/actions/notify-slack/action.yml +++ /dev/null @@ -1,10 +0,0 @@ -name: Notify Slack -inputs: - token: - required: true -runs: - using: composite - steps: - - shell: bash - run: >- - curl -X POST -H 'Content-type: application/json' --data '{"attachments": [{"text": "Job in *nightly* failed", "footer": "", "color": "danger", "mrkdwn_in": ["text"]}]}' ${{ inputs.token }} diff --git a/.github/workflows/nightly-results.yml b/.github/workflows/nightly-results.yml new file mode 100644 index 000000000000..c1c4da72ca56 --- /dev/null +++ b/.github/workflows/nightly-results.yml @@ -0,0 +1,16 @@ +name: Nightly results +on: + workflow_run: + workflows: + - Nightly + types: + - completed +jobs: + on-failure: + runs-on: ubuntu-latest + if: ${{ github.repository == 'php/php-src' && github.event.workflow_run.conclusion == 'failure' }} + steps: + - run: | + export DEBIAN_FRONTEND=noninteractive + sudo apt-get install -y curl + curl -X POST -H 'Content-type: application/json' --data '{"attachments": [{"text": "Job in *nightly* failed", "footer": "<${{ github.event.workflow_run.jobs_url }}|View Run>", "color": "danger", "mrkdwn_in": ["text"]}]}' ${{ secrets.ACTION_MONITORING_SLACK }} diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index e0322e3b1be2..c360146508b9 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -136,12 +136,6 @@ jobs: -d opcache.enable_cli=1 - name: Extra tests uses: ./.github/actions/extra-tests - - name: Notify Slack - if: failure() - uses: ./.github/actions/notify-slack - with: - token: ${{ secrets.ACTION_MONITORING_SLACK }} - LINUX_X64: services: mysql: @@ -273,11 +267,6 @@ jobs: uses: ./.github/actions/extra-tests - name: Verify generated files are up to date uses: ./.github/actions/verify-generated-files - - name: Notify Slack - if: failure() - uses: ./.github/actions/notify-slack - with: - token: ${{ secrets.ACTION_MONITORING_SLACK }} LINUX_X32: strategy: fail-fast: false @@ -362,11 +351,6 @@ jobs: -d opcache.enable_cli=1 - name: Extra tests uses: ./.github/actions/extra-tests - - name: Notify Slack - if: failure() - uses: ./.github/actions/notify-slack - with: - token: ${{ secrets.ACTION_MONITORING_SLACK }} MACOS: strategy: fail-fast: false @@ -425,11 +409,6 @@ jobs: uses: ./.github/actions/extra-tests - name: Verify generated files are up to date uses: ./.github/actions/verify-generated-files - - name: Notify Slack - if: failure() - uses: ./.github/actions/notify-slack - with: - token: ${{ secrets.ACTION_MONITORING_SLACK }} COVERAGE_DEBUG_NTS: if: inputs.branch == 'master' services: @@ -491,11 +470,6 @@ jobs: fail_ci_if_error: true token: ${{ secrets.CODECOV_TOKEN }} verbose: true - - name: Notify Slack - if: failure() - uses: ./.github/actions/notify-slack - with: - token: ${{ secrets.ACTION_MONITORING_SLACK }} COMMUNITY: strategy: fail-fast: false @@ -683,11 +657,6 @@ jobs: if [ $EXIT_CODE -gt 128 ]; then exit 1 fi - - name: Notify Slack - if: failure() - uses: ./.github/actions/notify-slack - with: - token: ${{ secrets.ACTION_MONITORING_SLACK }} OPCACHE_VARIATION: services: mysql: @@ -774,11 +743,6 @@ jobs: -d opcache.file_cache_only=1 - name: Verify generated files are up to date uses: ./.github/actions/verify-generated-files - - name: Notify Slack - if: failure() - uses: ./.github/actions/notify-slack - with: - token: ${{ secrets.ACTION_MONITORING_SLACK }} MSAN: name: MSAN runs-on: ubuntu-${{ inputs.ubuntu_version }} @@ -866,11 +830,6 @@ jobs: -d opcache.enable_cli=1 - name: Verify generated files are up to date uses: ./.github/actions/verify-generated-files - - name: Notify Slack - if: failure() - uses: ./.github/actions/notify-slack - with: - token: ${{ secrets.ACTION_MONITORING_SLACK }} LIBMYSQLCLIENT: name: LIBMYSQLCLIENT runs-on: ubuntu-${{ inputs.ubuntu_version }} @@ -913,11 +872,6 @@ jobs: withMysqli: ${{ inputs.libmysqlclient_with_mysqli }} - name: Verify generated files are up to date uses: ./.github/actions/verify-generated-files - - name: Notify Slack - if: failure() - uses: ./.github/actions/notify-slack - with: - token: ${{ secrets.ACTION_MONITORING_SLACK }} PECL: if: inputs.branch == 'master' runs-on: ubuntu-22.04 @@ -1025,11 +979,6 @@ jobs: /opt/php/bin/phpize ./configure --prefix=/opt/php --with-php-config=/opt/php/bin/php-config make -j$(/usr/bin/nproc) - - name: Notify Slack - if: failure() - uses: ./.github/actions/notify-slack - with: - token: ${{ secrets.ACTION_MONITORING_SLACK }} WINDOWS: strategy: fail-fast: false diff --git a/.github/workflows/root.yml b/.github/workflows/root.yml index b70b026eced0..4062358fcbc9 100644 --- a/.github/workflows/root.yml +++ b/.github/workflows/root.yml @@ -31,11 +31,6 @@ jobs: - name: Generate Matrix id: set-matrix run: php .github/nightly_matrix.php "${{ github.event_name }}" "${{ github.run_attempt }}" "${{ github.head_ref || github.ref_name }}" - - name: Notify Slack - if: failure() - uses: ./.github/actions/notify-slack - with: - token: ${{ secrets.ACTION_MONITORING_SLACK }} NIGHTLY: needs: GENERATE_MATRIX name: ${{ matrix.branch.ref }} From 0e17dcfe54f972e9531459b4b253a182eefcdef3 Mon Sep 17 00:00:00 2001 From: Shivam Mathur Date: Wed, 6 Aug 2025 16:11:38 +0530 Subject: [PATCH 16/17] Fix Windows test for openssl-3.5 upgrade (#19384) * Fix Windows test for openssl-3.5 upgrade * Update ext/openssl/tests/check_default_conf_path.phpt Co-authored-by: Christoph M. Becker --------- Co-authored-by: Christoph M. Becker --- ext/openssl/tests/check_default_conf_path.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/openssl/tests/check_default_conf_path.phpt b/ext/openssl/tests/check_default_conf_path.phpt index 4590ef7804da..267bebe16c1f 100644 --- a/ext/openssl/tests/check_default_conf_path.phpt +++ b/ext/openssl/tests/check_default_conf_path.phpt @@ -21,7 +21,7 @@ ob_end_clean(); preg_match(",Openssl default config [^ ]* (.*),", $info, $m); if (isset($m[1])) { - var_dump(str_replace('/', '\\', strtolower($m[1]))); + var_dump(str_replace('\\/', '\\', strtolower($m[1]))); } else { echo $info; } From 1bfe9340b722145be2f82b5880a28ba2b2ee0ba0 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 21 Oct 2025 00:55:08 +0200 Subject: [PATCH 17/17] [skip ci] Skip openssl tests currently failing on 8.2 --- ext/openssl/tests/bug74341.phpt | 1 + ext/openssl/tests/openssl_x509_parse_basic.phpt | 1 + 2 files changed, 2 insertions(+) diff --git a/ext/openssl/tests/bug74341.phpt b/ext/openssl/tests/bug74341.phpt index 97c4f9b55c46..b08e71234b64 100644 --- a/ext/openssl/tests/bug74341.phpt +++ b/ext/openssl/tests/bug74341.phpt @@ -5,6 +5,7 @@ openssl --SKIPIF-- = 0x30300000) die('skip For OpenSSL < 3.3'); +if (substr(PHP_OS, 0, 3) == "WIN") die("skip Failing on Windows"); ?> --FILE-- = 0x30200000) die('skip For OpenSSL < 3.2'); +if (substr(PHP_OS, 0, 3) == "WIN") die("skip Failing on Windows"); ?> --FILE--