Bug 211671 - Fix base64.any.html test
Summary: Fix base64.any.html test
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-05-09 13:06 PDT by Rob Buis
Modified: 2020-05-11 00:07 PDT (History)
6 users (show)

See Also:


Attachments
Patch (14.29 KB, patch)
2020-05-09 13:11 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (14.29 KB, patch)
2020-05-10 05:58 PDT, Rob Buis
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.