Bug 233921 - TextDecoder doesn't detect invalid UTF-8 sequences early enough
Summary: TextDecoder doesn't detect invalid UTF-8 sequences early enough
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
: 234030 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-12-07 03:55 PST by Andreu Botella
Modified: 2021-12-14 08:30 PST (History)
10 users (show)

See Also:


Attachments
Patch (16.63 KB, patch)
2021-12-13 05:50 PST, Andreu Botella
no flags Details | Formatted Diff | Diff
Patch (18.30 KB, patch)
2021-12-13 14:38 PST, Andreu Botella
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreu Botella 2021-12-07 03:55:22 PST
WPT tests: https://wpt.fyi/results/encoding/textdecoder-eof.any.html?label=experimental&label=master&aligned (stream: true case), https://wpt.fyi/results/encoding/textdecoder-streaming.any.html?label=experimental&label=master&aligned, https://wpt.fyi/results/encoding/streams/decode-utf8.any.html?label=experimental&label=master&aligned (non-SharedArrayBuffer cases)

Related Chromium bug: https://bugs.chromium.org/p/chromium/issues/detail?id=796697

When the TextCodecUTF8 decoder finds a non-ASCII lead byte, it waits until enough bytes are consumed to make a valid sequence starting at that position, before starting to process the bytes. This goes against the encoding spec, which requires the replacement character to be emitted as soon as enough bytes are consumed to tell that the sequence is in fact invalid.

While this does not make a difference for non-streaming input, or for streaming data coming from the network, it does make a difference in that TextDecoder returns the wrong result as per the spec when in streaming mode:

const decoder = new TextDecoder();
console.log(decoder.decode(new Uint8Array([0xF0, 0x9F]), { stream: true }));
console.log(decoder.decode(new Uint8Array([0x41]), { stream: true }));
console.log(decoder.decode(new Uint8Array([0x42]), { stream: true }));

As per the spec, and in Firefox and Chromium 98, this prints "", "�A", "B". In WebKit and previous versions of Chromium, it prints "", "", "�AB".
Comment 1 Alex Christensen 2021-12-08 13:23:00 PST
I think we will need to do basically the same thing as Chromium, but it won't be a simple cherry pick because their DecodeNonASCIISequence returns a different value than our decodeNonASCIISequence and doesn't change the length, and their handlePartialSequence for LChar and UChar are more similar than ours are.
Comment 2 Alex Christensen 2021-12-09 09:50:17 PST
*** Bug 234030 has been marked as a duplicate of this bug. ***
Comment 3 Andreu Botella 2021-12-13 05:50:55 PST
Created attachment 446994 [details]
Patch
Comment 4 EWS Watchlist 2021-12-13 05:52:03 PST
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 5 Andreu Botella 2021-12-13 14:38:43 PST
Created attachment 447070 [details]
Patch
Comment 6 Radar WebKit Bug Importer 2021-12-14 03:56:16 PST
<rdar://problem/86461983>
Comment 7 EWS 2021-12-14 08:30:29 PST
Committed r287024 (245229@main): <https://commits.webkit.org/245229@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 447070 [details].