Skip to content

Fix GH-13815: mb_trim() inaccurate $characters default value #13820

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

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Mar 27, 2024

Because the default characters are defined in the stub file, and the stub file is UTF-8 (typically), the characters are encoded in the string as UTF-8. When using a different character encoding, there is a mismatch between what mb_trim expects and the UTF-8 encoded string it gets.

One way of solving this is by making the characters argument nullable, which would mean that it always uses the internal code path that has the unicode codepoints that are defaulted actually stored as codepoint numbers instead of in a string.

Co-authored-by: @ranvis for the idea + test

Note: this is a proof-of-concept, not a commitment to this change. It must be discussed on the mailing list.

Because the default characters are defined in the stub file, and the
stub file is UTF-8 (typically), the characters are encoded in the string
as UTF-8. When using a different character encoding, there is a mismatch
between what mb_trim expects and the UTF-8 encoded string it gets.

One way of solving this is by making the characters argument nullable,
which would mean that it always uses the internal code path that has the
unicode codepoints that are defaulted actually stored as codepoint
numbers instead of in a string.

Co-authored-by: @ranvis
@youkidearitai
Copy link
Contributor

I talked to internals and I'm sure it's null.
I fixed original an RFC(https://wiki.php.net/rfc/mb_trim).

@ranvis
Copy link
Contributor

ranvis commented Apr 15, 2024

I humbly suggest waiting a little longer for responses. It's almost two weeks, but not yet.
8.4 won't be released in a few months, so...

@youkidearitai
Copy link
Contributor

This matter is being poked at behind the scenes from my email.

@nielsdos
Copy link
Member Author

It's been 18 days since @youkidearitai email and everyone so far agrees this needs no RFC. Last call for comments here until I proceed.

@nielsdos nielsdos marked this pull request as ready for review April 22, 2024 06:16
@youkidearitai
Copy link
Contributor

LGTM.

@nielsdos nielsdos changed the title [PoC] Fix GH-13815: mb_trim() inaccurate $characters default value Fix GH-13815: mb_trim() inaccurate $characters default value Apr 24, 2024
@nielsdos nielsdos merged commit f813708 into php:master Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants