mb_detect_encoding() results for UTF-7 differ between PHP 8.0 and 8.1 (if UTF-7 is present in the encodings list and the string contains '+' character)

Description

This is a very serious bug that may impact all strings in PHP.

The following code:

<?php

ini_set('display_errors', '1');	// display runtime errors
error_reporting(E_ALL & ~E_NOTICE & ~E_STRICT & ~E_DEPRECATED); // error reporting
date_default_timezone_set('UTC');

function detect_encoding($ystr, $csetlist='UTF-8, ISO-8859-1, ISO-8859-15, ISO-8859-2, ISO-8859-9, ISO-8859-3, ISO-8859-4, ISO-8859-5, ISO-8859-6, ISO-8859-7, ISO-8859-8, ISO-8859-10, ISO-8859-13, ISO-8859-14, ISO-8859-16, UTF-7, ASCII, SJIS, EUC-JP, JIS, ISO-2022-JP, EUC-CN, GB18030, ISO-2022-KR, KOI8-R, KOI8-U') {
	return mb_detect_encoding((string)$ystr, (string)$csetlist, true); // mixed: (bool) FALSE or (string) 'CHARSET'
}

echo detect_encoding('A + B'); // expected output: UTF-8, but on PHP 8.1.x / 8.2.x returns UTF-7 if the '+' character is present in a string ; the PHP 8.0.x and 7.4.x behaves correctly and outputs UTF-8 also in this case
echo "\n";
echo detect_encoding('A - B'); // expected output: UTF-8 ; correct on all PHP versions 8.2.x / 8.1.x / 8.0.x / 7.4.x

Resulted in this output ; On PHP 8.2 and 8.1, if the plus (+) character is present in a string will detect UTF-7 instead of UTF-8 as expected:

UTF-7
UTF-8

But I expected this output instead ; on PHP 7.4 and PHP 8.0 works correctly. Output is this):

UTF-8
UTF-8

I tested here with different PHP versions.
https://onlinephp.io/
I also tested in my computer with PHP 8.1.12 and 8.0.25

PHP Version

8.1.x / 8.2.x

Operating System

All

@alexdowad

I honestly doubt the severity of this bug as AFAIK any UTF-7 string is valid UTF-8, moreover mb_detect_encoding() is a misnomer as it tries to guess what encoding a string might be.

But I'll let @alexdowad comment, as he's the character encoding expect and has been refactoring mbstring.

It is very serious. Because if you need to normalize the output you rely on re-encoding all strings as UTF-8.
Thus, if re-encode 'A + B' from UTF-7 (which is actually UTF-8 already, but is wrong detected by PHP 8.1/8.2) to UTF-8 results in 'A B' (the plus sign disappears), as the strings get corrupted.
Actually this is how I start investigating and get to this bug.
Scenario:
Read from a data source -> Filter UTF-8 or re-encode as UTF-8 -> Output. If a string contains '+' sign will be mangled after normalization (re-encode as UTF-8 which actually would be not necessary if UTF-8 would be correctly detected as in PHP 8.0 / 7.4).

It impacts so many things in an unpredictable way so I can't explain all situations here.

I updated the example for you to see the real disaster !

<?php

ini_set('display_errors', '1');	// display runtime errors
error_reporting(E_ALL & ~E_NOTICE & ~E_STRICT & ~E_DEPRECATED); // error reporting
date_default_timezone_set('UTC');

function detect_encoding($ystr, $csetlist='UTF-8, ISO-8859-1, ISO-8859-15, ISO-8859-2, ISO-8859-9, ISO-8859-3, ISO-8859-4, ISO-8859-5, ISO-8859-6, ISO-8859-7, ISO-8859-8, ISO-8859-10, ISO-8859-13, ISO-8859-14, ISO-8859-16, UTF-7, ASCII, SJIS, EUC-JP, JIS, ISO-2022-JP, EUC-CN, GB18030, ISO-2022-KR, KOI8-R, KOI8-U') { // Fix: starting from PHP 7.1 it warns about illegal argument if using: ISO-8859-11
	return mb_detect_encoding((string)$ystr, (string)$csetlist, true); // mixed: (bool) FALSE or (string) 'CHARSET'
}

$str = 'A + B';
echo detect_encoding($str);
echo "\n";
echo mb_convert_encoding($str, 'UTF-8', detect_encoding($str));
echo "\n";
echo "\n";
$str = 'A - B';
echo detect_encoding('A - B');
echo "\n";
echo mb_convert_encoding($str, 'UTF-8', detect_encoding($str));

expected output (OK in PHP 8.0 / 7.4)

UTF-8
A + B

UTF-8
A - B

wrong output in PHP 8.1 / 8.2 (the + character is mangled, disappeared)

UTF-7
A  B

UTF-8
A - B

Thus in my opinion would affect a lot of database data. This is the most serious bug in PHP since I have seen. If you re-save in database all the data becomes corrupted without a chance to go backward only if you have a valid backup !!

I suspect a memory corruption there or something very ugly ...
If I convert a string from UTF-7 to UTF-8 should be preserved as is. Right ?
But actually because a string which contains + character is already UTF-8 but PHP detects it as UTF-7 it corrupts that string on re-encoding and + characters vanish ... !

And if 'A + B' is detected as UTF-7 instead of UTF-8 on PHP 8.1/8.2,... why 'A - B' is detected as UTF-8 ? Can't you see the anomaly in this case ?

EDIT: Sorry, you can ignore this comment. In UTF7-IMAP the closing delimiter is specified as -. So, it is different. I'll shut up now.

Just to provide some historical background, when UTF7-IMAP was designed, an explicit design goal was that there should be one, and only one, possible way to encode any given string. It was deliberately made stricter than UTF-7.

It's in fact a serious issue that affects us too, because "+" characters are being dropped when doing mb_convert_encoding("+", "UTF-8", "UTF-7");

<?php
function fvm_ensure_utf8($str) {
	$enc = mb_detect_encoding($str, mb_list_encodings(), true);
	var_dump($enc);
	if ($enc === false){
		return false; // could not detect encoding
	} else if ($enc !== "UTF-8") {
		return mb_convert_encoding($str, "UTF-8", $enc); // converted to utf8
	} else {
		return $str; // already utf8
	}
}

$css = 'input[type="radio"]:checked + img {
    border: 5px solid #0083ca;
}';
$css = fvm_ensure_utf8($css);
echo $css;

PHP8.1+

string(5) "UTF-7"
input[type="radio"]:checked  img {
    border: 5px solid #0083ca;
}

PHP8.0

string(5) "UTF-8"
input[type="radio"]:checked + img {
    border: 5px solid #0083ca;
}

@Tofandel

@Tofandel, mb_detect_encoding() is not a magic encoding/charset detector. Actually, there is no such thing, at least if the strings to check are short. If you are expecting UTF-8, I suggest to reject anything else, and not magically try to convert it.

Anyhow, I leave this for @alexdowad to decide.

@Tofandel

@Tofandel, thanks for letting us know about the issue you are experiencing.

It's not really the main point, but anyways, a + character (U+002B PLUS SIGN) is not represented by the single UTF-7 byte '+'. In UTF-7, a plus sign is represented by the two bytes '+-'. So it's not really correct to say that mb_convert_encoding is "dropping" plus signs when converting UTF-7 strings; those 0x2B bytes don't represent plus signs in UTF-7.

A bare '+' like you showed in the code above is invalid according to the UTF-7 RFC, so mbstring should treat it as an error; as you can see from the above discussion, I am just trying to figure out when this change should be made.

@cmb69's recommendation is very good, and you would probably be wise to follow it (though we don't know all the details of your application). However, even if you have good reason to use mb_detect_encoding, I would really suggest that mb_detect_encoding(..., mb_list_encodings()) is asking for trouble. Rather than mb_list_encodings(), it is much better to provide an explicit list of the text encodings which you actually need to support. For many applications, it might be something like ['UTF-8', 'UTF-16LE', 'UTF-16BE'], perhaps with a couple of legacy text encodings added to the list.

Supporting UTF-7 is quite dubious. The name "UTF-7" might make you think this is a "modern" way of encoding text, but that is not true at all. Wikipedia describes UTF-7 as "obsolete", and that is 100% true. It's a weird, badly designed text encoding which is hardly ever used for anything.

About the mb_list_encodings thing, when I finish the work I am currently doing on the mbstring codebase and start working on the documentation, I intend to put a warning right in the PHP manual not to pass mb_list_encodings() to mb_detect_encoding. It's just a bad idea.

Even so, I do acknowledge that many PHP users will continue to use mb_detect_encoding like this. When we make the UTF-7 conversion code a bit stricter about illegal uses of '+', it will help to prevent UTF-8 text from being accidentally identified as UTF-7. Again, we just need to figure out when to make this change and how to go about it.

It's not just for UTF-7. Since PHP 8.1.7 and up thinks this string is Windows-1252, even though it's seen as UTF-8 in PHP 8.0, 7.4, and 8.1.2

' ̞>'

It contains the U+031E character

mb_detect_encoding(' ̞>',['ASCII','UTF-8', 'ISO-8859-1', 'ISO-8859-2', 'ISO-8859-6','ISO-8859-15', 'Windows-1252'])

Another example is
mb_detect_encoding('Ⱥ',['UTF-8', 'ISO-8859-1', 'ISO-8859-2', 'ISO-8859-6','ISO-8859-15', 'Windows-1252','ASCII'])

in PHP 8.1 and up it's seen as ISO-8859-1, and in PHP 8.0 as UTF-8

@Tofandel

Just working on a patch to be slightly more strict when checking whether an input string might be UTF-7. I am adding @Tofandel's test case to the official test suite. This should be incorporated in the upcoming release of PHP 8.3.

@Lehren

@Lehren Thanks for the report. Frankly, I think the behavior in PHP 8.1+ makes more sense. Why would we expect to find U+031E COMBINING DOWN TACK BELOW coming after an ASCII space character?

Still, that is a valid (though nonsensical) UTF-8 string, so if all you really care about is that the string is valid UTF-8, then you can use mb_check_encoding($str, 'UTF-8'). That is a much better idea even for PHP 8.0 and below; for one thing, it's much faster.

This was fixed in #11239, but this issue was not closed automatically (despite being linked to the PR). So I'm closing this manually. If this is a mistake, let me know and I'll reopen.

Navigation Menu

Repository navigation

mb_detect_encoding() results for UTF-7 differ between PHP 8.0 and 8.1 (if UTF-7 is present in the encodings list and the string contains '+' character) #10192

Description

PHP Version

Operating System

36 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

mb_detect_encoding() results for UTF-7 differ between PHP 8.0 and 8.1 (if UTF-7 is present in the encodings list and the string contains '+' character) #10192

Description

Description

PHP Version

Operating System

Activity

Girgias commented on Dec 30, 2022

unix-world commented on Dec 30, 2022

unix-world commented on Dec 30, 2022

unix-world commented on Dec 30, 2022

unix-world commented on Dec 30, 2022

unix-world commented on Dec 30, 2022

36 remaining items

alexdowad commented on Jan 5, 2023

Tofandel commented on Jan 18, 2023

cmb69 commented on Jan 18, 2023

alexdowad commented on Jan 18, 2023

Lehren commented on Mar 22, 2023

alexdowad commented on May 14, 2023

alexdowad commented on May 14, 2023

ndossche commented on Nov 11, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions