From cd80ed6f7bcec1bd96708cba172255856f13a3be Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Mon, 4 Aug 2025 15:58:21 +0200 Subject: [PATCH] Implement changes to GH-17951 according to ML discussion --- Zend/zend_ini.c | 20 ++++++++++++--- main/main.c | 31 ++++++++--------------- tests/basic/gh17951_ini_parse_4.phpt | 4 +-- tests/basic/gh17951_ini_parse_5.phpt | 5 ++-- tests/basic/gh17951_runtime_change_3.phpt | 5 ++-- tests/basic/gh17951_runtime_change_4.phpt | 6 ++--- 6 files changed, 37 insertions(+), 34 deletions(-) diff --git a/Zend/zend_ini.c b/Zend/zend_ini.c index 199ebfb2e9b..689e76794df 100644 --- a/Zend/zend_ini.c +++ b/Zend/zend_ini.c @@ -248,10 +248,16 @@ ZEND_API zend_result zend_register_ini_entries_ex(const zend_ini_entry_def *ini_ zend_unregister_ini_entries_ex(module_number, module_type); return FAILURE; } + + zend_string *prev_value = p->value; + if (((default_value = zend_get_configuration_directive(p->name)) != NULL) && (!p->on_modify || p->on_modify(p, Z_STR_P(default_value), p->mh_arg1, p->mh_arg2, p->mh_arg3, ZEND_INI_STAGE_STARTUP) == SUCCESS)) { - p->value = zend_new_interned_string(zend_string_copy(Z_STR_P(default_value))); + /* Skip assigning the value if the handler has already done so. */ + if (p->value == prev_value) { + p->value = zend_new_interned_string(zend_string_copy(Z_STR_P(default_value))); + } } else { p->value = ini_entry->value ? zend_string_init_interned(ini_entry->value, ini_entry->value_length, 1) : NULL; @@ -389,14 +395,20 @@ ZEND_API zend_result zend_alter_ini_entry_ex(zend_string *name, zend_string *new zend_hash_add_ptr(EG(modified_ini_directives), ini_entry->name, ini_entry); } + zend_string *prev_value = ini_entry->value; duplicate = zend_string_copy(new_value); if (!ini_entry->on_modify || ini_entry->on_modify(ini_entry, duplicate, ini_entry->mh_arg1, ini_entry->mh_arg2, ini_entry->mh_arg3, stage) == SUCCESS) { - if (modified && ini_entry->orig_value != ini_entry->value) { /* we already changed the value, free the changed value */ - zend_string_release(ini_entry->value); + if (modified && ini_entry->orig_value != prev_value) { /* we already changed the value, free the changed value */ + zend_string_release(prev_value); + } + /* Skip assigning the value if the handler has already done so. */ + if (ini_entry->value == prev_value) { + ini_entry->value = duplicate; + } else { + zend_string_release(duplicate); } - ini_entry->value = duplicate; } else { zend_string_release(duplicate); return FAILURE; diff --git a/main/main.c b/main/main.c index 7e4925da685..1cff1cee27a 100644 --- a/main/main.c +++ b/main/main.c @@ -333,30 +333,19 @@ static PHP_INI_MH(OnChangeMemoryLimit) value = Z_L(1)<<30; /* effectively, no limit */ } - /* If max_memory_limit is not set to unlimited, verify change */ - if (PG(max_memory_limit) != -1) { - if (value == -1) { - zend_error( - E_WARNING, - "Failed to set memory_limit to unlimited. memory_limit (currently: " ZEND_LONG_FMT " bytes) cannot be set to unlimited if max_memory_limit (" ZEND_LONG_FMT " bytes) is not unlimited", - PG(memory_limit), - PG(max_memory_limit) - ); - - return FAILURE; + /* If memory_limit exceeds max_memory_limit, warn and set to max_memory_limit instead. */ + if (value > PG(max_memory_limit)) { + if (value != -1) { + zend_error(E_WARNING, + "Failed to set memory_limit to %zd bytes. Setting to max_memory_limit instead (currently: " ZEND_LONG_FMT " bytes)", + value, PG(max_memory_limit)); } - if (value > PG(max_memory_limit)) { - zend_error( - E_WARNING, - "Failed to set memory_limit to %zd bytes. memory_limit (currently: " ZEND_LONG_FMT " bytes) cannot exceed max_memory_limit (" ZEND_LONG_FMT " bytes)", - value, - PG(memory_limit), - PG(max_memory_limit) - ); + zend_ini_entry *max_mem_limit_ini = zend_hash_str_find_ptr(EG(ini_directives), ZEND_STRL("max_memory_limit")); + entry->value = zend_string_copy(max_mem_limit_ini->value); + PG(memory_limit) = PG(max_memory_limit); - return FAILURE; - } + return SUCCESS; } if (zend_set_memory_limit(value) == FAILURE) { diff --git a/tests/basic/gh17951_ini_parse_4.phpt b/tests/basic/gh17951_ini_parse_4.phpt index f29207183a1..5690a133d77 100644 --- a/tests/basic/gh17951_ini_parse_4.phpt +++ b/tests/basic/gh17951_ini_parse_4.phpt @@ -9,7 +9,7 @@ max_memory_limit=128M +--EXPECT-- 128M 128M diff --git a/tests/basic/gh17951_ini_parse_5.phpt b/tests/basic/gh17951_ini_parse_5.phpt index ee028caf9a3..20e41452600 100644 --- a/tests/basic/gh17951_ini_parse_5.phpt +++ b/tests/basic/gh17951_ini_parse_5.phpt @@ -9,7 +9,8 @@ max_memory_limit=128M +--EXPECT-- +Warning: Failed to set memory_limit to 268435456 bytes. Setting to max_memory_limit instead (currently: 134217728 bytes) in Unknown on line 0 128M 128M diff --git a/tests/basic/gh17951_runtime_change_3.phpt b/tests/basic/gh17951_runtime_change_3.phpt index d37c9da9b8e..975caad4ff0 100644 --- a/tests/basic/gh17951_runtime_change_3.phpt +++ b/tests/basic/gh17951_runtime_change_3.phpt @@ -9,6 +9,7 @@ max_memory_limit=512M --EXPECTF-- -Warning: Failed to set memory_limit to %d bytes. memory_limit (currently: %d bytes) cannot exceed max_memory_limit (%d bytes) in %s -128M +Warning: Failed to set memory_limit to 1073741824 bytes. Setting to max_memory_limit instead (currently: 536870912 bytes) in %s on line %d +512M diff --git a/tests/basic/gh17951_runtime_change_4.phpt b/tests/basic/gh17951_runtime_change_4.phpt index 0cbe3bb5c10..3b27e2c91a9 100644 --- a/tests/basic/gh17951_runtime_change_4.phpt +++ b/tests/basic/gh17951_runtime_change_4.phpt @@ -9,6 +9,6 @@ max_memory_limit=512M +--EXPECT-- +512M