Bug 211671

Summary: Fix base64.any.html test
Product: WebKit Reporter: Rob Buis <rbuis>
Component: New BugsAssignee: Rob Buis <rbuis>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, darin, ews-watchlist, japhet, koivisto, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Rob Buis 2020-05-09 13:06:27 PDT 
Fix base64.any.html test by extending DataURLDecoder with a
forgiving-base64 decode mode [1], as used by the Fetch
data: URL processor algorithm [2].

[1] https://infra.spec.whatwg.org/#forgiving-base64-decode
[2] https://fetch.spec.whatwg.org/#data-url-processor
Comment 1 Rob Buis 2020-05-09 13:11:24 PDT 
Created attachment 398936 [details]
Patch
Comment 2 Darin Adler 2020-05-09 14:32:32 PDT 
Comment on attachment 398936 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=398936&action=review

> Source/WebCore/loader/ResourceLoader.cpp:278
> +    auto mode = DataURLDecoder::Mode::Default;
> +    if (m_request.requester() == ResourceRequest::Requester::Fetch)
> +        mode = DataURLDecoder::Mode::ForgivingBase64;

In this context, the name ForgivingBase64 isn’t great. The reason is that this is actually *stricter* than the default. Maybe the other mode needs a name other than "Default"?
Comment 3 Rob Buis 2020-05-10 05:58:54 PDT 
Created attachment 398975 [details]
Patch
Comment 4 EWS 2020-05-10 23:57:31 PDT 
Committed r261468: <https://trac.webkit.org/changeset/261468>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398975 [details].
Comment 5 Radar WebKit Bug Importer 2020-05-10 23:58:16 PDT 
<rdar://problem/63078186>
Comment 6 Rob Buis 2020-05-11 00:04:39 PDT 
Comment on attachment 398936 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=398936&action=review

>> Source/WebCore/loader/ResourceLoader.cpp:278
>> +        mode = DataURLDecoder::Mode::ForgivingBase64;
> 
> In this context, the name ForgivingBase64 isn’t great. The reason is that this is actually *stricter* than the default. Maybe the other mode needs a name other than "Default"?

I went with Legacy before landing this.
Comment 7 Rob Buis 2020-05-11 00:07:03 PDT 
Antti, can you remember what specification the DataURLDecoder was following before Fetch was a thing? Was it HTMLx? I am wondering if we can just always use the new Fetch/forgiving-base64-decode now. It does not break the tests but I have no idea whether websites rely on the legacy behavior.