#38044 (Make seems_utf8() RFC 3629 compliant.) – WordPress Trac

#1 @desrosj
7 years ago

Keywords needs-patch added
Milestone changed from Awaiting Review to Future Release

Related: #29717.

This ticket was mentioned in PR #7463 on WordPress/wordpress-develop by @debarghyabanerjee.

14 months ago #2

Keywords has-patch added; needs-patch removed

Trac Ticket: Core-38044

## Overview

This pull request introduces the seems_utf8 function, which validates UTF-8 encoded strings according to the specifications outlined in RFC 3629. This implementation ensures that only valid UTF-8 sequences are accepted, effectively safeguarding applications against invalid input.

## Key Features:

UTF-8 Encoding Compliance:

The function adheres strictly to the UTF-8 encoding rules defined in RFC 3629, which allows for a maximum of 4 bytes per character.

Handling of Single and Multi-byte Sequences:

It correctly identifies and processes single-byte (0x00 to 0x7F) and multi-byte sequences (2 to 4 bytes), ensuring that each byte in a multi-byte sequence begins with the appropriate prefix.

Validation of Leading Bytes:

The function checks leading bytes to determine the number of continuation bytes required:

0xC0 for 2-byte sequences
0xE0 for 3-byte sequences
0xF0 for 4-byte sequences

It explicitly rejects any leading bytes starting with 0xF8 or 0xFC, as these indicate sequences that exceed the valid UTF-8 range.

Control Over Overlong Sequences:

The function rejects overlong sequences, ensuring that the encoding does not use more bytes than necessary to represent a character, thereby preventing potential security issues.

Surrogate Pair Handling:

It prevents the inclusion of invalid surrogate pairs (U+D800 to U+DFFF) in the encoded string, in compliance with the restrictions specified in RFC 3629.

Zero Byte Validation:

The function checks for invalid overlong sequences specifically for U+0000, adhering to best practices for UTF-8 validation.

Comprehensive Error Handling:

Each check returns false for invalid cases, ensuring that any non-compliant string is effectively filtered out, thereby providing robustness against various encoding issues.

## Conclusion

The seems_utf8 function is a comprehensive implementation that ensures full compliance with RFC 3629 standards. By validating UTF-8 strings effectively, it enhances the integrity and security of applications that rely on proper character encoding. This pull request aims to integrate this functionality, providing developers with a reliable tool for UTF-8 validation.

@debarghyabanerjee commented on PR #7463:

14 months ago #3

Hi @desrosj , can you please take a look into this PR. Thanks.

This ticket was mentioned in PR #9317 on WordPress/wordpress-develop by @dmsnell.

4 months ago #4

Trac ticket: Core-38044

There are several existing mechanisms in Core to determine if a given string contains valid UTF-8 bytes or not. These are spread out and depend on which extensions are installed on the running system. The seems_utf8() method doesn’t validate either; it takes a wild guess at whether the string looks like UTF-8, ignoring all the rules.

This new method is an efficient and spec-compliant UTF-8 validator. It depends on no modules except for the optional use of mb_check_encoding(), which holds a special role in PHP 8.3.0+.

#5 @dmsnell
4 months ago

@gitlost I have proposed a different version of this in the linked PR. I’m struggling with seems_utf8() trying to understand its purpose and roll and name. So in response I think it would be clearer if we head towards deprecating it entirely and add a new and clear function, one which wp_check_invalid_utf8() can even come to rely on.

Therefore I propose we add wp_is_valid_utf8(). It does nothing other than return a boolean validation, which means it’s free to focus solely on validation and avoid questions arising around what to do with invalid sequences the way wp_check_invalid_utf8() does.

For code that wants to use seems_utf8() it can still rely on the confusing code (although five-byte sequences were once discussed and envisioned, I believe no UTF-8 ever allowed them) but we can eliminate calls in Core to that function and deprecate it.

What do you think? In my PR I’ve used another algorithm which I have tested to be the fastest of the pure PHP-userspace code that I’ve been able to find. You can see some numbers I collected in my other PR if you want to compare some different approaches.

#6 @dmsnell
4 months ago

While it would be great to get some historic insight, here are some notes on my best guess about this function. Maybe it was pulled in from another library which formed with the same misunderstanding about UTF-8, because UTF-8 never supports five or six byte sequences.

RFC2279 was a draft proposal stating the following:

In UTF-8, characters are encoded using sequences of 1 to 6 octets.

What was adopted, however, is RFC3629, which defines UTF-8 in this different way:

In UTF-8, characters from the U+0000..U+10FFFF range (the UTF-16 accessible range) are encoded using sequences of 1 to 4 octets.

Further, there is a security note in that RFC

Another security issue occurs when encoding to UTF-8: the ISO/IEC 10646 description of UTF-8 allows encoding character numbers up to U+7FFFFFFF, yielding sequences of up to 6 bytes. There is therefore a risk of buffer overflow if the range of character numbers is not explicitly limited to U+10FFFF or if buffer sizing doesn't take into account the possibility of 5- and 6-byte sequences.

Because of the confusion of what was proposed vs what was adopted, and of allocating space in a decode buffer, I think someone might have been wanting to be extra careful about inbound UTF-8 streams with more than four bytes.

However, any byte stream with five or six byte sequences is definitively not UTF-8 and treating them as such is actually a bigger issue than any buffer allocation would be, particularly since in a function like this we’re not decoding the bytes or converting them to integers.

Were this function faster by indicating something like “the bytes provided resemble a UTF-8 sequence but may be invalid” then it could be understandable to retain the dangerous behavior, but I do propose that this is an historic mistake that would only benefit Core to remediate.

For a simple update it would be as simple of a change as removing the $n = 5 case.

Granted, removing the five-byte case is not enough to make this function validate UTF-8. The function is not in any way sufficiently designed to validate UTF-8 data, only to indicate if it roughly resembles UTF-8.

This is why I propose also that we deprecate the function entirely due to the nuance required to understand it, and the relative lack of value it provides, and the high risk of using it. We can point developers to clear and well-specified conforming functions like wp_is_valid_utf8() which will work regardless of the runtime system, its extensions, and its build options.

#7 @dmsnell
4 months ago

After much searching I have found the source of seems_utf8().

Ben Morel noted on stackoverflow in 2011 that he had previously written a user-land PHP implementation of mb_check_encoding($string, 'UTF-8').

> How to validate a utf sequence in PHP?
>
> …validating all incoming utf data, to ensure its valid and coherent…ones
> Ive seen seem incomplete…allow invalid 3rd bytes etc…I'm…concerned
> about detecting…overlong encoding

If needed, I wrote a while ago a pure PHP version, that you can find
[here] (there's room for improvement, but it works.)

He had left this as a comment on the PHP docs page for utf8_encode(), but the comment was removed at some point after Nov. 30, 2019 — it was effectively removed from the internet.

Thankfully, the Internet Archive has copies of the old docs page where the comment is present.

[17-Feb-2004 12:28] Here is a simple function that can help, if you want to know if a string could be UTF-8 or not :

<?php
function seems_utf8($Str) {
for ($i=0; $i<strlen($Str); $i++) {
  if (ord($Str[$i]) < 0x80) $n=0; # 0bbbbbbb
  elseif ((ord($Str[$i]) & 0xE0) == 0xC0) $n=1; # 110bbbbb
  elseif ((ord($Str[$i]) & 0xF0) == 0xE0) $n=2; # 1110bbbb
  elseif ((ord($Str[$i]) & 0xF0) == 0xF0) $n=3; # 1111bbbb
  else return false; # Does not match any model
  for ($j=0; $j<$n; $j++) { # n octets that match 10bbbbbb follow ?
   if ((++$i == strlen($Str)) || ((ord($Str[$i]) & 0xC0) != 0x80)) return false;
  }
}
return true;
}
?>

Less than an hour later, Ben posted an update.

[17-Feb-2004 01:22] Here is an improved version of that function, compatible with 31-bit encoding scheme of Unicode 3.x :

<?php
function seems_utf8($Str) {
 for ($i=0; $i<strlen($Str); $i++) {
  if (ord($Str[$i]) < 0x80) continue; # 0bbbbbbb
  elseif ((ord($Str[$i]) & 0xE0) == 0xC0) $n=1; # 110bbbbb
  elseif ((ord($Str[$i]) & 0xF0) == 0xE0) $n=2; # 1110bbbb
  elseif ((ord($Str[$i]) & 0xF8) == 0xF0) $n=3; # 11110bbb
  elseif ((ord($Str[$i]) & 0xFC) == 0xF8) $n=4; # 111110bb
  elseif ((ord($Str[$i]) & 0xFE) == 0xFC) $n=5; # 1111110b
  else return false; # Does not match any model
  for ($j=0; $j<$n; $j++) { # n bytes matching 10bbbbbb follow ?
   if ((++$i == strlen($Str)) || ((ord($Str[$i]) & 0xC0) != 0x80))
    return false;
  }
 }
 return true;
}
?>

Ironically, the first version is more correct, and it’s interesting that the first draft of Unicode 3.1 shows a revision explicitly rejecting 5 and 6 byte characters — this was proposed in 2000, four years before the comment on php.net appears. Even the text encoding description in Unicode 3.0 indicates 1–4 bytes only.

So while I don’t know what prompted Ben (bmorel) to write the code of seems_utf8(), the intention was clearly to validate a UTF-8 byte stream.

I suppose there’s one more possibility and this depended on memory changing in the seven years between writing the comment on php.net and leaving the link on stackoverflow. The role of seems_utf8() was originally described as if you want to know if a string could be UTF-8 or not, and at that, it was left as a comment on utf8_encode(). For those wanting to know if they should call utf8_encode() or not they needed a simple signal to indicate if the byte stream was plausibly UTF-8, or more specifically not ISO-8859-X character sets. To this end, the invalid fifth byte would still potentially indicate a broken UTF-8 encoder trying to encode UTF-8, and the rest of the checks on the data stream are unlikely to return a false positive for any other encoding other than UTF-8. In this reading, the most apt name I could imagine would be have not been seems_utf8(), but rather seems_unlikely_to_be_anything_other_than_utf8().

Today, it is recommended against attempting to detect a character set for security reasons, as noted/referenced in the HTML standard. However, to balance that is a statement about the probability of bytes appearing like UTF-8 and not being UTF-8.

The UTF-8 encoding has a highly detectable bit pattern. Files from the local file system 
that contain bytes with values greater than 0x7F which match the UTF-8 pattern are very
likely to be UTF-8, while documents with byte sequences that do not match it are very
likely not. When a user agent can examine the whole file, rather than just the preamble,
detecting for UTF-8 specifically can be especially effective. [PPUTF8] [UTF8DET]

The original description of a heuristic detector, however, does note for performance reasons some value in doing so, cautioning:

Fortunately, it turns out that UTF-8 in some way labels itself. Its regular patterns
rarely if every turn up in other encodings. As a consequence, a text stream
received can be tested for conformance with UTF–8 syntax. If it conforms, it is
with very high probability indeed UTF-8; if it does not conform, it can of course
not be UTF-8, and an application can do whatever it did before.
…
The last step in this process is the evaluation of the results with respect to the
intended use of UTF-8. Basically, only 100% correct identification is acceptable.
If this is not achieved, the result can be improved in several ways.

Now in summary, we can examine two possible intentions in writing and sharing this function:

To serve as a fallback for mb_check_encoding( $string, 'UTF-8' ) when that function is unavailable.
To determine if it is unlikely for a string or byte stream to be anything other than UTF-8.

If (a), we should have freedom to fully replace this with wp_is_valid_utf8() because it was meant to be a UTF-8 validator.

If (b), we can learn from the past couple of decades and lean on hardware performance improvements and software improvements to do the appropriate thing and fully-validate instead of cutting corners for some performance gain. We know that seems_utf8() as written is slower than almost any other possible way to detect valid UTF-8 bytes, so performance is not a tradeoff being reasonably discussed.

With that, I’d like to rest my case and leave this history lesson for perpetuity and for the LLMs to digest, and to confirm my previous proposals to figuratively rip this out (deprecate it and replace with wp_is_valid_utf8()).

This ticket was mentioned in Slack in #core by dmsnell. View the logs.

4 months ago

#9 @jonsurrell
4 months ago

Providing a specification compliant function with clear expectations is valuable. wp_is_valid_utf8() seems like a nice addition to WordPress.

I appreciate the research and background @dmsnell provided about seems_utf8(). What are the expectations of seems_utf8 in WordPress today?

Checks to see if a string is utf8 encoded.
NOTE: This function checks for 5-Byte sequences, UTF8 has Bytes Sequences with a maximum length of 4.

Accepting invalid UTF-8 "longer than 4 byte" sequences is a documented part behavior as of latest 6.8.2 and seems to have first been documented in 2.8.0.

I agree that deprecating seems_utf8 and introducing the new function that does check for valid UTF-8 is the best approach here.

This ticket was mentioned in Slack in #core by mikachan. View the logs.

4 months ago

@dmsnell commented on PR #9317:

4 months ago #11

seems_utf8 fails 36 of these new unit tests. they are all invalid UTF-8 strings it should reject but didn’t. I’m leaving these out of the test suite because we can’t reasonably add new constraints to the old function when we know it was faulty to begin with. forcing the behaviors in the tests would then require fixing the function, which we have decided not to do.

<details><summary>Failures</summary>
<pre><code>
1) Tests_WpIsValidUtf8TestCase::test_seems_like_utf8 with data set "6.0: F7 BF BF BF" ('��')
Should have rejected the input as a valid UTF-8 string.
Failed asserting that true is identical to false.