We should base64 encode byte[] when writing Json #29299
Comments
|
we'll need to weigh this JSON API against the existing 3.0 json backlog; I know @ahsonkhan had it on his original proposal but it was pushed out of 3.0 based on (no-need-)feedback originally |
|
With JWT/JWK, the
|
|
I will fish out and modify the original base64 api proposal from https://github.com/dotnet/corefx/issues/33552 on the writer which can then be leveraged by the serializer.
On deserializing, sure, since it would know the type is |
|
API Proposal: public sealed partial class Utf8JsonWriter : IDisposable
{
// Alternate names:
// - WriteBytesAsString{Value}
// - Bytes/Base64 to be suffix isn't as clear - WriteStringAsBase64?
// Chose bytes as parameter name following the existing Base64 APIs, maybe bytesValue?
// Can't overload existing WriteString APIs since we already have existing APIs that take ROS<byte>
public void WriteAsBase64String(ReadOnlySpan<byte> utf8PropertyName, ReadOnlySpan<byte> bytes) { }
public void WriteAsBase64String(JsonEncodedText propertyName, ReadOnlySpan<byte> bytes) { }
public void WriteAsBase64String(ReadOnlySpan<char> propertyName,ReadOnlySpan<byte> bytes) { }
public void WriteAsBase64String(string propertyName, ReadOnlySpan<byte> bytes) { }
public void WriteAsBase64StringValue(ReadOnlySpan<byte> bytes) { }
}
public ref partial struct Utf8JsonReader
{
// Alternate names: GetBase64EncodedBytes
// Throws invalid operation exception for invalid base 64 string, should it throw format exception?
public byte[] GetBytes() { throw null; }
public bool TryGetBytes(out byte[] value) { throw null; }
}
public readonly partial struct JsonElement
{
// Alternate names: GetBase64EncodedBytes
// Throws invalid operation exception for invalid base 64 string, should it throw format exception?
public byte[] GetBytes() { throw null; }
public bool TryGetBytes(out byte[] value) { throw null; }
} |
|
Do you expect to escape the base64 string? |
Good question, and yes, we would follow the default escaping behavior. |
|
|
Re-opening since the (de)serializer work is still left. The APIs have been added. |
this means data -> base64 -> escaping? |
Not currently, but if you have suggestions or perf improvement strategy, I would be interested in how we can leverage them. The only concern is we need to be able to use the user-provided |
|
Outline of the idea: When the base64-PR got merged into the json-serializer, I'll try to prototype something. * base64 is |
|
Remainder of the behavioral change was fixed in dotnet/corefx#38106. |
We likely want to stick to base64 encoding as the default (even though base64url has the benefit of not requiring escaping by default), to remain consistent with user expectations here. These encodings are not easily interchangeable, so it will break existing usage which depends on reading/writing base64.
Sounds good. If ready, we can use it as an internal implementation detail of the Json library for 3.0, especially if the pef wins are obvious. |
|
Hm, my initial thought of special handling while base64-encoding may not be the best route. Especially since we got simd-based base64, as this relies on a A better solution may be to introduce a So if the user doesn't provide a custom Is this a way to pursue? |
|
For the
So this looks quite promising, and if this route can be gone, the plan would be:
|
Sounds good to me. Can you also establish a baseline by submitting writer (and serializer/deserializer) perf test related to base64 encoded byte arrays to dotnet/performance? We should check that in and then add the perf improvements. Add something similar to these: |
|
Will do this during this week. I'll CC you on the PR then. |
ghost
locked as resolved and limited conversation to collaborators
When writing/serializing a
byte[]to Json we were expecting to see a base64 encoded string.Similarly when reading/deserializing, a base64 encoded string should be readable into a
byte[].The text was updated successfully, but these errors were encountered: