Implement changes to GH-17951 according to ML discussion

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 */
+		if (modified && ini_entry->orig_value != prev_value) { /* we already changed the value, free the changed value */
-			zend_string_release(ini_entry->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;

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 memory_limit exceeds max_memory_limit, warn and set to max_memory_limit instead. */
-	if (PG(max_memory_limit) != -1) {
+	if (value > PG(max_memory_limit)) {
-		if (value == -1) {
+		if (value != -1) {
-			zend_error(
+			zend_error(E_WARNING,
-				E_WARNING,
+				"Failed to set memory_limit to %zd bytes. Setting to max_memory_limit instead (currently: " ZEND_LONG_FMT " bytes)",
-				"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",
+				value, PG(max_memory_limit));
-				PG(memory_limit),
-				PG(max_memory_limit)
-			);
-
-			return FAILURE;
 		}
 
-		if (value > PG(max_memory_limit)) {
+		zend_ini_entry *max_mem_limit_ini = zend_hash_str_find_ptr(EG(ini_directives), ZEND_STRL("max_memory_limit"));
-			zend_error(
+		entry->value = zend_string_copy(max_mem_limit_ini->value);
-				E_WARNING,
+		PG(memory_limit) = PG(max_memory_limit);
-				"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)
-			);
 
-			return FAILURE;
+		return SUCCESS;
-		}
 	}
 
 	if (zend_set_memory_limit(value) == FAILURE) {

tests/basic/gh17951_ini_parse_4.phpt

@@ -9,7 +9,7 @@ max_memory_limit=128M
 <?php
 echo ini_get('max_memory_limit') . PHP_EOL;
 echo ini_get('memory_limit') . PHP_EOL;
---EXPECTF--
+?>
-Warning: Failed to set memory_limit to unlimited. memory_limit (currently: %d bytes) cannot be set to unlimited if max_memory_limit (%d bytes) is not unlimited in %s
+--EXPECT--
 128M
 128M

tests/basic/gh17951_ini_parse_5.phpt

@@ -9,7 +9,8 @@ max_memory_limit=128M
 <?php
 echo ini_get('max_memory_limit') . PHP_EOL;
 echo ini_get('memory_limit') . PHP_EOL;
---EXPECTF--
+?>
-Warning: Failed to set memory_limit to %d bytes. memory_limit (currently: %d bytes) cannot exceed max_memory_limit (%d bytes) in %s
+--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

tests/basic/gh17951_runtime_change_3.phpt

@@ -9,6 +9,7 @@ max_memory_limit=512M
 <?php
 ini_set('memory_limit', '1024M');
 echo ini_get('memory_limit');
+?>
 --EXPECTF--
-Warning: Failed to set memory_limit to %d bytes. memory_limit (currently: %d bytes) cannot exceed max_memory_limit (%d bytes) in %s
+Warning: Failed to set memory_limit to 1073741824 bytes. Setting to max_memory_limit instead (currently: 536870912 bytes) in %s on line %d
-128M
+512M

tests/basic/gh17951_runtime_change_4.phpt

@@ -9,6 +9,6 @@ max_memory_limit=512M
 <?php
 ini_set('memory_limit', '-1');
 echo ini_get('memory_limit');
---EXPECTF--
+?>
-Warning: Failed to set memory_limit to unlimited. memory_limit (currently: %d bytes) cannot be set to unlimited if max_memory_limit (%d bytes) is not unlimited in %s
+--EXPECT--
-128M
+512M