Skip to content
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

JSON src-gen fast-path does not consider writer Encoder option #59424

Open
Tracked by #77019
layomia opened this issue Sep 21, 2021 · 2 comments
Open
Tracked by #77019

JSON src-gen fast-path does not consider writer Encoder option #59424

layomia opened this issue Sep 21, 2021 · 2 comments
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions Priority:2 Work that is important, but not critical for the release source-generator Indicates an issue with a source generator feature
Milestone

Comments

@layomia
Copy link
Contributor

layomia commented Sep 21, 2021

When writing via the serializer e.g. JsonSerializer.Serialize(foo, context.Foo), fast-path logic does not consider the encoder on the writer (via Utf8JsonWriter.Options.Encoder). This means property names would be written with default encoding and values could be written with a custom encoder. This mismatch can be considered invalid - we should disallow the use of a custom encoder on the writer to align with supported features for fast-path.

We should also verify the behavior of both metadata and reflection-based serializers if/when JsonSerializerOptions.Encoder and Utf8JsonWriter.Options.Encoder differ.

Calling fast-path logic directly e.g. context.Foo.SerializeHandler(writer, foo) with writer encoder info that differs from options specified ahead of time via [JsonSourceGenerationOptions] is considered a user error. The raw serialize logic will not perform any options validation.

cc @eiriktsarpalis @steveharter for any thoughts.

@layomia layomia added this to the 7.0.0 milestone Sep 21, 2021
@layomia layomia self-assigned this Sep 21, 2021
@ghost
Copy link

ghost commented Sep 21, 2021

Tagging subscribers to this area: @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

When writing via the serializer e.g. JsonSerializer.Serialize(foo, context.Foo), fast-path logic does not consider the encoder on the writer (via Utf8JsonWriter.Options.Encoder). This means property names would be written with default encoding and values could be written with a custom encoder. This mismatch can be considered invalid - we should disallow the use of a custom encoder on the writer to align with supported features for fast-path.

We should also verify the behavior of both metadata and reflection-based serializers if/when JsonSerializerOptions.Encoder and Utf8JsonWriter.Options.Encoder differ.

Calling fast-path logic directly e.g. context.Foo.SerializeHandler(writer, foo) with writer encoder info that differs from options specified ahead of time via [JsonSourceGenerationOptions] is considered a user error. The raw serialize logic will not perform any options validation.

cc @eiriktsarpalis @steveharter for any thoughts.

Author: layomia
Assignees: layomia
Labels:

area-System.Text.Json

Milestone: 7.0.0

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Sep 21, 2021
@layomia layomia added Priority:2 Work that is important, but not critical for the release and removed untriaged New issue has not been triaged by the area owner labels Sep 21, 2021
@layomia layomia added the source-generator Indicates an issue with a source generator feature label Sep 22, 2021
@eiriktsarpalis eiriktsarpalis added the enhancement Product code improvement that does NOT require public API changes/additions label Oct 14, 2021
@eiriktsarpalis eiriktsarpalis modified the milestones: 7.0.0, Future Apr 18, 2022
@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Apr 18, 2022

Moving to Future, as we won't have time to work on this in the .NET 7 timeframe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions Priority:2 Work that is important, but not critical for the release source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

No branches or pull requests

2 participants