From 2157055eb1c2942adc2b3305148a04e261d6d44c Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 1 Sep 2026 18:30:21 +0100 Subject: [PATCH 01/10] pcre: reduce scope of variables --- ext/pcre/php_pcre.c | 44 ++++++++++++++++---------------------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/ext/pcre/php_pcre.c b/ext/pcre/php_pcre.c index 4fde4ed5f9b9..997c20d11093 100644 --- a/ext/pcre/php_pcre.c +++ b/ext/pcre/php_pcre.c @@ -1155,7 +1155,6 @@ PHPAPI void php_pcre_match_impl(pcre_cache_entry *pce, zend_string *subject_str, uint32_t num_subpats; /* Number of captured subpatterns */ int matched; /* Has anything matched */ zend_string **subpat_names; /* Array for named subpatterns */ - size_t i; uint32_t subpats_order; /* Order of subpattern matches */ uint32_t offset_capture; /* Capture match offsets: yes/no */ zend_long unmatched_as_null; /* Null non-matches: yes/no */ @@ -1248,7 +1247,7 @@ PHPAPI void php_pcre_match_impl(pcre_cache_entry *pce, zend_string *subject_str, /* Allocate match sets array and initialize the values. */ if (global && subpats && subpats_order == PREG_PATTERN_ORDER) { match_sets = safe_emalloc(num_subpats, sizeof(HashTable *), 0); - for (i=0; i old_replace_count) { /* Add to return array */ + zval zv; ZVAL_STR(&zv, result); if (string_key) { zend_hash_add_new(return_value_ht, string_key, &zv); @@ -2434,9 +2425,9 @@ PHP_FUNCTION(preg_replace_callback) /* {{{ Perform Perl-style regular expression replacement using replacement callback. */ PHP_FUNCTION(preg_replace_callback_array) { - zval *replace, *zcount = NULL; + zval *zcount = NULL; HashTable *pattern, *subject_ht; - zend_string *subject_str, *str_idx_regex; + zend_string *subject_str; zend_long limit = -1, flags = 0; size_t replace_count = 0; @@ -2456,7 +2447,7 @@ PHP_FUNCTION(preg_replace_callback_array) GC_TRY_ADDREF(subject_str); } - ZEND_HASH_FOREACH_STR_KEY_VAL(pattern, str_idx_regex, replace) { + ZEND_HASH_FOREACH_STR_KEY_VAL(pattern, zend_string *str_idx_regex, zval *replace) { if (!str_idx_regex) { zend_argument_type_error(1, "must contain only string patterns as keys"); goto error; @@ -2924,12 +2915,9 @@ PHP_FUNCTION(preg_grep) PHPAPI void php_pcre_grep_impl(pcre_cache_entry *pce, zval *input, zval *return_value, zend_long flags) /* {{{ */ { - zval *entry; /* An entry in the input array */ uint32_t num_subpats; /* Number of captured subpatterns */ int count; /* Count of matched subpatterns */ uint32_t options; /* Execution options */ - zend_string *string_key; - zend_ulong num_key; bool invert; /* Whether to return non-matching entries */ bool old_mdata_used; @@ -2960,7 +2948,7 @@ PHPAPI void php_pcre_grep_impl(pcre_cache_entry *pce, zval *input, zval *return options = (pce->compile_options & PCRE2_UTF) ? 0 : PCRE2_NO_UTF_CHECK; /* Go through the input array */ - ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(input), num_key, string_key, entry) { + ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(input), zend_ulong num_key, zend_string *string_key, zval *entry) { zend_string *tmp_subject_str; zend_string *subject_str = zval_get_tmp_string(entry, &tmp_subject_str); From cc507facf4da31e644676c87d769e505a4190bf0 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 1 Sep 2026 18:34:21 +0100 Subject: [PATCH 02/10] pcre: use php_pcre_error_code type instead of int type --- ext/pcre/php_pcre.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/pcre/php_pcre.c b/ext/pcre/php_pcre.c index 997c20d11093..9cb778f1b2d5 100644 --- a/ext/pcre/php_pcre.c +++ b/ext/pcre/php_pcre.c @@ -104,7 +104,7 @@ static void php_pcre_free_char_table(zval *data) static void pcre_handle_exec_error(int pcre_code) /* {{{ */ { - int preg_code = 0; + php_pcre_error_code preg_code = PHP_PCRE_NO_ERROR; switch (pcre_code) { case PCRE2_ERROR_MATCHLIMIT: From fdc9206eab856e832074da7876a8cccfa063922d Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 1 Sep 2026 18:36:27 +0100 Subject: [PATCH 03/10] pcre: use bool type instead of uint8_t --- ext/pcre/php_pcre.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/ext/pcre/php_pcre.c b/ext/pcre/php_pcre.c index 9cb778f1b2d5..76d99c6e9521 100644 --- a/ext/pcre/php_pcre.c +++ b/ext/pcre/php_pcre.c @@ -76,7 +76,7 @@ ZEND_TLS pcre2_compile_context *cctx = NULL; ZEND_TLS pcre2_match_context *mctx = NULL; ZEND_TLS pcre2_match_data *mdata = NULL; ZEND_TLS bool mdata_used = 0; -ZEND_TLS uint8_t pcre2_init_ok = 0; +ZEND_TLS bool pcre2_init_ok = false; #if defined(ZTS) && defined(HAVE_PCRE_JIT_SUPPORT) static MUTEX_T pcre_mt = NULL; #define php_pcre_mutex_alloc() \ @@ -204,7 +204,7 @@ static void php_pcre_init_pcre2(uint8_t jit) if (!gctx) { gctx = pcre2_general_context_create(php_pcre_malloc, php_pcre_free, NULL); if (!gctx) { - pcre2_init_ok = 0; + pcre2_init_ok = false; return; } } @@ -212,7 +212,7 @@ static void php_pcre_init_pcre2(uint8_t jit) if (!cctx) { cctx = pcre2_compile_context_create(gctx); if (!cctx) { - pcre2_init_ok = 0; + pcre2_init_ok = false; return; } } @@ -220,7 +220,7 @@ static void php_pcre_init_pcre2(uint8_t jit) if (!mctx) { mctx = pcre2_match_context_create(gctx); if (!mctx) { - pcre2_init_ok = 0; + pcre2_init_ok = false; return; } } @@ -229,7 +229,7 @@ static void php_pcre_init_pcre2(uint8_t jit) if (jit && !jit_stack) { jit_stack = pcre2_jit_stack_create(PCRE_JIT_STACK_MIN_SIZE, PCRE_JIT_STACK_MAX_SIZE, gctx); if (!jit_stack) { - pcre2_init_ok = 0; + pcre2_init_ok = false; return; } } @@ -238,12 +238,12 @@ static void php_pcre_init_pcre2(uint8_t jit) if (!mdata) { mdata = pcre2_match_data_create(PHP_PCRE_PREALLOC_MDATA_SIZE, gctx); if (!mdata) { - pcre2_init_ok = 0; + pcre2_init_ok = false; return; } } - pcre2_init_ok = 1; + pcre2_init_ok = true; }/*}}}*/ static void php_pcre_shutdown_pcre2(void) @@ -277,7 +277,7 @@ static void php_pcre_shutdown_pcre2(void) mdata = NULL; } - pcre2_init_ok = 0; + pcre2_init_ok = false; }/*}}}*/ static PHP_GINIT_FUNCTION(pcre) /* {{{ */ From 7215ba9501156d3d91310953594fdba00d2c6aa3 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 1 Sep 2026 18:39:48 +0100 Subject: [PATCH 04/10] pcre: add const qualifiers --- ext/pcre/php_pcre.c | 26 +++++++++++++------------- ext/pcre/php_pcre.h | 6 +++--- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/ext/pcre/php_pcre.c b/ext/pcre/php_pcre.c index 76d99c6e9521..fd5d407b1544 100644 --- a/ext/pcre/php_pcre.c +++ b/ext/pcre/php_pcre.c @@ -517,7 +517,7 @@ static void free_subpats_table(zend_string **subpat_names, uint32_t num_subpats) } /* {{{ static make_subpats_table */ -static zend_string **make_subpats_table(uint32_t name_cnt, pcre_cache_entry *pce) +static zend_string **make_subpats_table(uint32_t name_cnt, const pcre_cache_entry *pce) { uint32_t num_subpats = pce->capture_count + 1; uint32_t name_size, ni = 0; @@ -553,7 +553,7 @@ static zend_string **ensure_subpats_table(uint32_t name_cnt, pcre_cache_entry *p /* {{{ static calculate_unit_length */ /* Calculates the byte length of the next character. Assumes valid UTF-8 for PCRE2_UTF. */ -static zend_always_inline size_t calculate_unit_length(pcre_cache_entry *pce, const char *start) +static zend_always_inline size_t calculate_unit_length(const pcre_cache_entry *pce, const char *start) { size_t unit_len; @@ -1127,7 +1127,7 @@ static void php_do_pcre_match(INTERNAL_FUNCTION_PARAMETERS, bool global) /* {{{ /* }}} */ static zend_always_inline bool is_known_valid_utf8( - zend_string *subject_str, PCRE2_SIZE start_offset) { + const zend_string *subject_str, PCRE2_SIZE start_offset) { if (!ZSTR_IS_VALID_UTF8(subject_str)) { /* We don't know whether the string is valid UTF-8 or not. */ return false; @@ -1609,7 +1609,7 @@ PHPAPI zend_string *php_pcre_replace(zend_string *regex, /* }}} */ /* {{{ php_pcre_replace_impl() */ -PHPAPI zend_string *php_pcre_replace_impl(pcre_cache_entry *pce, zend_string *subject_str, const char *subject, size_t subject_len, zend_string *replace_str, size_t limit, size_t *replace_count) +PHPAPI zend_string *php_pcre_replace_impl(const pcre_cache_entry *pce, zend_string *subject_str, const char *subject, size_t subject_len, zend_string *replace_str, size_t limit, size_t *replace_count) { uint32_t options; /* Execution options */ int count; /* Count of matched subpatterns */ @@ -2073,8 +2073,8 @@ static zend_always_inline zend_string *php_pcre_replace_func(zend_string *regex, } /* {{{ php_pcre_replace_array */ -static zend_string *php_pcre_replace_array(HashTable *regex, - zend_string *replace_str, HashTable *replace_ht, +static zend_string *php_pcre_replace_array(const HashTable *regex, + zend_string *replace_str, const HashTable *replace_ht, zend_string *subject_str, size_t limit, size_t *replace_count) { zval *regex_entry; @@ -2099,7 +2099,7 @@ static zend_string *php_pcre_replace_array(HashTable *regex, tmp_replace_entry_str = NULL; break; } - zval *zv = ZEND_HASH_ELEMENT(replace_ht, replace_idx); + const zval *zv = ZEND_HASH_ELEMENT(replace_ht, replace_idx); replace_idx++; if (Z_TYPE_P(zv) != IS_UNDEF) { replace_entry_str = zval_get_tmp_string(zv, &tmp_replace_entry_str); @@ -2149,8 +2149,8 @@ static zend_string *php_pcre_replace_array(HashTable *regex, /* {{{ php_replace_in_subject */ static zend_always_inline zend_string *php_replace_in_subject( - zend_string *regex_str, HashTable *regex_ht, - zend_string *replace_str, HashTable *replace_ht, + zend_string *regex_str, const HashTable *regex_ht, + zend_string *replace_str, const HashTable *replace_ht, zend_string *subject, size_t limit, size_t *replace_count) { zend_string *result; @@ -2263,8 +2263,8 @@ static size_t php_preg_replace_func_impl(zval *return_value, static void _preg_replace_common( zval *return_value, HashTable *regex_ht, zend_string *regex_str, - HashTable *replace_ht, zend_string *replace_str, - HashTable *subject_ht, zend_string *subject_str, + const HashTable *replace_ht, zend_string *replace_str, + const HashTable *subject_ht, zend_string *subject_str, zend_long limit, zval *zcount, bool is_filter @@ -2552,7 +2552,7 @@ PHP_FUNCTION(preg_split) /* }}} */ /* {{{ php_pcre_split */ -PHPAPI void php_pcre_split_impl(pcre_cache_entry *pce, zend_string *subject_str, zval *return_value, +PHPAPI void php_pcre_split_impl(const pcre_cache_entry *pce, zend_string *subject_str, zval *return_value, zend_long limit_val, zend_long flags) { uint32_t options; /* Execution options */ @@ -2913,7 +2913,7 @@ PHP_FUNCTION(preg_grep) } /* }}} */ -PHPAPI void php_pcre_grep_impl(pcre_cache_entry *pce, zval *input, zval *return_value, zend_long flags) /* {{{ */ +PHPAPI void php_pcre_grep_impl(const pcre_cache_entry *pce, zval *input, zval *return_value, zend_long flags) /* {{{ */ { uint32_t num_subpats; /* Number of captured subpatterns */ int count; /* Count of matched subpatterns */ diff --git a/ext/pcre/php_pcre.h b/ext/pcre/php_pcre.h index ebaa686a31c3..8b104f204972 100644 --- a/ext/pcre/php_pcre.h +++ b/ext/pcre/php_pcre.h @@ -50,13 +50,13 @@ PHPAPI pcre_cache_entry* pcre_get_compiled_regex_cache_ex(zend_string *regex, bo PHPAPI void php_pcre_match_impl(pcre_cache_entry *pce, zend_string *subject_str, zval *return_value, zval *subpats, bool global, zend_long flags, zend_off_t start_offset); -PHPAPI zend_string *php_pcre_replace_impl(pcre_cache_entry *pce, zend_string *subject_str, const char *subject, size_t subject_len, zend_string *replace_str, +PHPAPI zend_string *php_pcre_replace_impl(const pcre_cache_entry *pce, zend_string *subject_str, const char *subject, size_t subject_len, zend_string *replace_str, size_t limit, size_t *replace_count); -PHPAPI void php_pcre_split_impl( pcre_cache_entry *pce, zend_string *subject_str, zval *return_value, +PHPAPI void php_pcre_split_impl(const pcre_cache_entry *pce, zend_string *subject_str, zval *return_value, zend_long limit_val, zend_long flags); -PHPAPI void php_pcre_grep_impl( pcre_cache_entry *pce, zval *input, zval *return_value, +PHPAPI void php_pcre_grep_impl(const pcre_cache_entry *pce, zval *input, zval *return_value, zend_long flags); PHPAPI pcre2_match_context *php_pcre_mctx(void); From fdd47f13baf0bfea2652c8a3ddc3e083d83d73f4 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 1 Sep 2026 20:06:19 +0100 Subject: [PATCH 05/10] pcre: refactor preg_get_backref() --- ext/pcre/php_pcre.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/ext/pcre/php_pcre.c b/ext/pcre/php_pcre.c index fd5d407b1544..088db5d7bef2 100644 --- a/ext/pcre/php_pcre.c +++ b/ext/pcre/php_pcre.c @@ -1514,17 +1514,17 @@ PHP_FUNCTION(preg_match_all) } /* }}} */ -/* {{{ preg_get_backref */ -static int preg_get_backref(char **str, int *backref) +static bool preg_get_backref(char **str, int *backref) { - char in_brace = 0; + bool in_brace = false; char *walk = *str; - if (walk[1] == 0) - return 0; + if (walk[1] == 0) { + return false; + } if (*walk == '$' && walk[1] == '{') { - in_brace = 1; + in_brace = true; walk++; } walk++; @@ -1532,8 +1532,9 @@ static int preg_get_backref(char **str, int *backref) if (*walk >= '0' && *walk <= '9') { *backref = *walk - '0'; walk++; - } else - return 0; + } else { + return false; + } if (*walk && *walk >= '0' && *walk <= '9') { *backref = *backref * 10 + *walk - '0'; @@ -1541,16 +1542,15 @@ static int preg_get_backref(char **str, int *backref) } if (in_brace) { - if (*walk != '}') - return 0; - else - walk++; + if (*walk != '}') { + return false; + } + walk++; } *str = walk; - return 1; + return true; } -/* }}} */ /* Return NULL if an exception has occurred */ static zend_string *preg_do_repl_func(zend_fcall_info *fci, zend_fcall_info_cache *fcc, const char *subject, PCRE2_SIZE *offsets, zend_string **subpat_names, uint32_t num_subpats, int count, const PCRE2_SPTR mark, zend_long flags) From fa60d5022f952e6a7fc9340a7db9e728a181f261 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 1 Sep 2026 20:14:49 +0100 Subject: [PATCH 06/10] pcre: pass subject as zend_string* to preg_do_repl_func() --- ext/pcre/php_pcre.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/pcre/php_pcre.c b/ext/pcre/php_pcre.c index 088db5d7bef2..4c63ab0920ae 100644 --- a/ext/pcre/php_pcre.c +++ b/ext/pcre/php_pcre.c @@ -1553,14 +1553,14 @@ static bool preg_get_backref(char **str, int *backref) } /* Return NULL if an exception has occurred */ -static zend_string *preg_do_repl_func(zend_fcall_info *fci, zend_fcall_info_cache *fcc, const char *subject, PCRE2_SIZE *offsets, zend_string **subpat_names, uint32_t num_subpats, int count, const PCRE2_SPTR mark, zend_long flags) +static zend_string *preg_do_repl_func(zend_fcall_info *fci, zend_fcall_info_cache *fcc, const zend_string *subject, PCRE2_SIZE *offsets, zend_string **subpat_names, uint32_t num_subpats, int count, const PCRE2_SPTR mark, zend_long flags) { zend_string *result_str = NULL; zval retval; /* Function return value */ zval arg; /* Argument to pass to function */ array_init_size(&arg, count + (mark ? 1 : 0)); - populate_subpat_array(Z_ARRVAL(arg), subject, offsets, subpat_names, num_subpats, count, mark, flags); + populate_subpat_array(Z_ARRVAL(arg), ZSTR_VAL(subject), offsets, subpat_names, num_subpats, count, mark, flags); fci->retval = &retval; fci->param_count = 1; @@ -1952,7 +1952,7 @@ static zend_string *php_pcre_replace_func_impl(pcre_cache_entry *pce, zend_strin /* Use custom function to get replacement string and its length. */ zend_string *eval_result = preg_do_repl_func( - fci, fcc, ZSTR_VAL(subject_str), offsets, subpat_names, num_subpats, count, + fci, fcc, subject_str, offsets, subpat_names, num_subpats, count, pcre2_get_mark(match_data), flags); if (UNEXPECTED(eval_result == NULL)) { From eacd1576adb5ec088ebf42ed8a69363a0174826e Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Tue, 22 Sep 2026 13:20:16 -0400 Subject: [PATCH 07/10] ext/session: Preserve exceptions thrown by create_sid() Do not replace pending exceptions with return-value validation errors in the user handler adapter, including exceptions raised while destroying invalid return values. This preserves the original exception for session_start(), session_create_id(), and session_regenerate_id(). Closes GH-23854 --- NEWS | 2 + ext/session/mod_user.c | 8 ++- .../session_create_id_create_sid_throws.phpt | 5 +- ...n_create_sid_return_destructor_throws.phpt | 37 +++++++++++ ...ssion_regenerate_id_create_sid_throws.phpt | 57 +++++++++++++++++ .../session_start_create_sid_throws.phpt | 64 +++++++++++++++++++ ...tart_strict_recreate_create_sid_fails.phpt | 2 +- 7 files changed, 169 insertions(+), 6 deletions(-) create mode 100644 ext/session/tests/user_session_module/session_create_sid_return_destructor_throws.phpt create mode 100644 ext/session/tests/user_session_module/session_regenerate_id_create_sid_throws.phpt create mode 100644 ext/session/tests/user_session_module/session_start_create_sid_throws.phpt diff --git a/NEWS b/NEWS index 6235628cbb17..9021450c1226 100644 --- a/NEWS +++ b/NEWS @@ -71,6 +71,8 @@ PHP NEWS - Session: . Fixed session_start() continuing after a failed create_sid() when session.use_strict_mode rejects the supplied ID. (Ilia Alshanetsky) + . Fixed exceptions from user-defined create_sid() handlers being replaced + by return-value validation errors. (Ilia Alshanetsky) - Sockets: . Fixed socket_select() silently truncating sets larger than FD_SETSIZE on diff --git a/ext/session/mod_user.c b/ext/session/mod_user.c index 71b18612683d..61db72b4cf12 100644 --- a/ext/session/mod_user.c +++ b/ext/session/mod_user.c @@ -237,12 +237,16 @@ PS_CREATE_SID_FUNC(user) } zval_ptr_dtor(&retval); } else { - zend_throw_error(NULL, "No session id returned by function"); + if (!EG(exception)) { + zend_throw_error(NULL, "No session id returned by function"); + } return NULL; } if (!id) { - zend_throw_error(NULL, "Session id must be a string"); + if (!EG(exception)) { + zend_throw_error(NULL, "Session id must be a string"); + } return NULL; } diff --git a/ext/session/tests/user_session_module/session_create_id_create_sid_throws.phpt b/ext/session/tests/user_session_module/session_create_id_create_sid_throws.phpt index b65c0671d940..a8648fdb5d68 100644 --- a/ext/session/tests/user_session_module/session_create_id_create_sid_throws.phpt +++ b/ext/session/tests/user_session_module/session_create_id_create_sid_throws.phpt @@ -36,14 +36,13 @@ try { session_create_id(); } catch (Throwable $e) { echo $e::class, ": ", $e->getMessage(), PHP_EOL; - $previous = $e->getPrevious(); - echo $previous::class, ": ", $previous->getMessage(), PHP_EOL; + var_dump($e->getPrevious()); } var_dump(session_status() === PHP_SESSION_ACTIVE); ?> --EXPECT-- -Error: Session id must be a string Exception: create_sid failed +NULL bool(true) diff --git a/ext/session/tests/user_session_module/session_create_sid_return_destructor_throws.phpt b/ext/session/tests/user_session_module/session_create_sid_return_destructor_throws.phpt new file mode 100644 index 000000000000..18bb97779e00 --- /dev/null +++ b/ext/session/tests/user_session_module/session_create_sid_return_destructor_throws.phpt @@ -0,0 +1,37 @@ +--TEST-- +Exceptions from destruction of an invalid create_sid() return value are preserved +--EXTENSIONS-- +session +--FILE-- +getMessage(), PHP_EOL; +} +?> +--EXPECT-- +RuntimeException: destructor failed diff --git a/ext/session/tests/user_session_module/session_regenerate_id_create_sid_throws.phpt b/ext/session/tests/user_session_module/session_regenerate_id_create_sid_throws.phpt new file mode 100644 index 000000000000..21e796ca13d0 --- /dev/null +++ b/ext/session/tests/user_session_module/session_regenerate_id_create_sid_throws.phpt @@ -0,0 +1,57 @@ +--TEST-- +session_regenerate_id() preserves exceptions from create_sid(), including collision retries +--EXTENSIONS-- +session +--INI-- +session.use_cookies=0 +session.cache_limiter= +session.use_strict_mode=1 +session.gc_probability=0 +--FILE-- +calls === $this->throwAt) { + throw new RuntimeException('create_sid failed'); + } + return 'session' . $this->calls; + } +} + +foreach ([2, 3] as $throwAt) { + $handler = new FailingHandler(); + $handler->throwAt = $throwAt; + session_set_save_handler($handler); + session_id(''); + session_start(); + + try { + session_regenerate_id(); + } catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; + } + echo 'create_sid calls: ', $handler->calls, PHP_EOL; +} +?> +--EXPECT-- +RuntimeException: create_sid failed +create_sid calls: 2 +RuntimeException: create_sid failed +create_sid calls: 3 diff --git a/ext/session/tests/user_session_module/session_start_create_sid_throws.phpt b/ext/session/tests/user_session_module/session_start_create_sid_throws.phpt new file mode 100644 index 000000000000..b7cd663007e0 --- /dev/null +++ b/ext/session/tests/user_session_module/session_start_create_sid_throws.phpt @@ -0,0 +1,64 @@ +--TEST-- +session_start() preserves exceptions from create_sid() +--EXTENSIONS-- +session +--INI-- +session.use_cookies=0 +session.cache_limiter= +session.gc_probability=0 +--FILE-- +invalidReturn) { + return []; + } + throw new RuntimeException('create_sid failed'); + } + + public function validateId(string $id): bool + { + return false; + } +} + +$handler = new FailingHandler(); +session_set_save_handler($handler); + +foreach ([false, true] as $strict) { + foreach ([false, true] as $invalidReturn) { + echo 'strict mode: ', (int) $strict, ', invalid return: ', (int) $invalidReturn, PHP_EOL; + $handler->invalidReturn = $invalidReturn; + session_id($strict ? 'rejected' : ''); + try { + session_start(['use_strict_mode' => $strict]); + } catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; + } + } +} +?> +--EXPECT-- +strict mode: 0, invalid return: 0 +RuntimeException: create_sid failed +strict mode: 0, invalid return: 1 +TypeError: FailingHandler::create_sid(): Return value must be of type string, array returned +strict mode: 1, invalid return: 0 +RuntimeException: create_sid failed +strict mode: 1, invalid return: 1 +TypeError: FailingHandler::create_sid(): Return value must be of type string, array returned diff --git a/ext/session/tests/user_session_module/session_start_strict_recreate_create_sid_fails.phpt b/ext/session/tests/user_session_module/session_start_strict_recreate_create_sid_fails.phpt index 3c3ac1dea327..0ddddaefdbb5 100644 --- a/ext/session/tests/user_session_module/session_start_strict_recreate_create_sid_fails.phpt +++ b/ext/session/tests/user_session_module/session_start_strict_recreate_create_sid_fails.phpt @@ -44,6 +44,6 @@ var_dump(defined('SID')); ?> --EXPECT-- -Error: Session id must be a string +RuntimeException: create_sid failed bool(false) bool(false) From 32e268fd55cadd9922b44f01dab75671176ba2a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 23 Sep 2026 19:55:01 +0200 Subject: [PATCH 08/10] zend: Remove `zend_execute_scripts()` (#23864) This function is unsafe, because it reused the `retval` for each execution, without clearing it in-between each script, leading to memory leaks. Instead of fixing it, we just remove it: Writing a loop yourself is not much more complicated than using this variadic function and provides more flexibility. --- UPGRADING.INTERNALS | 3 +++ Zend/zend.c | 24 ------------------------ Zend/zend_compile.h | 1 - ext/readline/readline_cli.c | 2 +- main/main.c | 4 ++-- sapi/apache2handler/sapi_apache2.c | 2 +- 6 files changed, 7 insertions(+), 29 deletions(-) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index 41432be1e429..b2aa21986efb 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -14,6 +14,9 @@ PHP 8.7 INTERNALS UPGRADE NOTES 1. Internal API changes ======================== +- Removed zend_execute_scripts(). Manually call zend_execute_script() in a loop + instead. + ======================== 2. Build system changes ======================== diff --git a/Zend/zend.c b/Zend/zend.c index 3cfc0ffd2512..3b16dc9c315b 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -1996,30 +1996,6 @@ ZEND_API zend_result zend_execute_script(int type, zval *retval, zend_file_handl return ret; } -ZEND_API zend_result zend_execute_scripts(int type, zval *retval, int file_count, ...) /* {{{ */ -{ - va_list files; - int i; - zend_file_handle *file_handle; - zend_result ret = SUCCESS; - - va_start(files, file_count); - for (i = 0; i < file_count; i++) { - file_handle = va_arg(files, zend_file_handle *); - if (!file_handle) { - continue; - } - if (ret == FAILURE) { - continue; - } - ret = zend_execute_script(type, retval, file_handle); - } - va_end(files); - - return ret; -} -/* }}} */ - #define COMPILED_STRING_DESCRIPTION_FORMAT "%s(%d) : %s" ZEND_API char *zend_make_compiled_string_description(const char *name) /* {{{ */ diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h index 34a91183b2a9..6502770a2662 100644 --- a/Zend/zend_compile.h +++ b/Zend/zend_compile.h @@ -962,7 +962,6 @@ ZEND_API zend_op_array *compile_filename(int type, zend_string *filename); ZEND_API zend_op_array *zend_compile_ast(zend_ast *ast, int type, zend_string *filename); ZEND_API zend_ast *zend_compile_string_to_ast( zend_string *code, struct _zend_arena **ast_arena, zend_string *filename); -ZEND_API zend_result zend_execute_scripts(int type, zval *retval, int file_count, ...); ZEND_API zend_result zend_execute_script(int type, zval *retval, zend_file_handle *file_handle); ZEND_API zend_result open_file_for_scanning(zend_file_handle *file_handle); ZEND_API void init_op_array(zend_op_array *op_array, zend_function_type type, int initial_ops_size); diff --git a/ext/readline/readline_cli.c b/ext/readline/readline_cli.c index 1fe7c9c6b4df..973aea106112 100644 --- a/ext/readline/readline_cli.c +++ b/ext/readline/readline_cli.c @@ -607,7 +607,7 @@ static int readline_shell_run(void) /* {{{ */ zend_file_handle prepend_file; zend_stream_init_filename(&prepend_file, PG(auto_prepend_file)); - zend_execute_scripts(ZEND_REQUIRE, NULL, 1, &prepend_file); + zend_execute_script(ZEND_REQUIRE, NULL, &prepend_file); zend_destroy_file_handle(&prepend_file); } diff --git a/main/main.c b/main/main.c index 0539220de362..7a8d440c0d75 100644 --- a/main/main.c +++ b/main/main.c @@ -2553,7 +2553,7 @@ PHPAPI bool php_execute_script_ex(zend_file_handle *primary_file, zval *retval) } /* Only lookup the real file path and add it to the included_files list if already opened - * otherwise it will get opened and added to the included_files list in zend_execute_scripts + * otherwise it will get opened and added to the included_files list in zend_execute_script */ if (primary_file->filename && !zend_string_equals_literal(primary_file->filename, "Standard input code") && @@ -2654,7 +2654,7 @@ PHPAPI int php_execute_simple_script(zend_file_handle *primary_file, zval *ret) php_ignore_value(VCWD_GETCWD(old_cwd, OLD_CWD_SIZE-1)); VCWD_CHDIR_FILE(ZSTR_VAL(primary_file->filename)); } - zend_execute_scripts(ZEND_REQUIRE, ret, 1, primary_file); + zend_execute_script(ZEND_REQUIRE, ret, primary_file); } zend_end_try(); if (old_cwd[0] != '\0') { diff --git a/sapi/apache2handler/sapi_apache2.c b/sapi/apache2handler/sapi_apache2.c index 83b3f02fb743..72dff2b03a7b 100644 --- a/sapi/apache2handler/sapi_apache2.c +++ b/sapi/apache2handler/sapi_apache2.c @@ -714,7 +714,7 @@ zend_first_try { if (!parent_req) { php_execute_script(&zfd); } else { - zend_execute_scripts(ZEND_INCLUDE, NULL, 1, &zfd); + zend_execute_script(ZEND_INCLUDE, NULL, &zfd); } zend_destroy_file_handle(&zfd); From ddea064a336baa65ffbc3c643866234e410286bd Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Wed, 23 Sep 2026 11:09:23 -0700 Subject: [PATCH 09/10] release-process.md: update list after last element was removed Move the "and" and add a period --- docs/release-process.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/release-process.md b/docs/release-process.md index 55bd45279b30..08a0c0e9f637 100644 --- a/docs/release-process.md +++ b/docs/release-process.md @@ -949,9 +949,9 @@ feature development that cannot go into the new version. * clear the `NEWS`, `UPGRADING`, and `UPGRADING.INTERNALS` files; * update the version numbers in `configure.ac`, `main/php_version.h`, - `Zend/zend.h`, and `win32/build/confutils.js`; + `Zend/zend.h`, and `win32/build/confutils.js`; and * update the API version numbers in `Zend/zend_extensions.h`, - `Zend/zend_modules.h`, and `main/php.h`; and + `Zend/zend_modules.h`, and `main/php.h`. See [Prepare for PHP 8.2][] and [Prepare for PHP 8.2 (bis)][] for an example of what this commit should include. From 8723e6606a5a52ad1aa6d8851c17d980085032b6 Mon Sep 17 00:00:00 2001 From: Marc Bennewitz Date: Wed, 23 Sep 2026 20:06:41 +0100 Subject: [PATCH 10/10] Fix GH-23811: SplFixedArray leak on re-init and broken setSize() zend_object_alloc() zeroes the object, so cached_resize started at 0, which spl_fixedarray_resize() reads as "resize in progress" and returns early. setSize() therefore did nothing on subclasses whose constructor does not call parent::__construct(). Initialise the struct on object creation. setSize(0) clears the array before destroying its elements, so it looks unconstructed to userland. __construct(), __wakeup() and __unserialize() then re-initialised it, and the in-progress clear discarded the buffer they had installed. cb3dc62fd90 fixed the same leak for a re-entrant setSize() by testing the resize sentinel first; apply that test to the other three entry points. Close GH-23812 --- NEWS | 6 ++ ext/spl/spl_fixedarray.c | 27 ++++++-- ..._setSize_destruct_reinit_during_clear.phpt | 68 +++++++++++++++++++ ...ray_subclass_without_parent_construct.phpt | 63 +++++++++++++++++ 4 files changed, 158 insertions(+), 6 deletions(-) create mode 100644 ext/spl/tests/SplFixedArray_setSize_destruct_reinit_during_clear.phpt create mode 100644 ext/spl/tests/SplFixedArray_subclass_without_parent_construct.phpt diff --git a/NEWS b/NEWS index 9021450c1226..ba4baf654613 100644 --- a/NEWS +++ b/NEWS @@ -78,6 +78,12 @@ PHP NEWS . Fixed socket_select() silently truncating sets larger than FD_SETSIZE on Windows. (David Carlier) +- SPL: + . Fixed SplFixedArray::setSize() doing nothing on subclasses whose + constructor does not call parent::__construct(). (Marc Bennewitz) + . Fixed memory leak when __construct(), __wakeup() or __unserialize() is + called from an element destructor during setSize(0). (Marc Bennewitz) + - SQLite: . Fixed a crash when SQLite3::close() is called from a userland callback. (Ilia Alshanetsky) diff --git a/ext/spl/spl_fixedarray.c b/ext/spl/spl_fixedarray.c index 8f8108e2f90b..18025e0cbeb8 100644 --- a/ext/spl/spl_fixedarray.c +++ b/ext/spl/spl_fixedarray.c @@ -77,6 +77,14 @@ static bool spl_fixedarray_empty(spl_fixedarray *array) return true; } +/* True while spl_fixedarray_resize() runs. A clear empties the array before + * destroying its elements, so emptiness alone cannot tell "never constructed" + * from "clear in progress"; re-initialising in that window leaks. */ +static bool spl_fixedarray_resize_in_progress(const spl_fixedarray *array) +{ + return array->cached_resize >= 0; +} + static void spl_fixedarray_default_ctor(spl_fixedarray *array) { array->size = 0; @@ -188,9 +196,9 @@ static void spl_fixedarray_resize(spl_fixedarray *array, zend_long size) /* clearing the array */ if (size == 0) { + /* Clears elements and size; resetting them afterwards would leak + * anything a destructor re-installed. */ spl_fixedarray_dtor(array); - array->elements = NULL; - array->size = 0; } else if (size > array->size) { array->elements = safe_erealloc(array->elements, size, sizeof(zval), 0); spl_fixedarray_init_elems(array, array->size, size); @@ -201,8 +209,12 @@ static void spl_fixedarray_resize(spl_fixedarray *array, zend_long size) array->elements = erealloc(array->elements, sizeof(zval) * size); } - /* If resized within the destructor, take the last resize command and perform it */ + /* If resized within the destructor, take the last resize command and + * perform it. The sentinel is still set: re-initialising during a + * resize is refused. */ zend_long cached_resize = array->cached_resize; + ZEND_ASSERT(cached_resize >= 0); + array->cached_resize = -1; if (cached_resize != size) { spl_fixedarray_resize(array, cached_resize); @@ -285,6 +297,9 @@ static zend_object *spl_fixedarray_object_new_ex(zend_class_entry *class_type, z if (orig && clone_orig) { spl_fixedarray_object *other = spl_fixed_array_from_obj(orig); spl_fixedarray_copy_ctor(&intern->array, &other->array); + } else { + /* The zeroed struct would mean "resizing"; set the sentinel. */ + spl_fixedarray_default_ctor(&intern->array); } while (parent) { @@ -554,7 +569,7 @@ PHP_METHOD(SplFixedArray, __construct) intern = Z_SPLFIXEDARRAY_P(object); - if (!spl_fixedarray_empty(&intern->array)) { + if (UNEXPECTED(!spl_fixedarray_empty(&intern->array) || spl_fixedarray_resize_in_progress(&intern->array))) { /* called __construct() twice, bail out */ return; } @@ -572,7 +587,7 @@ PHP_METHOD(SplFixedArray, __wakeup) RETURN_THROWS(); } - if (intern->array.size == 0) { + if (EXPECTED(intern->array.size == 0 && !spl_fixedarray_resize_in_progress(&intern->array))) { int index = 0; int size = zend_hash_num_elements(intern_ht); @@ -634,7 +649,7 @@ PHP_METHOD(SplFixedArray, __unserialize) RETURN_THROWS(); } - if (intern->array.size == 0) { + if (EXPECTED(intern->array.size == 0 && !spl_fixedarray_resize_in_progress(&intern->array))) { size = zend_hash_num_elements(data); spl_fixedarray_init_non_empty_struct(&intern->array, size); if (!size) { diff --git a/ext/spl/tests/SplFixedArray_setSize_destruct_reinit_during_clear.phpt b/ext/spl/tests/SplFixedArray_setSize_destruct_reinit_during_clear.phpt new file mode 100644 index 000000000000..838bf30c8282 --- /dev/null +++ b/ext/spl/tests/SplFixedArray_setSize_destruct_reinit_during_clear.phpt @@ -0,0 +1,68 @@ +--TEST-- +SplFixedArray::setSize: re-initialising from a destructor during clear (GH-23811) +--DESCRIPTION-- +setSize(0) clears elements and size before running the element destructors, so +the array momentarily looks like it was never constructed. __construct(), +__wakeup() and __unserialize() must not re-initialise it in that window: the +in-progress clear would discard whatever they installed, leaking it. +--FILE-- +setSize(0); + echo "size: ", $arr->getSize(), "\n"; + + /* The array must still be usable. */ + $arr->setSize(1); + $arr[0] = "ok"; + var_dump($arr[0]); + + Reentrant::$arr = null; + Reentrant::$action = null; +} + +echo "-- __construct() --\n"; +clear_with(function ($arr) { $arr->__construct(5); }); + +/* __construct() is ignored, but the following setSize() is still recorded as + * the pending resize and applied once the clear finishes. */ +echo "-- __construct() then setSize() --\n"; +clear_with(function ($arr) { $arr->__construct(7); $arr->setSize(3); }); + +echo "-- __unserialize() --\n"; +clear_with(function ($arr) { $arr->__unserialize(["a", "b", "c"]); }); + +echo "-- __wakeup() --\n"; +clear_with(function ($arr) { @$arr->__wakeup(); }); +?> +--EXPECT-- +-- __construct() -- +size: 0 +string(2) "ok" +-- __construct() then setSize() -- +size: 3 +string(2) "ok" +-- __unserialize() -- +size: 0 +string(2) "ok" +-- __wakeup() -- +size: 0 +string(2) "ok" diff --git a/ext/spl/tests/SplFixedArray_subclass_without_parent_construct.phpt b/ext/spl/tests/SplFixedArray_subclass_without_parent_construct.phpt new file mode 100644 index 000000000000..969661b3dca4 --- /dev/null +++ b/ext/spl/tests/SplFixedArray_subclass_without_parent_construct.phpt @@ -0,0 +1,63 @@ +--TEST-- +SplFixedArray: subclass not calling parent::__construct() (GH-23811) +--DESCRIPTION-- +The internal struct is zeroed on object creation, which used to leave the +"resize in progress" sentinel at 0 instead of -1. setSize() then took the +re-entrancy early return and silently did nothing, leaving the array stuck +at size 0 for the lifetime of the object. +--FILE-- +getSize(), "\n"; + +$a->setSize(3); +echo "after setSize(3): ", $a->getSize(), "\n"; + +$a[0] = "x"; +$a[2] = "z"; +var_dump($a->toArray()); + +$a->setSize(1); +echo "after setSize(1): ", $a->getSize(), "\n"; + +$a->setSize(0); +echo "after setSize(0): ", $a->getSize(), "\n"; + +/* Deferred initialisation: calling the parent constructor later still works. */ +class LateInit extends SplFixedArray { + public function __construct() { + } + public function init(int $size): void { + parent::__construct($size); + } +} +$b = new LateInit(); +$b->init(2); +echo "deferred parent::__construct(2): ", $b->getSize(), "\n"; + +/* Cloning one of these must also yield a resizable array. */ +$c = clone new Unconstructed(); +$c->setSize(2); +echo "clone then setSize(2): ", $c->getSize(), "\n"; +?> +--EXPECT-- +initial: 0 +after setSize(3): 3 +array(3) { + [0]=> + string(1) "x" + [1]=> + NULL + [2]=> + string(1) "z" +} +after setSize(1): 1 +after setSize(0): 0 +deferred parent::__construct(2): 2 +clone then setSize(2): 2