General: Replace core's remaining utf8_encode() calls with wp_scrub_utf8() - #12920
General: Replace core's remaining utf8_encode() calls with wp_scrub_utf8()#12920itzmekhokan wants to merge 2 commits into
utf8_encode() calls with wp_scrub_utf8()#12920Conversation
…b_utf8()`.
`wxr_cdata()` and `wp_read_image_metadata()` hold the last three calls to
`utf8_encode()` in core. That function was deprecated in PHP 8.2, is removed in
PHP 9.0, and core itself polyfilled it with a deprecation notice in [60950], so
these sites emit a deprecation notice on every affected export and image upload.
All three used the `if ( ! wp_is_valid_utf8( $text ) ) { utf8_encode( $text ); }`
pattern, which assumes any text failing UTF-8 validation is ISO-8859-1 and
re-encodes the raw bytes on that assumption. That guess is wrong for every other
single-byte encoding and silently produces mojibake. Replacing the invalid spans
with the Unicode replacement character neutralizes the corruption without
inventing an encoding, using the UTF-8 pipeline core added in 6.9.
Adds regression tests covering the WXR export and IPTC metadata that carry
invalid UTF-8.
See #65828, #55603.
dmsnell
left a comment
dmsnell
left a comment
There was a problem hiding this comment.
without commenting on the change itself I have left a few code-level notes.
| * Ensures the WXR export neutralizes invalid UTF-8 instead of reinterpreting it as ISO-8859-1. | ||
| * | ||
| * `wxr_cdata()` previously called the deprecated `utf8_encode()`, which assumed any | ||
| * string that failed UTF-8 validation was ISO-8859-1 and re-encoded the raw bytes on |
There was a problem hiding this comment.
see note below, but noting in a comment how code used to work can be interesting, but focuses on code that no longer exists.
| 'Never-valid byte' => array( "a\xC0b", "a\u{FFFD}b" ), | ||
| 'Truncated sequence' => array( "a\xE2\x9Cb", "a\u{FFFD}b" ), | ||
| 'Overlong sequence' => array( "a\xC1\xBFb", "a\u{FFFD}\u{FFFD}b" ), | ||
| 'Surrogate half' => array( "a\xED\xA0\x80b", "a\u{FFFD}\u{FFFD}\u{FFFD}b" ), |
There was a problem hiding this comment.
these tests are asserting invalid UTF-8 as we tend to think about it, but the noted behavioral change is about alternate input encodings, none of which are demonstrated in these tests.
something that can be clearer than leaving a comment about previous bugs is to leave tests demonstrating the current behaviors. i.e. drop the comment “previously” and add test cases demonstrating the behavior in the presence of alternative inputs.
'Non-UTF-8-compatible input' => array(
mb_convert_encoding( 'wyróżnij', 'ISO-8859-2', 'UTF-8' ),
"wyr\u{FFFD}nij"
)| } | ||
|
|
||
| foreach ( $meta['keywords'] as $key => $keyword ) { | ||
| if ( ! wp_is_valid_utf8( $keyword ) ) { |
There was a problem hiding this comment.
there’s no value in calling wp_is_valid_utf8() before calling wp_scrub_utf8() as the latter does the former internally.
- `wp_scrub_utf8()` validates internally, so the preceding `wp_is_valid_utf8()` checks were redundant. The `$meta[ $key ]` truthiness check stays, as `iso` defaults to int `0`. - Test docblocks describe current behaviour rather than narrating the code that was removed. - Adds data-provider cases for text in ISO-8859-1, ISO-8859-2, Windows-1251 and Windows-1252, which exercise the actual behavioural change; the previous cases only covered malformed UTF-8. See #65828.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
2 participants
wxr_cdata()andwp_read_image_metadata()hold the last three calls toutf8_encode()in core, which PHP deprecated in 8.2 and removes in 9.0, and which core itself polyfilled with a deprecation notice in [60950].All three used the
if ( ! wp_is_valid_utf8( $text ) ) { utf8_encode( $text ); }pattern, which assumes anything failing UTF-8 validation is ISO-8859-1 and re-encodes the raw bytes on that guess — wrong for every other single-byte encoding, and silent. This replaces the invalid spans with the Unicode replacement character viawp_scrub_utf8(), per @dmsnell's guidance in #55603 comment:99 that core should not assume an encoding it cannot know.This is a deliberate behaviour change: invalid bytes become U+FFFD rather than latin1 mojibake. No new API, and the
wp_is_valid_utf8()guards are unchanged.Against unpatched trunk the added tests produce 1 error (
Function utf8_encode() is deprecatedatimage.php:1052) and 5 failures. With the patch: 17/17 pass across the two touched test files, 1178/1178 in--group image --group unicode, and PHPCS reports no new errors or warnings.Trac ticket: https://core.trac.wordpress.org/ticket/65828
Use of AI Tools
AI assistance: Yes
Tool(s): Claude
Model(s): Opus 5
Used for: initial exploration, tests andTicket and PR details. All changes are reviewed and validated by me.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.