Because this routine used a signed char buffer to hold the bytes
in a (possible) HTML entity, any bytes with the MSB set would
be sign-extended when converting to int; for example, 0x86 would
become 0xFFFFFF86 (or -121).
Codepoints with huge values, like 0xFFFFFF86, are not valid and
if any were passed to the output filter, it would treat them
as errors and emit error markers.
• The legacy conversion code did not emit an error marker if an
escape sequence was truncated.
• BOTH old and new conversion code would shift from KSC5601
(KS X 1001) mode to ASCII mode on an invalid escape sequence.
This doesn't make any sense.
SJIS-Mobile#SOFTBANK text encoding supports special escape sequences,
which shift the decoder into a mode where each single byte represents
an emoji. To get out of this mode, a 0xF (SHIFT OUT) byte can be
used.
After one of these special escape sequences, the new conversion
code expected to see at least one more byte. However, there doesn't
seem to be any particular reason why it should be treated as an
error condition if a string ends abruptly after one of these
escapes. Well, the escape sequence is useless in that case, but
it is a complete and valid escape sequence.
The legacy conversion code did allow a string to end immediately
after one of these escape sequences. Amend the new code to allow
the same.
The use of a special 'vtbl' for converting between '7bit' and
'8bit' text meant that '7bit' text would not be converted to
wchars before going to '8bit'. This meant that the special
value MBFL_BAD_INPUT, which we use to flag an erroneous byte
sequence in input text (and which is required by functions
like mb_check_encoding), would pass directly to the output,
instead of being converted to the error marker specified
by mb_substitute_character.
This issue dates back to the time when I removed the mbfl
'identify filters' and made encoding validity checking and
encoding detection rely only on the conversion filters.
Up until now, I believed that mbstring had been designed such
that (legacy) text conversion filter objects should not be
re-used after the 'flush' function is called to complete a
text conversion operation.
However, it turns out that the implementation of
_php_mb_encoding_handler_ex DID re-use filter objects
after flush. That means that functions which were based on
_php_mb_encoding_handler_ex, including mb_parse_str and
php_mb_post_handler, would break in some cases; state left
over from converting one substring (perhaps a variable name)
would affect the results of converting another substring
(perhaps the value of the same variable), and could cause
extraneous characters to get inserted into the output.
All this code should be deleted soon, but fixing it helps me
to avoid spurious failures when fuzzing the new/old code to
look for differences in behavior.
EUC-JP-2004 includes special byte sequences starting with 0x8E
for kana. The legacy output routine for EUC-JP-2004 emits
these sequences if the value of the output variable `s` is
between 0x80 and 0xFF.
Since the same routine was also used for SJIS-2004 and
ISO-2022-JP-2004, before 8a915ed26c, the same 0x8E sequences
would be emitted when converting to those text encodings as well.
But that is completely wrong. 0x8E 0x__ does not mean the same
in SJIS-2004 or ISO-2022-JP-2004 as it does in EUC-JP-2004.
Therefore, in 8a915ed26c, I fixed the legacy conversion routine
by checking whether the output encoding is EUC-JP-2004 or not.
If it's not, and `s` is 0x80-0xFF, I made it emit an error.
Well, it turns out that single bytes with values from 0xA1
to 0xDF are meaningful in SJIS-2004. To emit these bytes when
appropriate, I had to amend the legacy conversion routine again.
(For clarity, this does NOT mean reverting to the behavior prior
to 8a915ed26c. We were right not to emit sequences starting with
0x8E in SJIS-2004. But in SJIS-2004, we *do* sometimes need to
emit single bytes from 0xA1-0xDF.)
CP50220 converts some codepoints which represent kana
(hiragana/katakana) to a different form. This is the only difference
between CP50220 and CP50221 (which doesn't perform such conversion).
In some cases, this conversion means collapsing two codepoints to
a single output byte sequence. Since the legacy text conversion
filters only worked a byte at a time, the legacy filter had to
cache a byte, then wait until it was called again with the next
byte to compare the cached byte with the following one.
That was all fine, but it didn't work as intended when there were
errors (invalid byte sequences) in the input. Our code (both old
and new) for emitting error markers recursively calls the same
conversion filter. When the old CP50220 filter was called
recursively, the logic for managing cached bytes did not behave
as intended. As a result, the error markers could be reordered
with other characters in the output.
I used an ugly hack to fix this in 6938e3512; when making a
recursive call to emit an error marker, temporarily swap out
`filter->filter_function` to bypass the byte-caching code,
so the error marker immediately goes through to the output.
This worked, but I overlooked the fact that the very same
problem can occur if an invalid byte sequence is detected
*in the flush function*. Apply the same (ugly) fix.
The legacy Base64 conversion code in mbstring automatically
wrapped the output to 72 columns, and the new code imitates
this behavior. Frankly, I'm not sure if this is a good idea
or not (people could easily manually wrap it if they want to),
but have stuck with this behavior for backwards compatibility.
However, fuzzing revealed one case where we were not wrapping
to 72 columns; if the input string is not a multiple of 3
characters, meaning that the output must be padded, and the
point where we must add the final (padded) output happens to
be just beyond 72 columns.
Fuzzing revealed a small difference between the number of error
markers which the legacy ISO-2022-JP and JIS7/8 conversion code
emitted for truncated escape sequences and those emitted by the
new code. The behavior of the old code seems more reasonable
here, so we will imitate it.
In 04e59c916f, I amended the UTF-8 conversion code, so that when given
invalid input, it would emit a number of errors markers harmonizing
with the WHATWG's specification of the standard UTF-8 decoding
algorithm. (Which, gentle reader of commit logs, you can find online
at https://encoding.spec.whatwg.org/#utf-8-decoder.) However, the code
in 04e59c916f was faulty in the case that a truncated UTF-8 code unit
starts with 0xF1.
Then, in dc1ba61d09, when making a small refactoring to a different
part of the UTF-8 conversion code, I inexplicably broke part of the
working code, causing the same fault which was already present with
truncated UTF-8 code units starting with 0xF1 to also occur with
0xF2 and 0xF3 as well. I don't remember what inane thoughts I was
thinking when I pulled off this feat of utter mental confusion.
None of these cases were covered by unit tests, by the way.
Thankfully, my trusty fuzzer picked up on this when testing the
new implementation of mb_parse_str (since the legacy UTF-8
conversion filter did not suffer from the same problem, and I was
fuzzing to find any differences in behavior between the old and
new implementations).
Fortuitously, the fuzzer also picked up another issue which was
present in 04e59c916f. I was emitting only one error marker for
truncated code units starting with 0xE0 or 0xED, in cases where
the WHATWG standard indicates two should be emitted. Examples
are 0xE0 0x9F <END OF STRING> or 0xED 0xA0 <END OF STRING>.
Code units starting with 0xE0-0xED should have 3 bytes. If the
first byte is 0xE0, the second MUST be 0xA0 or greater. (Otherwise,
the codepoint could have fit in a two-byte code unit.) And if the
first byte is 0xED, the second MUST be 0x9F or less. According to
the WHATWG algorithm, step 4, if the second byte is outside the
legal range, then the decoder should emit an error... AND
reprocess the out-of-range byte. The reprocessing will then
cause another error. That's why the decoder should indicate two
errors and not one.
Fuzzing revealed that something was missed here when making the new
encoding conversion code match the behavior of the old code. In the
next major release of PHP, support for these non-encodings will be
dropped, but in the meantime, it is better to match the legacy
behavior.
This fix is another solution to replace d0527427be, use zend_try and zend_catch to make sure persistent stream will be released when error occurred.
Closes GH-9332.
This reverts commit d0527427be.
This patch makes Swoole/Swow can not work anymore, because Coroutine will yield to another one during socket operation, EG(record_errors) assertion will always fail, and zend_begin_record_errors() was only used during compile time before.
Note: zend_emit_recorded_errors() and the typo fix are reserved.
This is not actually related to SSL handshake but stream socket creation
which does not clean errors if the error handler is set. This fix
prevents emitting errors until the stream is freed.
The comparator function used at ksort in SORT_REGULAR mode
need to be consistent with basic comparison rules. These rules
were changed in PHP-8.0 for numeric strings, but comparator
used at ksort kept the old behaviour. It leads to inconsistent
situations, when after ksort the first key is GREATER than some
of the next ones by according to the basic comparison operators.
Closes GH-9293.
