Hi internals,
I would like to ask for feedback on PR #22489: https://github.com/php/php-src/pull/22489
to make sure that nobody objects this change. If yes, I will write a RFC.
The PR changes how invalid quantity values are handled for the
upload_max_filesize and post_max_size INI settings.
Currently, malformed values may be interpreted through the legacy
quantity parser. For example, upload_max_filesize=1GB may effectively be
interpreted as 1 byte by request_parse_body, while ini_get() still returns the
original string. A completely invalid value can also lead to unclear behavior.
(for example, empty string, which is an undefined behavior)
This makes configuration mistakes hard to diagnose and may result in PHP
enforcing a different limit than the one an administrator expects. (e.g. a 1 byte
upload maximum)
After the proposal, the invalid value is rejected in request_parse_body. And
in ini_get, we emit a warning, and use the defalu value instead.
For example:
upload_max_filesize=1GB
would now emit a warning and keep the default 2M value, instead of being
interpreted as 1. And is rejected with ValueError if parsed by
request_parse_body, instead of being interpreted as "1GB".
Arguably this falls in line with https://wiki.php.net/rfc/policy-exempt-type-value-error-bc-policy
as some other extension INI setting better validate their values. However, as
this might cause bigger impact I am writing this to make sure nobody is
pushing back on this :)
Weilin
Hi,
Hi internals,
I would like to ask for feedback on PR #22489: https://github.com/php/php-src/pull/22489
https://github.com/php/php-src/pull/22489
to make sure that nobody objects this change. If yes, I will write a RFC.The PR changes how invalid quantity values are handled for the
upload_max_filesize and post_max_size INI settings.Currently, malformed values may be interpreted through the legacy
quantity parser. For example, upload_max_filesize=1GB may effectively be
interpreted as 1 byte by request_parse_body, whileini_get()still returns
the
original string. A completely invalid value can also lead to unclear
behavior.
(for example, empty string, which is an undefined behavior)
This makes configuration mistakes hard to diagnose and may result in PHP
enforcing a different limit than the one an administrator expects. (e.g. a
1 byte
upload maximum)After the proposal, the invalid value is rejected in request_parse_body.
And
in ini_get, we emit a warning, and use the defalu value instead.For example:
upload_max_filesize=1GBwould now emit a warning and keep the default 2M value, instead of being
interpreted as 1. And is rejected with ValueError if parsed by
request_parse_body, instead of being interpreted as "1GB".
This doesn't seem to be covered by our BC exceptions (see
https://github.com/php/policies/blob/6ef640f75147c17f160f8a537b42b0a6d1877dc1/release-process.rst#bc-breaks-and-exceptions
) in the policy so technically it should be classified as a BC break but I
think it's more an incomplete wording in the policy as we should probably
treat it in the same way.
Arguably this falls in line with
https://wiki.php.net/rfc/policy-exempt-type-value-error-bc-policy
as some other extension INI setting better validate their values. However,
as
this might cause bigger impact I am writing this to make sure nobody is
pushing back on this :)
I think the best way how to proceed would be to extend the policy RFC and
add this there as well. It would be actually good because we could also
extend it to add output types so this
https://github.com/php/php-src/pull/22538 is not considered a BC break.
Kind regards,
Jakub