- Identify filter was completely wrong.
- Respect `mb_substitute_character` rather than converting invalid bytes to
Unicode 0xFFFD (generic replacement character).
- Don't convert Unicode 0xFFFD to a valid ARMSCII-8 character.
- When converting ARMSCII-8 to ARMSCII-8, don't pass invalid bytes through
silently.
Previously, `mb_check_encoding` did an awful lot of unneeded work. In order to
determine whether a string was valid or not, it would convert the whole string
into wchar (code points), which required dynamically allocating a (potentially
large) buffer. Then it would turn right around and convert that big 'ol buffer
of code points back to the original encoding again. Finally, it would check
whether any invalid bytes were detected during that long and onerous process.
The thing is, mbstring _already_ has machinery for detecting whether a string
is valid in a certain encoding or not, and it doesn't require copying any data
around or allocating buffers. Better yet, it can fail fast when an invalid byte
is found. Why not use it? It's sure a lot faster!
Further, the legacy code was also badly broken. Why? Because aside from
checking whether illegal characters were detected, it would also check whether
the conversion to and from wchars was lossless. But, some encodings have
more than one valid encoding for the same character. In such cases, it is
not possible to make the conversion to and from wchars lossless for every
valid character. So `mb_check_encoding` would actually reject good strings
in a lot of encodings!
One funny thing: while the original author used Unicode 0xFFFD (generic
replacement character) for invalid bytes in CP1251 and CP1252, for CP1254
they used 0xFFFE, which is not a valid Unicode codepoint at all, but is a
reversed byte-order mark. Probably this was by mistake.
Anyways,
- Fixed identify filter, which was completely wrong.
- Don't convert Unicode 0xFFFE to a random (but valid) CP1254 byte.
- When converting CP1254 to CP1254, don't pass invalid bytes through silently.
- Identify filter was as wrong as wrong can be.
- Invalid CP1251 byte 0x98 was converted to Unicode 0xFFFD (generic
replacement character), rather than respecting `mb_substitute_character`.
- Unicode 0xFFFD was converted to some random CP1251 byte.
- When converting CP1251 to CP1251, don't pass invalid bytes through silently.
The implementation is based on the upstream PMurHash. The following
variants are implemented
- murmur3a, 32-bit hash
- murmur3c, 128-bit hash for x86
- murmur3f, 128-bit hash for x64
The custom seed support is not targeted by this implementation. It will
need a major change to the API, so then custom arguments can be passed
through `hash_init`. For now, the starting hash is always zero.
Fixes bug #68109, closes#6059
Signed-off-by: Anatol Belski <ab@php.net>
Co-Developed-by: Michael Wallner <mike@php.net>
Signed-off-by: Michael Wallner <mike@php.net>
Also remove a bogus test (bug62545.phpt) which wrongly assumed that all invalid
characters in CP1251 and CP1252 should map to Unicode 0xFFFD (REPLACEMENT
CHARACTER).
mbstring has an interface to specify what invalid characters should be
replaced with; it's called `mb_substitute_character`. If a user wants to see
the Unicode 'replacement character', they can specify that using
`mb_substitute_character`. But if they specify something else, we should
follow that.
It's a bit surprising how much was broken here.
- Identify filter was utterly and completely wrong.
- Instead of handling invalid CP1252 bytes as specified by
`mb_substitute_character`, it would convert them to Unicode 0xFFFD
(generic replacement character).
- When converting ISO-8859-1 to CP1252, invalid ISO-8859-1 bytes would
be passed through silently.
- Unicode codepoints from 0x80-0x9F were converted to CP1252 bytes 0x80-0x9F,
which is wrong.
- Unicode codepoint 0xFFFD was converted to CP1252 0x9F, which is very wrong.
Also clean up some unneeded code, and make the conversion table consistent with
others by using zero as a 'invalid' marker, rather than 0xFFFD.
While fixing bugs in mbstring, one of my new test cases failed with a strange
error message stating: 'Warning: Undefined array key 1...', when clearly the
array key had been set properly.
GDB'd that sucker and found that JIT'd PHP code was calling directly into
`zend_hash_add_new` (which was not converting the numeric string key to an
integer properly). But where was that code coming from? I examined the disasm,
looked up symbols to figure out where call instructions were going, then grepped
the codebase for those function names. It soon became clear that the disasm I
was looking at was compiled from `zend_jit_fetch_dim_w_helper`.
This testing mode executes the test multiple times in the same
process (but in different requests). It is primarily intended to
catch tracing JIT bugs, but also catches state leaks across
requests.
Closes GH-6365.
JIT ignores that the `zend_write` callback is overwritten, so we define
our own callback and caller.
We also fix the "inconsistent DLL binding" warnings on Windows, by
introducing `PHP_ZEND_TEST_API`.
Closes GH-6391.
Nikita Popov