[php-src] master: Zend: Revert strict validation of invalid upload_max_filesize and post_max_size values
Weilin Du <[email protected]>
| Newsgroups | gmane.comp.php.cvs.general |
|---|---|
| Message-ID | <[email protected]> |
Author: Weilin Du (LamentXU123)
Date: 2026-06-27T23:33:46+08:00
Commit: https://github.com/php/php-src/commit/a7204686a9a7cb9e1f41a3e307ec092008bf605f
Raw diff: https://github.com/php/php-src/commit/a7204686a9a7cb9e1f41a3e307ec092008bf605f.diff
Zend: Revert strict validation of invalid upload_max_filesize and post_max_size values
998bce0 is accidentally pushed directly to master as this should go to a PR branch.
This commit revert that.
Changed paths:
D Zend/tests/zend_ini/gh14695.phpt
D Zend/tests/zend_ini/gh14695_empty.phpt
M NEWS
M Zend/zend_ini.c
M Zend/zend_ini.h
M ext/standard/http.c
M ext/standard/tests/http/request_parse_body/multipart_options_invalid_quantity.phpt
M main/main.c
Diff:
diff --git a/NEWS b/NEWS
index 5f1d3b951738..9df461452c1c 100644
--- a/NEWS
+++ b/NEWS
@@ -32,8 +32,6 @@ PHP NEWS
surrounding property access). (timwolla)
. Fixed GH-22422 (zend_arena layout mismatch leaked memory in separately
built extensions under AddressSanitizer). (iliaal)
- . Added validation for invalid upload_max_filesize and post_max_size values
- (GH-14695). (Weilin Du)
. TSRM: use local-exec TLS in PIE executables. (henderkes)
. perf: make all static extensions use TSRMG_STATIC. (henderkes)
. Fixed bug GH-22257 (type confusion in Exception::getTraceAsString()).
diff --git a/Zend/tests/zend_ini/gh14695.phpt b/Zend/tests/zend_ini/gh14695.phpt
deleted file mode 100644
index a682c16eff36..000000000000
--- a/Zend/tests/zend_ini/gh14695.phpt
+++ /dev/null
@@ -1,17 +0,0 @@
---TEST--
-GH-14695: Invalid upload_max_filesize and post_max_size values are rejected
---INI--
-upload_max_filesize=1zz
-post_max_size=bogus
---FILE--
-<?php
-
-var_dump(ini_get('upload_max_filesize'));
-var_dump(ini_get('post_max_size'));
-
-?>
---EXPECTF--
-Warning: Invalid "upload_max_filesize" setting. Invalid quantity "1zz": unknown multiplier "z" in %s on line %d
-Warning: Invalid "post_max_size" setting. Invalid quantity "bogus": no valid leading digits in %s on line %d
-string(2) "2M"
-string(2) "8M"
diff --git a/Zend/tests/zend_ini/gh14695_empty.phpt b/Zend/tests/zend_ini/gh14695_empty.phpt
deleted file mode 100644
index 260644122df2..000000000000
--- a/Zend/tests/zend_ini/gh14695_empty.phpt
+++ /dev/null
@@ -1,13 +0,0 @@
---TEST--
-GH-14695: Empty upload_max_filesize value is rejected
---INI--
-upload_max_filesize=
---FILE--
-<?php
-
-var_dump(ini_get('upload_max_filesize'));
-
-?>
---EXPECTF--
-Warning: Invalid "upload_max_filesize" setting. Invalid quantity "": no valid leading digits in %s on line %d
-string(2) "2M"
diff --git a/Zend/zend_ini.c b/Zend/zend_ini.c
index 78a1974a92ea..093683526d31 100644
--- a/Zend/zend_ini.c
+++ b/Zend/zend_ini.c
@@ -614,12 +614,7 @@ static const char *zend_ini_consume_quantity_prefix(const char *const digits, co
return digits_consumed;
}
-static zend_ulong zend_ini_parse_quantity_internal(
- const zend_string *value,
- zend_ini_parse_quantity_signed_result_t signed_result,
- zend_string **errstr,
- bool strict
-) /* {{{ */
+static zend_ulong zend_ini_parse_quantity_internal(const zend_string *value, zend_ini_parse_quantity_signed_result_t signed_result, zend_string **errstr) /* {{{ */
{
char *digits_end = NULL;
const char *str = ZSTR_VAL(value);
@@ -639,18 +634,6 @@ static zend_ulong zend_ini_parse_quantity_internal(
while (digits < str_end && zend_is_whitespace(*(str_end-1))) {--str_end;}
if (digits == str_end) {
- if (strict) {
- if (ZSTR_LEN(value) == 0) {
- *errstr = zend_strpprintf(0, "Invalid quantity \"\": no valid leading digits");
- } else {
- smart_str_append_escaped(&invalid, ZSTR_VAL(value), ZSTR_LEN(value));
- smart_str_0(&invalid);
- *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits",
- ZSTR_VAL(invalid.s));
- smart_str_free(&invalid);
- }
- return 0;
- }
*errstr = NULL;
return 0;
}
@@ -670,13 +653,8 @@ static zend_ulong zend_ini_parse_quantity_internal(
smart_str_append_escaped(&invalid, ZSTR_VAL(value), ZSTR_LEN(value));
smart_str_0(&invalid);
- if (strict) {
- *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits",
- ZSTR_VAL(invalid.s));
- } else {
- *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits, interpreting as \"0\" for backwards compatibility",
- ZSTR_VAL(invalid.s));
- }
+ *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits, interpreting as \"0\" for backwards compatibility",
+ ZSTR_VAL(invalid.s));
smart_str_free(&invalid);
return 0;
@@ -712,12 +690,8 @@ static zend_ulong zend_ini_parse_quantity_internal(
base = 2;
break;
default:
- if (strict) {
- *errstr = zend_strpprintf(0, "Invalid prefix \"0%c\"", digits[1]);
- } else {
- *errstr = zend_strpprintf(0, "Invalid prefix \"0%c\", interpreting as \"0\" for backwards compatibility",
- digits[1]);
- }
+ *errstr = zend_strpprintf(0, "Invalid prefix \"0%c\", interpreting as \"0\" for backwards compatibility",
+ digits[1]);
return 0;
}
digits += 2;
@@ -728,13 +702,8 @@ static zend_ulong zend_ini_parse_quantity_internal(
smart_str_append_escaped(&invalid, ZSTR_VAL(value), ZSTR_LEN(value));
smart_str_0(&invalid);
- if (strict) {
- *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no digits after base prefix",
- ZSTR_VAL(invalid.s));
- } else {
- *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no digits after base prefix, interpreting as \"0\" for backwards compatibility",
- ZSTR_VAL(invalid.s));
- }
+ *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no digits after base prefix, interpreting as \"0\" for backwards compatibility",
+ ZSTR_VAL(invalid.s));
smart_str_free(&invalid);
return 0;
@@ -775,13 +744,8 @@ static zend_ulong zend_ini_parse_quantity_internal(
smart_str_append_escaped(&invalid, ZSTR_VAL(value), ZSTR_LEN(value));
smart_str_0(&invalid);
- if (strict) {
- *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits",
- ZSTR_VAL(invalid.s));
- } else {
- *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits, interpreting as \"0\" for backwards compatibility",
- ZSTR_VAL(invalid.s));
- }
+ *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits, interpreting as \"0\" for backwards compatibility",
+ ZSTR_VAL(invalid.s));
smart_str_free(&invalid);
return 0;
@@ -817,13 +781,8 @@ static zend_ulong zend_ini_parse_quantity_internal(
smart_str_append_escaped(&chr, str_end-1, 1);
smart_str_0(&chr);
- if (strict) {
- *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": unknown multiplier \"%s\"",
- ZSTR_VAL(invalid.s), ZSTR_VAL(chr.s));
- } else {
- *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": unknown multiplier \"%s\", interpreting as \"%s\" for backwards compatibility",
- ZSTR_VAL(invalid.s), ZSTR_VAL(chr.s), ZSTR_VAL(interpreted.s));
- }
+ *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": unknown multiplier \"%s\", interpreting as \"%s\" for backwards compatibility",
+ ZSTR_VAL(invalid.s), ZSTR_VAL(chr.s), ZSTR_VAL(interpreted.s));
smart_str_free(&invalid);
smart_str_free(&interpreted);
@@ -856,13 +815,8 @@ static zend_ulong zend_ini_parse_quantity_internal(
smart_str_append_escaped(&chr, str_end-1, 1);
smart_str_0(&chr);
- if (strict) {
- *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": invalid characters before multiplier \"%s\"",
- ZSTR_VAL(invalid.s), ZSTR_VAL(chr.s));
- } else {
- *errstr = zend_strpprintf(0, "Invalid quantity \"%s\", interpreting as \"%s%s\" for backwards compatibility",
- ZSTR_VAL(invalid.s), ZSTR_VAL(interpreted.s), ZSTR_VAL(chr.s));
- }
+ *errstr = zend_strpprintf(0, "Invalid quantity \"%s\", interpreting as \"%s%s\" for backwards compatibility",
+ ZSTR_VAL(invalid.s), ZSTR_VAL(interpreted.s), ZSTR_VAL(chr.s));
smart_str_free(&invalid);
smart_str_free(&interpreted);
@@ -879,13 +833,8 @@ static zend_ulong zend_ini_parse_quantity_internal(
/* Not specifying the resulting value here because the caller may make
* additional conversions. Not specifying the allowed range
* because the caller may do narrower range checks. */
- if (strict) {
- *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": value is out of range",
- ZSTR_VAL(invalid.s));
- } else {
- *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": value is out of range, using overflow result for backwards compatibility",
- ZSTR_VAL(invalid.s));
- }
+ *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": value is out of range, using overflow result for backwards compatibility",
+ ZSTR_VAL(invalid.s));
smart_str_free(&invalid);
smart_str_free(&interpreted);
@@ -901,25 +850,13 @@ static zend_ulong zend_ini_parse_quantity_internal(
ZEND_API zend_long zend_ini_parse_quantity(const zend_string *value, zend_string **errstr) /* {{{ */
{
- return (zend_long) zend_ini_parse_quantity_internal(value, ZEND_INI_PARSE_QUANTITY_SIGNED, errstr, false);
+ return (zend_long) zend_ini_parse_quantity_internal(value, ZEND_INI_PARSE_QUANTITY_SIGNED, errstr);
}
/* }}} */
ZEND_API zend_ulong zend_ini_parse_uquantity(const zend_string *value, zend_string **errstr) /* {{{ */
{
- return zend_ini_parse_quantity_internal(value, ZEND_INI_PARSE_QUANTITY_UNSIGNED, errstr, false);
-}
-/* }}} */
-
-ZEND_API zend_result zend_ini_parse_quantity_strict(const zend_string *value, zend_long *result, zend_string **errstr) /* {{{ */
-{
- zend_long retval = (zend_long) zend_ini_parse_quantity_internal(value, ZEND_INI_PARSE_QUANTITY_SIGNED, errstr, true);
- if (*errstr) {
- return FAILURE;
- }
-
- *result = retval;
- return SUCCESS;
+ return zend_ini_parse_quantity_internal(value, ZEND_INI_PARSE_QUANTITY_UNSIGNED, errstr);
}
/* }}} */
@@ -1037,23 +974,6 @@ ZEND_API ZEND_INI_MH(OnUpdateLong) /* {{{ */
}
/* }}} */
-ZEND_API ZEND_INI_MH(OnUpdateLongStrict) /* {{{ */
-{
- zend_long tmp;
- zend_string *errstr;
- if (UNEXPECTED(zend_ini_parse_quantity_strict(new_value, &tmp, &errstr) == FAILURE)) {
- zend_error(E_WARNING, "Invalid \"%s\" setting. %s", ZSTR_VAL(entry->name), ZSTR_VAL(errstr));
- zend_string_release(errstr);
- return FAILURE;
- }
-
- zend_long *p = ZEND_INI_GET_ADDR();
- *p = tmp;
-
- return SUCCESS;
-}
-/* }}} */
-
ZEND_API ZEND_INI_MH(OnUpdateLongGEZero) /* {{{ */
{
zend_long tmp = zend_ini_parse_quantity_warn(new_value, entry->name);
diff --git a/Zend/zend_ini.h b/Zend/zend_ini.h
index a181048ad9ab..dbe650675b66 100644
--- a/Zend/zend_ini.h
+++ b/Zend/zend_ini.h
@@ -142,12 +142,6 @@ ZEND_API zend_long zend_ini_parse_quantity(const zend_string *value, zend_string
*/
ZEND_API zend_ulong zend_ini_parse_uquantity(const zend_string *value, zend_string **errstr);
-/**
- * Strict variant of zend_ini_parse_quantity. Ill-formatted values fail instead
- * of returning a backwards-compatible interpretation.
- */
-ZEND_API zend_result zend_ini_parse_quantity_strict(const zend_string *value, zend_long *result, zend_string **errstr);
-
ZEND_API zend_long zend_ini_parse_quantity_warn(const zend_string *value, zend_string *setting);
ZEND_API zend_ulong zend_ini_parse_uquantity_warn(const zend_string *value, zend_string *setting);
@@ -213,7 +207,6 @@ END_EXTERN_C()
BEGIN_EXTERN_C()
ZEND_API ZEND_INI_MH(OnUpdateBool);
ZEND_API ZEND_INI_MH(OnUpdateLong);
-ZEND_API ZEND_INI_MH(OnUpdateLongStrict);
ZEND_API ZEND_INI_MH(OnUpdateLongGEZero);
ZEND_API ZEND_INI_MH(OnUpdateReal);
/* char* versions */
diff --git a/ext/standard/http.c b/ext/standard/http.c
index 667eddaf0be0..d65e7a8acaae 100644
--- a/ext/standard/http.c
+++ b/ext/standard/http.c
@@ -247,17 +247,17 @@ PHP_FUNCTION(http_build_query)
}
/* }}} */
-static zend_result cache_request_parse_body_option(zval *option, const char *option_name, int cache_offset)
+static zend_result cache_request_parse_body_option(HashTable *options, zval *option, int cache_offset)
{
if (option) {
zend_long result;
ZVAL_DEREF(option);
if (Z_TYPE_P(option) == IS_STRING) {
zend_string *errstr;
- if (UNEXPECTED(zend_ini_parse_quantity_strict(Z_STR_P(option), &result, &errstr) == FAILURE)) {
- zend_value_error("Invalid \"%s\" value in $options argument: %s", option_name, ZSTR_VAL(errstr));
+ result = zend_ini_parse_quantity(Z_STR_P(option), &errstr);
+ if (errstr) {
+ zend_error(E_WARNING, "%s", ZSTR_VAL(errstr));
zend_string_release(errstr);
- return FAILURE;
}
} else if (Z_TYPE_P(option) == IS_LONG) {
result = Z_LVAL_P(option);
@@ -290,7 +290,7 @@ static zend_result cache_request_parse_body_options(HashTable *options)
#define CHECK_OPTION(name) \
if (zend_string_equals_literal_ci(key, #name)) { \
- if (cache_request_parse_body_option(value, #name, REQUEST_PARSE_BODY_OPTION_ ## name) == FAILURE) { \
+ if (cache_request_parse_body_option(options, value, REQUEST_PARSE_BODY_OPTION_ ## name) == FAILURE) { \
return FAILURE; \
} \
continue; \
diff --git a/ext/standard/tests/http/request_parse_body/multipart_options_invalid_quantity.phpt b/ext/standard/tests/http/request_parse_body/multipart_options_invalid_quantity.phpt
index 63514f597b51..b1efa0dbc91a 100644
--- a/ext/standard/tests/http/request_parse_body/multipart_options_invalid_quantity.phpt
+++ b/ext/standard/tests/http/request_parse_body/multipart_options_invalid_quantity.phpt
@@ -3,17 +3,15 @@ request_parse_body() invalid quantity
--FILE--
<?php
-foreach (['1GB', ''] as $value) {
- try {
- request_parse_body(options: [
- 'upload_max_filesize' => $value,
- ]);
- } catch (Throwable $e) {
- echo get_class($e), ': ', $e->getMessage(), "\n";
- }
+try {
+ request_parse_body(options: [
+ 'upload_max_filesize' => '1GB',
+ ]);
+} catch (Throwable $e) {
+ echo get_class($e), ': ', $e->getMessage(), "\n";
}
?>
--EXPECTF--
-ValueError: Invalid "upload_max_filesize" value in $options argument: Invalid quantity "1GB": unknown multiplier "B"
-ValueError: Invalid "upload_max_filesize" value in $options argument: Invalid quantity "": no valid leading digits
+Warning: Invalid quantity "1GB": unknown multiplier "B", interpreting as "1" for backwards compatibility in %s on line %d
+RequestParseBodyException: Request does not provide a content type
diff --git a/main/main.c b/main/main.c
index 15bed5be9880..6bda55ac8746 100644
--- a/main/main.c
+++ b/main/main.c
@@ -845,8 +845,8 @@ PHP_INI_BEGIN()
STD_PHP_INI_ENTRY("open_basedir", NULL, PHP_INI_ALL, OnUpdateBaseDir, open_basedir, php_core_globals, core_globals)
STD_PHP_INI_BOOLEAN("file_uploads", "1", PHP_INI_SYSTEM, OnUpdateBool, file_uploads, php_core_globals, core_globals)
- STD_PHP_INI_ENTRY("upload_max_filesize", "2M", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLongStrict, upload_max_filesize, php_core_globals, core_globals)
- STD_PHP_INI_ENTRY("post_max_size", "8M", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLongStrict, post_max_size, sapi_globals_struct,sapi_globals)
+ STD_PHP_INI_ENTRY("upload_max_filesize", "2M", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLong, upload_max_filesize, php_core_globals, core_globals)
+ STD_PHP_INI_ENTRY("post_max_size", "8M", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLong, post_max_size, sapi_globals_struct,sapi_globals)
STD_PHP_INI_ENTRY("upload_tmp_dir", NULL, PHP_INI_SYSTEM, OnUpdateStringUnempty, upload_tmp_dir, php_core_globals, core_globals)
STD_PHP_INI_ENTRY("max_input_nesting_level", "64", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLongGEZero, max_input_nesting_level, php_core_globals, core_globals)
STD_PHP_INI_ENTRY("max_input_vars", "1000", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLongGEZero, max_input_vars, php_core_globals, core_globals)