[WTF] allow `Base64EncodeMode::URL` to include padding #29530

dcrousso · 2024-06-05T05:41:53Z

`1d7c9c3`

[WTF] allow `Base64EncodeMode::URL` to include padding
https://bugs.webkit.org/show_bug.cgi?id=275141

Reviewed by Yusuke Suzuki.

Technically "base64url" is the exact same as "base64" but the alphabet slightly adjusted (e.g. `_` instead of `/`), and "base64" by default includes padding, so "base64url" should too.

This will assist with implementing `Uint8Array.prototype.toBase64` since that expects padding to be included regardless of the alphabet <https://webkit.org/b/274635>.

* Source/WTF/wtf/text/Base64.h:
(WTF::base64Encode):
(WTF::base64EncodeToVector):
(WTF::base64EncodeToString):
(WTF::base64EncodeToStringReturnNullIfOverflow):
(WTF::base64URLEncodeToVector):
(WTF::base64URLEncodeToString):
(WTF::base64Encoded):
(WTF::calculateBase64EncodedSize):
(WTF::base64URLEncoded):
(WTF::StringTypeAdapter<Base64Specification>::writeTo const):
* Source/WTF/wtf/text/Base64.cpp:
(WTF::base64EncodeInternal):
(WTF::base64Encode):
(WTF::base64EncodeToVector):
(WTF::base64EncodeToString):
(WTF::base64EncodeToStringReturnNullIfOverflow):
Introduce a `Base64EncodeOption` that's wrapped in an `OptionSet` (for future extensibility) as a parameter for adjusting base64 encoding.
Convert `Base64EncodeMode::URL` into `Base64EncodeOption::URL`.
Add a new `Base64EncodeOption::OmitPadding` that controls whether padding is included instead of assuming it should be omitted if `Base64EncodeOption::URL`.

* Tools/TestWebKitAPI/Tests/WTF/Base64.cpp: Added.
(TestWebKitAPI::TEST(Base64, Encode)):
(TestWebKitAPI::TEST(Base64, EncodeOmitPadding)):
(TestWebKitAPI::TEST(Base64, EncodeURL)):
(TestWebKitAPI::TEST(Base64, EncodeURLOmitPadding)):

* Tools/TestWebKitAPI/CMakeLists.txt:
* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:

Canonical link: https://commits.webkit.org/280647@main

f082f21

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	❌ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	❌ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	❌ 🛠 gtk
❌ 🛠 🧪 jsc-arm64	⏳ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	⏳ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
✅ 🛠 🧪 merge	⏳ 🧪 vision-wk2		✅ 🛠 jsc-armv7
	✅ 🛠 tv		✅ 🧪 jsc-armv7-tests
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-06-05T06:16:59Z

Starting EWS tests for cdcee4b. Live statuses available at the PR page, #29530

webkit-early-warning-system · 2024-06-05T06:17:00Z

EWS run on previous version of this PR (hash 9e885ca)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
~~🧪 style~~	🛠 ios	🛠 mac	✅ 🛠 wpe	⏳ 🛠 wincairo
~~🧪 bindings~~	~~🛠 ios-sim~~	⏳ 🛠 mac-AS-debug	🧪 wpe-wk2	⏳ 🧪 wincairo-tests
~~🧪 webkitperl~~	~~🧪 ios-wk2~~	🧪 api-mac	✅ 🧪 api-wpe
	~~🧪 ios-wk2-wpt~~	🧪 mac-wk1	🛠 wpe-cairo
⏳ 🛠 🧪 jsc	~~🧪 api-ios~~	🧪 mac-wk2	✅ 🛠 gtk
🛠 🧪 jsc-arm64	⏳ 🛠 tv	⏳ 🧪 mac-AS-debug-wk2	🧪 gtk-wk2
🧪 services	⏳ 🛠 tv-sim	🧪 mac-wk2-stress	~~🧪 api-gtk~~
	⏳ 🛠 watch		✅ 🛠 jsc-armv7
	⏳ 🛠 watch-sim		✅ 🧪 jsc-armv7-tests

webkit-early-warning-system · 2024-06-05T06:17:00Z

Starting EWS tests for 4189bd1. Live statuses available at the PR page, #29530

webkit-early-warning-system · 2024-06-05T06:17:01Z

EWS run on previous version of this PR (hash cdcee4b)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	~~🛠 ios~~	~~🛠 mac~~	~~🛠 wpe~~	~~🛠 wincairo~~
~~🧪 bindings~~	~~🛠 ios-sim~~	⏳ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	~~🧪 wincairo-tests~~
~~🧪 webkitperl~~	~~🧪 ios-wk2~~	~~🧪 api-mac~~	~~🧪 api-wpe~~
	~~🧪 ios-wk2-wpt~~	~~🧪 mac-wk1~~	~~🛠 wpe-cairo~~
~~🛠 🧪 jsc~~	~~🧪 api-ios~~	~~🧪 mac-wk2~~	~~🛠 gtk~~
~~🛠 🧪 jsc-arm64~~	⏳ 🛠 tv	⏳ 🧪 mac-AS-debug-wk2	~~🧪 gtk-wk2~~
	⏳ 🛠 tv-sim	~~🧪 mac-wk2-stress~~	~~🧪 api-gtk~~
	⏳ 🛠 watch		~~🛠 jsc-armv7~~
	⏳ 🛠 watch-sim		~~🧪 jsc-armv7-tests~~

webkit-early-warning-system · 2024-06-05T06:17:01Z

Starting EWS tests for c6a36bb. Live statuses available at the PR page, #29530

webkit-early-warning-system · 2024-06-05T20:15:53Z

Starting EWS tests for c6a36bb. Live statuses available at the PR page, #29530

webkit-early-warning-system · 2024-06-06T02:55:31Z

Starting EWS tests for c7922b9. Live statuses available at the PR page, #29530

webkit-early-warning-system · 2024-06-06T02:55:31Z

EWS run on previous version of this PR (hash c6a36bb)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	❌ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc	⏳ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	❌ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	⏳ 🛠 watch		✅ 🛠 jsc-armv7
	✅ 🛠 watch-sim		✅ 🧪 jsc-armv7-tests

webkit-early-warning-system · 2024-06-06T03:27:20Z

Starting EWS tests for c7922b9. Live statuses available at the PR page, #29530

webkit-early-warning-system · 2024-06-06T03:28:44Z

Starting EWS tests for c7922b9. Live statuses available at the PR page, #29530

annevk · 2024-06-10T07:11:34Z

Hmm, part of the premise of this JS API was that it wouldn't require implementations to support more permutations. I guess this is not too bad, but it still seems unfortunate.

Any reason not to just make it three modes?

Default
URLWithoutPadding
URL

I don't think you ever want the Default without padding.

annevk · 2024-06-10T07:13:06Z

Also some test coverage would be good.

dcrousso · 2024-06-10T16:36:37Z

Hmm, part of the premise of this JS API was that it wouldn't require implementations to support more permutations. I guess this is not too bad, but it still seems unfortunate.

Any reason not to just make it three modes?

Default

URLWithoutPadding

URL

I don't think you ever want the Default without padding.

actually, there's currently some discussion around having an omitPadding option for both alphabet: "base64" and alphabet: "base64url" tc39/proposal-arraybuffer-base64#59

i think an options set is probably preferred as it allows for far more flexibility in the future if new things are added

webkit-early-warning-system · 2024-06-11T04:34:00Z

EWS run on previous version of this PR (hash 2dd13f7)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🛠 watch		✅ 🛠 jsc-armv7
	✅ 🛠 watch-sim		✅ 🧪 jsc-armv7-tests

webkit-early-warning-system · 2024-06-11T04:34:01Z

EWS run on previous version of this PR (hash c7922b9)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🛠 watch		✅ 🛠 jsc-armv7
	✅ 🛠 watch-sim		✅ 🧪 jsc-armv7-tests

webkit-early-warning-system · 2024-06-18T00:31:30Z

EWS run on previous version of this PR (hash 25a8349)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc	⏳ 🧪 api-ios	❌ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	⏳ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	⏳ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	⏳ 🧪 vision-wk2		~~🛠 jsc-armv7~~
	✅ 🛠 tv		~~🧪 jsc-armv7-tests~~
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-06-18T03:40:04Z

EWS run on previous version of this PR (hash 1b70f97)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc	~~🧪 api-ios~~	✅ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	⏳ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	⏳ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	⏳ 🧪 vision-wk2		~~🛠 jsc-armv7~~
	✅ 🛠 tv		~~🧪 jsc-armv7-tests~~
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-06-18T06:38:22Z

EWS run on current version of this PR (hash f082f21)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	❌ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	❌ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	❌ 🛠 gtk
❌ 🛠 🧪 jsc-arm64	⏳ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	⏳ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
✅ 🛠 🧪 merge	⏳ 🧪 vision-wk2		✅ 🛠 jsc-armv7
	✅ 🛠 tv		✅ 🧪 jsc-armv7-tests
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

Constellation

r=me

https://bugs.webkit.org/show_bug.cgi?id=275141 Reviewed by Yusuke Suzuki. Technically "base64url" is the exact same as "base64" but the alphabet slightly adjusted (e.g. `_` instead of `/`), and "base64" by default includes padding, so "base64url" should too. This will assist with implementing `Uint8Array.prototype.toBase64` since that expects padding to be included regardless of the alphabet <https://webkit.org/b/274635>. * Source/WTF/wtf/text/Base64.h: (WTF::base64Encode): (WTF::base64EncodeToVector): (WTF::base64EncodeToString): (WTF::base64EncodeToStringReturnNullIfOverflow): (WTF::base64URLEncodeToVector): (WTF::base64URLEncodeToString): (WTF::base64Encoded): (WTF::calculateBase64EncodedSize): (WTF::base64URLEncoded): (WTF::StringTypeAdapter<Base64Specification>::writeTo const): * Source/WTF/wtf/text/Base64.cpp: (WTF::base64EncodeInternal): (WTF::base64Encode): (WTF::base64EncodeToVector): (WTF::base64EncodeToString): (WTF::base64EncodeToStringReturnNullIfOverflow): Introduce a `Base64EncodeOption` that's wrapped in an `OptionSet` (for future extensibility) as a parameter for adjusting base64 encoding. Convert `Base64EncodeMode::URL` into `Base64EncodeOption::URL`. Add a new `Base64EncodeOption::OmitPadding` that controls whether padding is included instead of assuming it should be omitted if `Base64EncodeOption::URL`. * Tools/TestWebKitAPI/Tests/WTF/Base64.cpp: Added. (TestWebKitAPI::TEST(Base64, Encode)): (TestWebKitAPI::TEST(Base64, EncodeOmitPadding)): (TestWebKitAPI::TEST(Base64, EncodeURL)): (TestWebKitAPI::TEST(Base64, EncodeURLOmitPadding)): * Tools/TestWebKitAPI/CMakeLists.txt: * Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj: Canonical link: https://commits.webkit.org/280647@main

webkit-commit-queue · 2024-07-04T00:00:34Z

Committed 280647@main (1d7c9c3): https://commits.webkit.org/280647@main

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

dcrousso requested review from cdumez and zdobersek as code owners June 5, 2024 05:41

dcrousso self-assigned this Jun 5, 2024

dcrousso force-pushed the eng/275141 branch from cdcee4b to 4189bd1 Compare June 5, 2024 05:42

dcrousso mentioned this pull request Jun 5, 2024

[JSC] add support for Uint8Array.prototype.toBase64 and Uint8Array.prototype.toHex #29039

Merged

dcrousso force-pushed the eng/275141 branch 2 times, most recently from 9e885ca to c6a36bb Compare June 5, 2024 06:09

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 5, 2024

dcrousso removed the merging-blocked Applied to prevent a change from being merged label Jun 6, 2024

dcrousso force-pushed the eng/275141 branch from c6a36bb to c7922b9 Compare June 6, 2024 02:39

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 6, 2024

dcrousso removed the merging-blocked Applied to prevent a change from being merged label Jun 11, 2024

dcrousso force-pushed the eng/275141 branch from c7922b9 to 2dd13f7 Compare June 11, 2024 04:33

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 11, 2024

dcrousso removed the merging-blocked Applied to prevent a change from being merged label Jun 18, 2024

dcrousso force-pushed the eng/275141 branch from 2dd13f7 to 25a8349 Compare June 18, 2024 00:31

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 18, 2024

dcrousso removed the merging-blocked Applied to prevent a change from being merged label Jun 18, 2024

dcrousso force-pushed the eng/275141 branch from 25a8349 to 1b70f97 Compare June 18, 2024 03:39

dcrousso force-pushed the eng/275141 branch from 1b70f97 to f082f21 Compare June 18, 2024 06:38

Constellation approved these changes Jul 3, 2024

View reviewed changes

dcrousso added the merge-queue Applied to send a pull request to merge-queue label Jul 3, 2024

webkit-commit-queue force-pushed the eng/275141 branch from f082f21 to 1d7c9c3 Compare July 4, 2024 00:00

webkit-commit-queue merged commit 1d7c9c3 into WebKit:main Jul 4, 2024

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jul 4, 2024

dcrousso deleted the eng/275141 branch July 4, 2024 00:51

[WTF] allow Base64EncodeMode::URL to include padding #29530

[WTF] allow Base64EncodeMode::URL to include padding #29530

Uh oh!

Conversation

dcrousso commented Jun 5, 2024 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1d7c9c3

Uh oh!

webkit-early-warning-system commented Jun 5, 2024

Uh oh!

webkit-early-warning-system commented Jun 5, 2024

Uh oh!

webkit-early-warning-system commented Jun 5, 2024

Uh oh!

webkit-early-warning-system commented Jun 5, 2024

Uh oh!

webkit-early-warning-system commented Jun 5, 2024

Uh oh!

webkit-early-warning-system commented Jun 5, 2024

Uh oh!

webkit-early-warning-system commented Jun 6, 2024

Uh oh!

webkit-early-warning-system commented Jun 6, 2024

Uh oh!

webkit-early-warning-system commented Jun 6, 2024

Uh oh!

webkit-early-warning-system commented Jun 6, 2024

Uh oh!

annevk commented Jun 10, 2024

Uh oh!

annevk commented Jun 10, 2024

Uh oh!

dcrousso commented Jun 10, 2024

Uh oh!

webkit-early-warning-system commented Jun 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Jun 11, 2024

Uh oh!

webkit-early-warning-system commented Jun 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Jun 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Jun 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Constellation left a comment

Choose a reason for hiding this comment

Uh oh!

webkit-commit-queue commented Jul 4, 2024

Uh oh!

Uh oh!

[WTF] allow `Base64EncodeMode::URL` to include padding #29530

[WTF] allow `Base64EncodeMode::URL` to include padding #29530

dcrousso commented Jun 5, 2024 •

edited by webkit-early-warning-system

Loading

`1d7c9c3`

webkit-early-warning-system commented Jun 11, 2024 •

edited

Loading

webkit-early-warning-system commented Jun 18, 2024 •

edited

Loading

webkit-early-warning-system commented Jun 18, 2024 •

edited

Loading

webkit-early-warning-system commented Jun 18, 2024 •

edited

Loading