fix: php output_buffering setup check allowed values

[pReya]

Both Off and off are valid values for a PHP configuration. Therefore, the Nextcloud setup check should also accept these values. Otherwise, Nextcloud will falsely report a wrong/missing PHP config directive.

See also nextcloud/documentation#478

[kesselb]

Thanks 👍

How do you set the value to off or Off?

php -d output_buffering=On -r "var_dump(ini_get('output_buffering'));"
string(1) "1"

php -d output_buffering=Off -r "var_dump(ini_get('output_buffering'));"
string(0) ""

[pReya]

Thanks 👍

How do you set the value to off or Off?

php -d output_buffering=On -r "var_dump(ini_get('output_buffering'));"
string(1) "1"

php -d output_buffering=Off -r "var_dump(ini_get('output_buffering'));"
string(0) ""

Not sure how it works via the CLI. In my case I just edited the php.ini directly. I'm not sure where/how hosters set their default php.ini, but mine had/has 'Off' and 'On' in it, instead of 0 and 1. Not sure how common this is. But it should work nevertheless.

[pReya]

According to the PHP source it can be either Off, False, No or None. I added all these values to the PR as well.

[kesselb]

According to the PHP source it can be either Off, False, No or None.

https://github.com/php/php-src/blob/bcdb54d47694cd9767e7f2772d6d061e03ba316f/php.ini-production#L55-L56

Nextcloud will falsely report a wrong/missing PHP config directive.

Do you encounter this problem?

A boolean ini value of off will be returned as an empty string or "0" while a boolean ini value of on will be returned as "1". The function can also return the literal string of INI value.

Taken from the ini_get documentation: https://www.php.net/manual/en/function.ini-get.php#refsect1-function.ini-get-notes. In my tests the value was always normalized.

'Off' and 'On'

Just to mention it: "Off" and Off are not the same.

[pReya]

@kesselb Yes, I did have this problem. However, you are right that ini_get() will always normalize the value, so the PR is not useful. Sorry for this.

As you suggested, I fixed my problem by using Off instead of "Off" as a value. However, what I don't understand about this is:

In both cases (Off and "Off"), ini_get('output_buffering') will return 0. But only in the latter case ("Off") will I see the error in the Nextcloud UI. How can Nextcloud know about this, if the error is only triggered by the return value of ini_get(), which is identical in both cases.

[kesselb]

Note: This directive is hardcoded to Off for the CLI SAPI

That's the reason it always worked with CLI ;) I'm not sure what to do now. If output_buffering = "Off" turns the output buffering off the check should also accept the value.

Mind to reopen or create a new PR? Please limit the values to https://github.com/php/php-src/blob/bcdb54d47694cd9767e7f2772d6d061e03ba316f/php.ini-production#L55-L56. true, no or false (=> lowercase) should not work.

[kesselb]

I found some time to have a second look at it. On my development setup the current approach seems correct.

php.ini/user.ini	ini_get
0	0
Off	""
False	""
No	""

A value like "Off" is returned as string literal "Off" which is not a valid value for output_buffering and fallsback to the default value Off. While "Off", "False", "No" or "0" disable output_buffering they are still invalid values. "On", "True", "Yes" or "1" to eanble output_buffering would not work.