data: URL base64 handling different from atob()

• Component: DOM
• Status: RESOLVED FIXED

[Anne van Kesteren]

Tests: https://github.com/w3c/web-platform-tests/pull/6890

Eventual standard: https://github.com/whatwg/fetch/pull/579

It seems better if data: URLs and atob() are aligned.

[Alexey Proskuryakov]

Maybe, but don't they explicitly implement different specs (base64 vs. base64url)?

See also https://github.com/whatwg/html/issues/351

[Anne van Kesteren]

They don't, but it does seem like that's how Safari implements data: URLs, contrary to other browsers.

That's why data:;base64,___ decodes in Safari and not in Chrome and Firefox.

I'm surprised this hasn't caused you more compatibility issues. I'll make sure to add some additional tests for this.

[Chris Dumez]

Looking at DataURLDecoder, I see the following logic:
    // First try base64url.
    if (!base64URLDecode(task.encodedData.toStringWithoutCopying(), buffer)) {
        // Didn't work, try unescaping and decoding as base64.
        auto unescapedString = decodeURLEscapeSequences(task.encodedData.toStringWithoutCopying());
        if (!base64Decode(unescapedString, buffer, Base64IgnoreSpacesAndNewLines))
            return;
    }

[Chris Dumez]

(In reply to Chris Dumez from comment #3)
> Looking at DataURLDecoder, I see the following logic:
>     // First try base64url.
>     if (!base64URLDecode(task.encodedData.toStringWithoutCopying(), buffer))
> {
>         // Didn't work, try unescaping and decoding as base64.
>         auto unescapedString =
> decodeURLEscapeSequences(task.encodedData.toStringWithoutCopying());
>         if (!base64Decode(unescapedString, buffer,
> Base64IgnoreSpacesAndNewLines))
>             return;
>     }

This was introduced in Bug 148128 when WebKit started doing data URL decoding by itself. Before this, I believe we let CFNetwork decode those for us (No idea exactly how this behaved).

[Antti Koivisto]

It is there to match the earlier CFNetwork behavior. I believe some tests fail if you remove it.

[Anne van Kesteren]

Rob fixed the tests in https://github.com/WebKit/WebKit/commit/7b467a9d964d4214cfa29005135d429279af7d00 it seems, but I don't get the impression the code is shared.

[Anne van Kesteren]

Okay, this in WebLoaderStrategy.cpp seems bad:

    auto mode = DataURLDecoder::Mode::Legacy;
    if (request.requester() == ResourceRequestRequester::Fetch)
        mode = DataURLDecoder::Mode::ForgivingBase64;

Removing DataURLDecoder::Mode::Legacy completely and seeing what breaks seems like a logical next step here.

[Radar WebKit Bug Importer]

<rdar://problem/107982669>

[Anne van Kesteren]

Pull request: https://github.com/WebKit/WebKit/pull/12693

[EWS]

Committed 262976@main (2518c514c679): <https://commits.webkit.org/262976@main>

Reviewed commits have been landed. Closing PR #12693 and removing active labels.