Fix GH-23094: Use byte offsets in NumberFormatter parsing - #23318
Fix GH-23094: Use byte offsets in NumberFormatter parsing#23318ColumbusLabs wants to merge 3 commits into
Conversation
|
The issue is: NumberFormatter rejects offsets inside UTF-8 sequences should return But in this PR, it is Otherwise, this is correct! |
|
Thanks for catching that. I changed the byte-to-UTF-16 conversion to reject offsets that split a UTF-8 sequence while leaving the referenced offset unchanged. I also added regression coverage for both |
|
Could you please set the error state after failure? expected |
|
|
||
| if(zposition) { | ||
| position = (int32_t) zval_get_long(zposition); | ||
| if (!numfmt_utf8_offset_to_utf16(str, str_len, &position)) { |
There was a problem hiding this comment.
@LamentXU123 perhaps $position needs your master changes to be always correct.
There was a problem hiding this comment.
That's painful, but yes. #22572 is rather a feature or a BC break than a bug fix and must not be backported. So yes, 😭😭
|
This fix is a little different in the master branch since we have the UnicodeString refactors. I will take care. |
Fixes #23094.
PHP exposes NumberFormatter parsing offsets as UTF-8 byte offsets, while ICU expects UTF-16 code-unit positions. Convert the input offset before parsing and the returned offset afterward for both parse() and parseCurrency().
This targets PHP-8.4 as the lowest actively supported branch; the equivalent C++ change can be merged upward.
Tests: