Support Base 64 URL #1658
Comments
Dotnet-GitSync-Bot
added
the
untriaged
jkotas
added
area-System.Buffers
api-suggestion
|
What we're considering is no longer base64. It should really be a different API named |
|
@GrabYourPitchforks not exactly - that's merely a canonical name. From an encoding and decoding perspective they are just different alphabets at the lowest level. You literally just exchange 2 characters in the respective encoding and decoding (e.g. 4 numbers). It took me exponentially more time to understand the existing implementation than it did to to make the actual change. Mostly, I wanted to get the discussion going. There are several ways this proposed change could be implemented. If we split out the API into a new type called Base64Url I think that's more than sufficient. That's actually how I implemented it in my solution. After realizing that this is a common scenario, I thought it would be appropriate to formally discuss supporting it for others to leverage. The method signatures between Base64 and Base64Url would be exactly the same or, at least, they certainly can be. If we were to take this approach, then it feels like most of the existing Base64 implementation should be refactored into a some other internal type (ex: Base64Core), which can be shared between the two implementations (since they would both be static classes). The internal implementation would then accept the appropriate alphabet leaving the rest of the encoding and decoding algorithm the same. Thoughts? |
This depends how padding is handled (especially for base64Url). Furthermore the current implementation is vectorized. The same holds for the option with the user specified alphabet. |
|
@gfoidl the padding need not be handled directly in the encoding/decoding, which makes things simpler; the RFC also says as much. I agree, however, that most people will expect the padding to be handled automatically, which is why my original suggestion was to move that responsibility outside of the encoding/decoding process. If we want to include that in the Base64Url class, that's fine by me, but that is merely encapsulating some very basic behavior after encoding and before decoding. Code is worth a thousand words. I've thrown up the gist of what I'm talking about. I'm well aware that the implementation uses vectors to map the alphabets. Like you, I merely changed the mapping of the alphabet here and here (on this line and this line). I didn't change anything else. I suspect you changed things to make it work with .NET Standard, which is not one of my goals. In pure terms of implementing the RFC specification, this is functionally correct. I acknowledge that consumers will expect padding to be automatically handled, which is why you can see everything come together in my rendition of Base64Url.cs. I'm not saying that my implementation should necessarily be the winning design. Step one is to agree that these two things are very, very similar and that it makes sense for the core implementation to not only live together, but that the two have a shared implementation. Exactly which part of API is publicly exposed and what it looks like is wide open for discussion. The specification implies that there could be alternate alphabets. These are the only two I know of. I'm completely onboard with closing the door on open-ended alphabets. There's no need to over-engineer this. I'm content with saying these are the only two alphabets supported and they are internally mapped. If that somehow ever changes in the future, there seems to be a straight forward path as to how that would be achieved. |
|
In general I agree with you.
👍
I don't know to which change you refer. In the linked project (in my last comment) most changes are for perf, and the vectorized approach -- especially for decoding -- is different for base64 and base64Url. |
|
Happy to update the title. What should it say now? "Support Base 64 URL"? I didn't dissect your implementation, but it clearly looked different and I saw compiler directives. Maybe there's something I don't understand about the encoding and decoding process, which would negate my initial assumptions. My understanding of the specification and the vectorized implementation is as follows: EncodingDefine the alphabet as a 64 element vector where each element represents the alphabet character. A bunch of optimized techniques and math are then used to calculate which character to use while encoding. DecodingDefine the decoding map as a 256 (technically only 255 is needed) element vector where each element represents the encoded character which maps back to the corresponding encoding alphabet character. A bunch more optimized techniques and math are used to decode the text. The differences between the encoding and decoding is merely using I'll reiterate that padding need not be handled directly in the encoding/decoding process (refer to my example). Unless I've completely missed something else, that means the only difference between the encoding/decoding process is alphabet-to-vector map. Conceptually, this feels like this can be handled by refactoring to a parameter or something other method. I recognize these are very low-level APIs and performance sensitive. I defer to bigger brains than mine as to what the best approach is, but seems to me that choosing vector A or B or passing it as a ref parameter is quite cheap. I re-reviewed your implementation and it seems you are accounting for the padding directly in the encoding/decoding process. That is not what I'm suggesting, but maybe that's a better way to go. In that case, we end up with two distinct encoding/decoding implementations for each alphabet, where the Base 64 URL implementation directly accounts for padding too. The underlying algorithms are quite complex and advanced. My original thinking was that we don't need different algorithms, just different alphabets where padding can be handled outside of the encoding/decoding process for Base 64 URL (the spec even shows I'm onboard to support two different, optimized algorithms if that's the agreed upon approach, it just seems like it's unnecessary maintenance overhead and complexity. |
Sounds good. We're going down to implementation details...I think it's too early for this. First there should be a consense about the API design / shape. Maybe we mismatch "vectorized". When I mean "vectorized" it's SIMD. You mean "array of data" (here a lookup table (LUT)). For padding: I understood what you suggested 😉 But all users of base64Url (in the .NET landscape) use it without padding (e.g. ASP.NET Core), so this should be handled directly. At leasst with a default param -- so a user could opt-in to keep the padding. Pure scalar code and w/o padding in mind, you're absolutely right that the implementations are equivalent, just with differnt LUTs. Hence it makes sense to share as much code as possible. As said before, it's heavily optimized so I'm afraid it's not that easy without perf-loss or introducing even more complex code by playing JIT-tricks. I'll wait for some info from the API designers here. |
commonsensesoftware
changed the title
|
Title updated. Alright ... we are agreed. I guess we enter the holding pattern for stakeholders to decide. I'm personally not married to any code or design, I'd just like to see all of the perf goodness flow from the lowest levels up, which currently isn't happening. 😉 |
GrabYourPitchforks
removed
the
untriaged
|
Any news on this one? It doesn't seem to be a big deal... A standardized System.Memory.Base64Url will be useful... |
|
(I opened what's effectively a dupe of this as #66841. Oops!) I'd like to propose an alternative API shape for this: instead of the two options above, just create a new class Just having two separate classes that share code lets people make a simpler decision: do I want to encode in base64 or base64url? And having a separate class might also make it easier to have different defaults for padding between base64 and base64url if options are added to control padding. So, I propose option 3: "make it its own class": namespace System.Buffers.Text
{
public static class Base64Url
{
public static System.Buffers.OperationStatus DecodeFromUtf8(System.ReadOnlySpan<byte> utf8, System.Span<byte> bytes, out int bytesConsumed, out int bytesWritten, bool isFinalBlock = true);
public static System.Buffers.OperationStatus DecodeFromUtf8InPlace(System.Span<byte> buffer, out int bytesWritten);
public static System.Buffers.OperationStatus EncodeToUtf8(System.ReadOnlySpan<byte> bytes, System.Span<byte> utf8, out int bytesConsumed, out int bytesWritten, bool isFinalBlock = true);
public static System.Buffers.OperationStatus EncodeToUtf8InPlace(System.Span<byte> buffer, int dataLength, out int bytesWritten);
public static int GetMaxDecodedFromUtf8Length(int length);
public static int GetMaxEncodedToUtf8Length(int length);
}
} |
|
@TravisSpomer the term "alphabet" is the vernacular of the spec RFC 4648. I don't really care what it's called in the code, but some level of symmetry and meaning tends to be self-describing. Where I was really going with in terms of an alphabet is in relation to how the spec calls them out. I'm more than onboard with having a restricted public surface area. My thought process was that there could be the ability to swap out alphabets at a lower level. As the spec calls out, there are more than just 2 possible alphabets. If we're only interested in having these two public approaches, then I don't have any resistance to restricting it that way. There's no sense in over-engineering it. I posted a Gist (above) which is extracted from a working implementation. I didn't post the unit tests for it, but have those handy if we were to move forward with a PR. I believe the only real hold up on this issue is an agreed upon API design. I've contributed my 2¢, but I'm not married to anything. In comparing the implementations between Base64 and Base64Url (as provided thus far), the only real low-level difference is the encoding and decoding maps/table/vectors. Handling padding, be it trim or add, is easily handled at a higher API level. As it relates to this issue, the allocation sizes are exactly the same. Trimming a simple I'm totally onboard with having 2 laser-focused public static classes for I would ❤️ to see this land as a supported API. There's a wide benefit to client and server web stacks, in particular. According to the linked .NET 7 roadmap, this issue is queued on the backlog to potentially land. @jeffhandley, is there something we can do to help drive this? |
|
Sorry, "What's an 'alphabet' in this context?" and the neighboring questions were supposed to be theoretical questions a person trying to comprehend the API would ask, not a literal question. I know what it means here, because I've read through that spec, but a normal person has not and will not. 🙂 |
|
Roger that. Apologies, I didn't mean to spec-splain. 😛 |
|
To be clear, padding will always be produced for encoding and required during decoding if the implementation uses the same vectorization approach that the existing
The encoding and decoding always have to follow The primary reason that padding is excluded from the encoded result is because
I'm not sure I follow this statement. The alphabet is intrinsically part of any of the data encoding schemes outlined in the RFC. The encoded characters cannot simply be updated. The Base 64 and Base 64 Encoding with URL and Filename already use different alphabets. Specifically, the 62nd and 63rd characters for I don't want to rat hole on alphabets - again. I'll be more than happy for The surface area for the proposed changes thus far seem reasonable IMO. 👍🏽 |
|
I did not look into implementation deeply and doesn't want to go deep into implementation detail, plus I am not planning to implement it unless nobody would contribute. The padding handling approach especially based on the existing ASP.NET/Azure implementations we want to replace by adding this new API. Based on that I would still prefer ignore (meant do not require) padding on decode and skip/truncate padding on encode, I will reply to your proposed options inline:
Anyway, the final decisions will be made by the API review board. |
|
Thanks for the API proposal, @buyaa-n. I understand the logic of just replicating the Base64* surface area for Base64Url, but I'm wondering if we should actually instead just put everything into a Base64Url class rather than splitting it across that and Convert. I expect if we were starting over, we wouldn't have the APIs for Base64 on Convert at all (and maybe not Convert at all), and so while we've added more Base64 APIs there, it was only because it was the logical place for span-based versions of string-based versions that already existed. Base64Url is sufficiently it's own thing that I don't think we need to mirror the Base64 APIs directly. Further, I want to make sure we're considering all the different input / output types we want here. Looking for example at https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/dev/src/Microsoft.IdentityModel.Tokens/Base64UrlEncoder.cs and https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/dev/src/Microsoft.IdentityModel.Tokens/Base64UrlEncoding.cs, would we be exposing all the APIs necessary to efficiently go between, say, a UTF16 string to UTF8 bytes through Base64Url encoding and out to a string? Would the proposed APIs enable that to be achieved easily with minimal ceremony, or would it be more complicated for a consumer? Similarly, for an API like https://github.com/Azure/azure-sdk-for-net/blob/f65fb9760cbfd8894d40b3f02419d363690eb3bc/sdk/core/Azure.Core/src/Shared/Base64Url.cs#L32, are we making it easy and efficient to go from a Base64Url UTF16 string through UTF8 to a UTF16 string with minimal ceremony and overheads? |
Sounds good to me
Good point, thank you! Did not really think about that, will look into and update the proposal as needed |
|
There are also examples of this in ASP.NET, e.g. |
We also have two public versions of Base64UrlTextEncoder that pass through to WebEncoders, but I'm okay with deprecating all of them if its usage can be trivially replaced by the new Base64Url and/or Convert APIs. I'm not sure if it'd ever be worth it to actually delete anything especially if they all pass through to the runtime APIs. We only just recently in .NET 8 started shipping it as a package again to unblock customers who wanted to use it without an ASP.NET Core framework reference. The scenario of doing Obviously, we'd still want to support cc: @keegan-caruso who just pinged me about improving the perf of Base 64 URL decoding in IdentityModel. |
|
Wanting to insert new lines into Base64Url seems odd - do we need the If we're embracing the new type over the split with |
Good point
Could you explain why? |
|
After applying the feedbacks the API could look like this: namespace System.Buffers.Text
{
public static class Base64Url
{
// Encoding APIs, could just rename all with 'Encode' or 'TryEncode' without To***
public static string EncodeToString(string str) { }
public static string EncodeToString(byte[] inArray) { }
public static string EncodeToString(byte[] inArray, int offset, int length) { }
public static string EncodeToString(ReadOnlySpan<byte> bytes) { }
public static int EncodeToCharArray(byte[] inArray, int offsetIn, int length, char[] outArray, int offsetOut) { }
public static bool TryEncodeToChars(ReadOnlySpan<byte> bytes, Span<char> chars, out int charsWritten) { }
public static OperationStatus EncodeToUtf8(ReadOnlySpan<byte> bytes, Span<byte> utf8, out int bytesConsumed, out int bytesWritten, bool isFinalBlock = true);
public static OperationStatus EncodeToUtf8InPlace(Span<byte> buffer, int dataLength, out int bytesWritten);
// Decoding APIs, also could rename all 'Decode' or 'TryDecode' without From***
public static string DecodeFromString(string str) { }
public static byte[] DecodeFromString(string str) { }
public static byte[] DecodeFromCharArray(char[] inArray, int offset, int length) { }
public static bool TryDecodeFromChars(ReadOnlySpan<char> chars, Span<byte> bytes, out int bytesWritten) { }
public static bool TryDecodeFromString(string s, Span<byte> bytes, out int bytesWritten) { }
public static OperationStatus DecodeFromUtf8(ReadOnlySpan<byte> utf8, Span<byte> bytes, out int bytesConsumed, out int bytesWritten, bool isFinalBlock = true);
public static OperationStatus DecodeFromUtf8InPlace(Span<byte> buffer, out int bytesWritten);
// Max length APIs
public static int GetMaxDecodedFromUtf8Length(int length);
public static int GetMaxEncodedToUtf8Length(int length);
}
} |
Other than for trying to match other APIs, having both a The The decoded bytes could be anything. It feels odd to have string/chars overloads that assume an encoding, even if it's likely to be UTF8. -public static string EncodeToString(string str) { }
-public static string EncodeToString(byte[] inArray) { }
-public static string EncodeToString(byte[] inArray, int offset, int length) { }
+public static string EncodeToString(ReadOnlySpan<char> chars, Encoding encoding) { }
public static string EncodeToString(ReadOnlySpan<byte> bytes) { }I would add an -public static int EncodeToCharArray(byte[] inArray, int offsetIn, int length, char[] outArray, int offsetOut) { }
public static bool TryEncodeToChars(ReadOnlySpan<byte> bytes, Span<char> chars, out int charsWritten) { Would -public static string DecodeFromString(string str) { }
-public static byte[] DecodeFromString(string str) { }
-public static byte[] DecodeFromCharArray(char[] inArray, int offset, int length) { }
+public static string DecodeToString(ReadOnlySpan<char> chars, Encoding encoding) { }
+public static byte[] DecodeToByteArray(ReadOnlySpan<char> chars) { }You can't overload just on the return type. -public static bool TryDecodeFromChars(ReadOnlySpan<char> chars, Span<byte> bytes, out int bytesWritten) { }
-public static bool TryDecodeFromString(string s, Span<byte> bytes, out int bytesWritten) { }
public static bool TryDecodeFromChars(ReadOnlySpan<char> chars, Span<byte> bytes, out int bytesWritten) { }If you have a Together, that might look something like namespace System.Buffers.Text;
public static class Base64Url
{
// Encode bytes => utf8
public static OperationStatus EncodeToUtf8(ReadOnlySpan<byte> bytes, Span<byte> utf8, out int bytesConsumed, out int bytesWritten, bool isFinalBlock = true);
public static OperationStatus EncodeToUtf8InPlace(Span<byte> buffer, int dataLength, out int bytesWritten);
// Decode from utf8 => bytes
public static OperationStatus DecodeFromUtf8(ReadOnlySpan<byte> utf8, Span<byte> bytes, out int bytesConsumed, out int bytesWritten, bool isFinalBlock = true);
public static OperationStatus DecodeFromUtf8InPlace(Span<byte> buffer, out int bytesWritten);
// Max length APIs
public static int GetMaxDecodedFromUtf8Length(int length);
public static int GetMaxEncodedToUtf8Length(int length);
// IsValid
public static bool IsValid(ReadOnlySpan<char> base64UrlText);
public static bool IsValid(ReadOnlySpan<char> base64UrlText, out int decodedLength);
public static bool IsValid(ReadOnlySpan<byte> base64UrlTextUtf8);
public static bool IsValid(ReadOnlySpan<byte> base64UrlTextUtf8, out int decodedLength);
// Up to this point, this is a mirror of System.Buffers.Text.Base64
// Below are more helpers that bring over functionality similar to Convert.*Base64*
// Encode to / decode from chars
public static bool TryEncodeToChars(ReadOnlySpan<byte> bytes, Span<char> chars, out int charsWritten) { }
public static bool TryDecodeFromChars(ReadOnlySpan<char> chars, Span<byte> bytes, out int bytesWritten) { }
// These are just accelerator methods.
// Should be efficiently implementable on top of the other ones in just a few lines.
// Encode to string
public static string EncodeToString(ReadOnlySpan<char> chars, Encoding encoding) { }
public static string EncodeToString(ReadOnlySpan<byte> bytes) { }
// Decode from chars => string
// Decode from chars => byte[]
// The names could also just be "Decode" without naming the return type
public static string DecodeToString(ReadOnlySpan<char> chars, Encoding encoding) { }
public static byte[] DecodeToByteArray(ReadOnlySpan<char> chars) { }
}That would get you
Do we care about other combinations? |
|
Thank you @MihaZupan, the updates make sense to me, we might want to rename the |
MihaZupan
added
api-ready-for-review
|
I work on the Identity Model project; the current proposal should work for us to remove our implementation. |
|
In IdentityModel, we have scenarios where we will have a base64UrlEncoded JWT as a |
@brentschmaltz, could you share what that would translate to in terms of your ideal API in the proposal above? |
|
@stephentoub consider a protocol, such as SHR which is a JWS inside a JWS (the envelope). When we hit that claim, we will remember where it was in the buffer, and pass a Span to the decoder when it is time to process. We don't have to worry about escaping as the value must be base64url encoded or we will fault on the decoding. Something simple like:
|
|
That would be |
|
Perfect, love it. |
|
Could we make it NS 2.0? This is so heavily used in the Azure SDK and we keep duplicating this all over the place. |
This type is needed for .NET Standard 2.0. System.Memory.nupkg is the most obvious destination, but we may have problems with that which require a net new package. namespace System.Buffers.Text;
public static class Base64Url
{
public static int GetMaxDecodedLength(int base64Length);
public static int GetEncodedLength(int bytesLength);
public static OperationStatus EncodeToUtf8(ReadOnlySpan<byte> source, Span<byte> destination, out int bytesConsumed, out int bytesWritten, bool isFinalBlock = true);
public static int EncodeToUtf8(ReadOnlySpan<byte> source, Span<byte> destination);
public static bool TryEncodeToUtf8(ReadOnlySpan<byte> source, Span<byte> destination, out int charsWritten);
public static byte[] EncodeToUtf8(ReadOnlySpan<byte> source);
public static OperationStatus EncodeToChars(ReadOnlySpan<byte> source, Span<char> destination, out int bytesConsumed, out int charsWritten, bool isFinalBlock = true);
public static int EncodeToChars(ReadOnlySpan<byte> source, Span<char> destination);
public static bool TryEncodeToChars(ReadOnlySpan<byte> source, Span<char> destination, out int charsWritten);
public static char[] EncodeToChars(ReadOnlySpan<byte> source);
public static string EncodeToString(ReadOnlySpan<byte> source);
public static bool TryEncodeToUtf8InPlace(Span<byte> buffer, int dataLength, out int bytesWritten);
public static OperationStatus DecodeFromUtf8(ReadOnlySpan<byte> source, Span<byte> destination, out int bytesConsumed, out int bytesWritten, bool isFinalBlock = true);
public static int DecodeFromUtf8(ReadOnlySpan<byte> source, Span<byte> destination);
public static bool TryDecodeFromUtf8(ReadOnlySpan<byte> source, Span<byte> destination, out int bytesWritten);
public static byte[] DecodeFromUtf8(ReadOnlySpan<byte> source);
public static OperationStatus DecodeFromChars(ReadOnlySpan<char> source, Span<byte> destination, out int charsConsumed, out int bytesWritten, bool isFinalBlock = true);
public static int DecodeFromChars(ReadOnlySpan<char> source, Span<byte> destination);
public static bool TryDecodeFromChars(ReadOnlySpan<char> source, Span<byte> destination, out int bytesWritten);
public static byte[] DecodeFromChars(ReadOnlySpan<char> source);
public static int DecodeFromUtf8InPlace(Span<byte> buffer);
public static bool IsValid(ReadOnlySpan<char> base64UrlText);
public static bool IsValid(ReadOnlySpan<char> base64UrlText, out int decodedLength);
public static bool IsValid(ReadOnlySpan<byte> utf8Base64UrlText);
public static bool IsValid(ReadOnlySpan<byte> utf8Base64UrlText, out int decodedLength);
} |
bartonjs
added
api-approved
Are we going to follow the established |
dotnet-policy-service
bot
added
the
in-pr
and others
Updated by @MihaZupan on 2024-02-27
Proposed API
Original issue
The Base64 implementation in System.Memory provides excellent low-level optimizations for RFC 4648, but it currently uses a fixed alphabet. Minor refactoring would add additional use cases using the existing implementation.
Rationale and Usage
The most obvious use case for this change is to support the encoding variant known as Base 64 URL also described in RFC 4648 §4.
This excerpt from the RFC describes the difference between the standard Base 64 alphabet and the Base 64 URL alphabet.
I have already been able to verify the encoding and decoding in a copy of the current implementation by merely changing the 2 relevant characters in the alphabet mappings at:
Today, this logic is suboptimally implemented in at least:
Furthermore, the encoding is generic and has use cases outside of ASP.NET (e.g. you shouldn't have to reference ASP.NET to use it).
Proposed API Change
The existing
Base64.EncodeToUtf8andBase64.DecodeFromUtf8should each add a new method overload that allows one of the following:A new enumeration of allowed alphabets (ex:
Base64Alphabet) which internally maps to well-known alphabet spansAllow any custom alphabet to be supplied as
ReadOnlySpan<sbyte>that must have an exact length of 64 for encoding and 256 for decodinga. One or more new types could be provided with static properties for the well-known alphabet spans
Details
Option 1 requires less validation, but has less flexibility. Option 2 has more flexibility, including scenarios not described here, but requires more validation before using the alphabet.
Although the standard Base 64 and Base 64 URL are technically the same encoding with different alphabets, Base 64 URL typically does not include padding. RFC 4648 §3.2 indicates this is allowed (as it's explicitly stated), but there is no need to make that concession in this API. Including the
=character for padding in Base 64 URL encoding is still correct.Both approaches would have a similar looking API:
Figure 1: Supply an alphabet enumeration
Figure 2: Supply a custom alphabet
The support for trimming off and re-adding padding should be implemented separately. This could be in a separate
Base64Urlclass or, perhaps more appropriately, added as a new type in System.Text.Encodings.Web.Padding is generally very cheap to deal with compared to other implementation methods. Trimming involves walking the tail end of the span while there are padding characters and then slicing it off. Re-padding fills the end of the span buffer before decoding. Neither operation requires additional allocations.
Open Questions
The text was updated successfully, but these errors were encountered: