From: Christoph M. Becker <cmbecker69@gmx.de>
Date: Tue, 2 Jun 2020 12:58:26 +0000 (+0200)
Subject: Fix #79665: ini_get() and opcache_get_configuration() inconsistency
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d23cd354c04651f740b1ddc8e040a4a33c586d8a;p=php

Fix #79665: ini_get() and opcache_get_configuration() inconsistency

Overriding the given INI values in modifier callbacks is not possible,
so instead of enforcing "normalized" internal values, we just reject
the attempted changes.
---

diff --git a/NEWS b/NEWS
index 03b5d7415e..9ceeaae530 100644
--- a/NEWS
+++ b/NEWS
@@ -95,6 +95,8 @@ PHP                                                                        NEWS
   . Fixed bug #78654 (Incorrectly computed opcache checksum on files with
     non-ascii characters). (mhagstrand)
   . Fixed bug #76535 (Opcache does not replay compile-time warnings). (Nikita)
+  . Fixed bug #79665 (ini_get() and opcache_get_configuration() inconsistency).
+    (cmb)
 
 - PCRE:
   . Don't ignore invalid escape sequences. (sjon)
diff --git a/ext/opcache/tests/bug79665.phpt b/ext/opcache/tests/bug79665.phpt
new file mode 100644
index 0000000000..153f63740c
--- /dev/null
+++ b/ext/opcache/tests/bug79665.phpt
@@ -0,0 +1,27 @@
+--TEST--
+Bug #79665 (ini_get() and opcache_get_configuration() may be inconsistent)
+--SKIPIF--
+<?php
+require __DIR__ . '/skipif.inc';
+?>
+--INI--
+opcache.max_wasted_percentage=60
+opcache.memory_consumption=7
+opcache.max_accelerated_files=10
+--FILE--
+<?php
+$config = opcache_get_configuration();
+var_dump(ini_get('opcache.max_wasted_percentage'));
+var_dump($config['directives']['opcache.max_wasted_percentage']);
+var_dump(ini_get('opcache.memory_consumption'));
+var_dump($config['directives']['opcache.memory_consumption']);
+var_dump(ini_get('opcache.max_accelerated_files'));
+var_dump($config['directives']['opcache.max_accelerated_files']);
+?>
+--EXPECT--
+string(1) "5"
+float(0.05)
+string(3) "128"
+int(134217728)
+string(5) "10000"
+int(10000)
diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c
index dd03e8170a..2b0db1d18f 100644
--- a/ext/opcache/zend_accelerator_module.c
+++ b/ext/opcache/zend_accelerator_module.c
@@ -67,20 +67,8 @@ static ZEND_INI_MH(OnUpdateMemoryConsumption)
 	zend_long memsize = atoi(ZSTR_VAL(new_value));
 	/* sanity check we must use at least 8 MB */
 	if (memsize < 8) {
-		const char *new_new_value = "8";
-		zend_ini_entry *ini_entry;
-
-		memsize = 8;
 		zend_accel_error(ACCEL_LOG_WARNING, "opcache.memory_consumption is set below the required 8MB.\n");
-		zend_accel_error(ACCEL_LOG_WARNING, ACCELERATOR_PRODUCT_NAME " will use the minimal 8MB configuration.\n");
-
-		if ((ini_entry = zend_hash_str_find_ptr(EG(ini_directives),
-					"opcache.memory_consumption",
-					sizeof("opcache.memory_consumption")-1)) == NULL) {
-			return FAILURE;
-		}
-
-		ini_entry->value = zend_string_init_interned(new_new_value, 1, 1);
+		return FAILURE;
 	}
 	if (UNEXPECTED(memsize > ZEND_ULONG_MAX / (1024 * 1024))) {
 		*p = ZEND_ULONG_MAX;
@@ -95,29 +83,13 @@ static ZEND_INI_MH(OnUpdateMaxAcceleratedFiles)
 	zend_long *p = (zend_long *) ZEND_INI_GET_ADDR();
 	zend_long size = atoi(ZSTR_VAL(new_value));
 	/* sanity check we must use a value between MIN_ACCEL_FILES and MAX_ACCEL_FILES */
-
-	if (size < MIN_ACCEL_FILES || size > MAX_ACCEL_FILES) {
-		const char *new_new_value;
-		zend_ini_entry *ini_entry;
-
-		if (size < MIN_ACCEL_FILES) {
-			size = MIN_ACCEL_FILES;
-			new_new_value = TOKENTOSTR(MIN_ACCEL_FILES);
-			zend_accel_error(ACCEL_LOG_WARNING, "opcache.max_accelerated_files is set below the required minimum (%d).\n", MIN_ACCEL_FILES);
-			zend_accel_error(ACCEL_LOG_WARNING, ACCELERATOR_PRODUCT_NAME " will use the minimal configuration.\n");
-		}
-		if (size > MAX_ACCEL_FILES) {
-			size = MAX_ACCEL_FILES;
-			new_new_value = TOKENTOSTR(MAX_ACCEL_FILES);
-			zend_accel_error(ACCEL_LOG_WARNING, "opcache.max_accelerated_files is set above the limit (%d).\n", MAX_ACCEL_FILES);
-			zend_accel_error(ACCEL_LOG_WARNING, ACCELERATOR_PRODUCT_NAME " will use the maximal configuration.\n");
-		}
-		if ((ini_entry = zend_hash_str_find_ptr(EG(ini_directives),
-					"opcache.max_accelerated_files",
-					sizeof("opcache.max_accelerated_files")-1)) == NULL) {
-			return FAILURE;
-		}
-		ini_entry->value = zend_string_init_interned(new_new_value, strlen(new_new_value), 1);
+	if (size < MIN_ACCEL_FILES) {
+		zend_accel_error(ACCEL_LOG_WARNING, "opcache.max_accelerated_files is set below the required minimum (%d).\n", MIN_ACCEL_FILES);
+		return FAILURE;
+	}
+	if (size > MAX_ACCEL_FILES) {
+		zend_accel_error(ACCEL_LOG_WARNING, "opcache.max_accelerated_files is set above the limit (%d).\n", MAX_ACCEL_FILES);
+		return FAILURE;
 	}
 	*p = size;
 	return SUCCESS;
@@ -129,18 +101,8 @@ static ZEND_INI_MH(OnUpdateMaxWastedPercentage)
 	zend_long percentage = atoi(ZSTR_VAL(new_value));
 
 	if (percentage <= 0 || percentage > 50) {
-		const char *new_new_value = "5";
-		zend_ini_entry *ini_entry;
-
-		percentage = 5;
 		zend_accel_error(ACCEL_LOG_WARNING, "opcache.max_wasted_percentage must be set between 1 and 50.\n");
-		zend_accel_error(ACCEL_LOG_WARNING, ACCELERATOR_PRODUCT_NAME " will use 5%%.\n");
-		if ((ini_entry = zend_hash_str_find_ptr(EG(ini_directives),
-					"opcache.max_wasted_percentage",
-					sizeof("opcache.max_wasted_percentage")-1)) == NULL) {
-			return FAILURE;
-		}
-		ini_entry->value = zend_string_init_interned(new_new_value, strlen(new_new_value), 1);
+		return FAILURE;
 	}
 	*p = (double)percentage / 100.0;
 	return SUCCESS;