-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add Base64 parity APIs with Base64Url #123151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Extensions.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Extensions.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Extensions.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Extensions.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Extensions.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Extensions.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Extensions.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Extensions.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Extensions.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Memory/tests/Base64/Base64UnicodeAPIsUnitTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds new APIs to System.Buffers.Text.Base64 to achieve parity with Base64Url, particularly enabling callers to distinguish between InvalidData and DestinationTooSmall when decoding—a capability not available in Convert.TryFromBase64Chars.
Changes:
- Adds 16 new public methods to the Base64 class including encoding/decoding for chars and UTF-8, both throwing and Try-variants
- Introduces helper methods
GetEncodedLengthandGetMaxDecodedLengthas aliases matching Base64Url naming conventions - Implements internal encoder/decoder structs (
Base64EncoderChar,Base64DecoderChar) to support char-based operations
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| System.Runtime.cs | Adds public API surface declarations for all 16 new Base64 methods |
| Base64Extensions.cs | Implements the new APIs using encoder/decoder pattern from Base64Url |
| System.Private.CoreLib.Shared.projitems | Registers the new Base64Extensions.cs file in the build |
| System.Memory.Tests.csproj | Adds the new test file to the test project |
| Base64UnicodeAPIsUnitTests.cs | Provides 39 unit tests covering all new APIs with various scenarios |
| Base64TestHelper.cs | Adds InitializeDecodableChars helper method for test data generation |
src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Extensions.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Extensions.cs
Outdated
Show resolved
Hide resolved
MihaZupan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot please address the new comments
src/libraries/System.Memory/tests/Base64/Base64DecoderUnitTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Memory/tests/Base64/Base64DecoderUnitTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Memory/tests/Base64/Base64DecoderUnitTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Decoder.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Decoder.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Encoder.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Encoder.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Encoder.cs
Outdated
Show resolved
Hide resolved
|
@copilot Ensure that the new APIs are covered in |
Added comprehensive fuzz testing coverage for all new Base64 APIs in Base64Fuzzer.cs:
The fuzzers remain separate since Base64 and Base64Url use different encoding schemes ('+/' vs '-_' and padding behavior), so the encoded outputs differ. However, both now have parallel structure for their char-based APIs. See commit f3969dc. |
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Restored the original whitespace-tolerant decoding implementation for Convert APIs. The |
…h whitespace support Co-authored-by: MihaZupan <25307628+MihaZupan@users.noreply.github.com>
Simplified Convert decoding in commit 3464c7d. Since
Removed |
…ify DecodeFromChars call Co-authored-by: MihaZupan <25307628+MihaZupan@users.noreply.github.com>
|
I'm not sure why Build Analysis was green on this one, there are a bunch of related test failures (fixed by #123260) |
|
I think this broke browser tests |
Interesting, that would mean if a PR adds new tests and those tests are failing, Build Analysis reports the PR as green, and the failures only surface on later PRs. I can’t think of a good reason for that behavior, so this feels like a bug in BA. |
|
On browser this was hidden behind another issue that prevented any tests from running at all. |
…#123260) Fixes #123222 Replaces #123258 From `InvalidDataFallback` we'll call back into `DecodeFrom` to try and process more data. But if the destination is too small and the next block of Base64 contains whitespace, it won't be able to make progress. Use the block-wise fallback in those cases. Tests in #123151 now pass when including this change.
|
From build analysis log: 512 Known test errors not displayed due to space limitations There was very large amount of known MSQUIC failures. The problem is that build Analysis gives up analyzing failures after it hits a threshold but remains green if all failures that have been analyzed are known. |
ToBase64String(byte[])delegates directly toBase64.EncodeToStringToBase64String(ReadOnlySpan<byte>, options)delegates early toBase64.EncodeToStringwhen!insertLineBreaksToBase64CharArrayandTryToBase64CharsuseBase64.EncodeToCharswhen!insertLineBreaksConvertToBase64ArrayInsertLineBreaksnow accepts spans instead of pointers for callersBase64VectorizationLengthThresholdandToBase64CharsLargeNoLineBreaksTryFromBase64Charsnow delegates toBase64.DecodeFromChars(withOperationStatusreturn), setsbytesWritten=0on failure to match previous behaviorFromBase64Stringnow delegates toBase64.DecodeFromChars(s)directlyFromBase64CharArraynow delegates toBase64.DecodeFromCharswith spanConvert.Base64.csentirelySecurity Summary
No security vulnerabilities were detected.
Original prompt
This section details on the original issue you should resolve
<issue_title>[API Proposal]: Base64 parity with Base64Url</issue_title>
<issue_description>EDITED by @stephentoub on 1/12/2026:
Copying members from Base64Url:
Background and motivation
The API on
Base64Urlis very nice. However, there are no equivalents for the things that deal with UTF-16 instead of UTF-8 onBase64and the UTF-16 base64 functionality onConvertis not as nice.Here's an example: using
Convert, I can't tell if it has failed due to invalid input or destination-too-small. This is actually getting in my way right now in a use case where I want decode only some bytes from base64. I went to copy some code that usedBase64Urland was surprised that just dropping theUrlsuffix didn't work.Output: