Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ticket/16708] Update the UTF-8 check for multiple encoding settings #6144

Open
wants to merge 2 commits into
base: 3.3.x
Choose a base branch
from
Open

[ticket/16708] Update the UTF-8 check for multiple encoding settings #6144

wants to merge 2 commits into from

Conversation

EA117
Copy link
Contributor

@EA117 EA117 commented Feb 25, 2021

Checklist:

  • Correct branch: master for new features; 3.3.x for fixes
  • Tests pass
  • Code follows coding guidelines: master and 3.3.x
  • Commit follows commit message format

https://tracker.phpbb.com/browse/PHPBB3-16708

Update the mbstring.http_input and mbstring.http_output configuration check, from being the deprecated PHP 5.6.x "PASS, or blank" to now be "UTF-8, or blank".  PHP documentation specifies this configuration should be left blank, in order to inherit default_charset, which phpBB requires to be configured as UTF-8.

For a phpBB customer where mbstring.http_input and mbstring.http_output is set to UTF-8 rather than being left blank, and is a shared hosting configuration that the customer isn't permitted to change, the phpBB ACP warning that "your http_input/http_output configuration is invalid" isn't helpful.

Their setting is UTF-8, which is the encoding http_input/http_output would be using if it had been left blank and allowed to inherit default_charset.  Our "warning" isn't advising the user of a misconfiguration which is causing a phpBB functionality problem.  phpBB is getting exactly the UTF-8 encoding it was intending to get; same as when it was checking whether the setting was left blank, too.

Also update the default_charset test to permit "UTF-8, or blank".  This had been correctly changed to effectively test "UTF-8, or NULL" during PHPBB3-16698, based on code inspection of /vendor/bantu/ini-get-wrapper/src/IniGetWrapper.php which confirmed IniGetWrapper::getString() will return NULL for any setting which does not exist.

However, testing on some PHP platforms shows PHP itself returning an empty string for a parameter which isn't defined in the PHP.INI, and so we receive an empty string instead of NULL.  So the test was updated so that we effectively check for "UTF-8, or blank, or NULL", all of which result in UTF-8 being used.

PHPBB3-16708
phpBB/includes/acp/acp_main.php Outdated Show resolved Hide resolved
Fixed short array syntax, moved testing logic into member helper function.

PHPBB3-16708
@EA117 EA117 requested a review from CHItA February 25, 2021 18:01
@3D-I
Copy link
Contributor

3D-I commented Mar 1, 2021

I am not a fan of this changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants