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

JsonSerializer.Deserialize(... JsonSerializerContext context) Ignores configurations in JsonSerializerContext #80634

Closed
trwalke opened this issue Jan 13, 2023 · 6 comments
Labels
area-System.Text.Json bug enhancement Product code improvement that does NOT require public API changes/additions source-generator Indicates an issue with a source generator feature
Milestone

Comments

@trwalke
Copy link

trwalke commented Jan 13, 2023

Description

We have an SDK which provides authentication services to client applications and when using Deserialize(string json, Type returnType, JsonSerializerContext context), the functions seems to not honor the supplied JsonSerializerContext configurations. Specifically NumberHandling = JsonNumberHandling.AllowReadingFromString,

We have an int value that we receive from one of our services that is in the form of a string.

 {  
   "expires_in":"3600",  
 }  

When trying to deserialize with JsonSerializer.Deserialize and a JsonSerializerContext configured with NumberHandling = JsonNumberHandling.AllowReadingFromString,. We get the following error:

System.Text.Json.JsonException
The JSON value could not be converted to System.Int64. Path: $.expires_in | LineNumber: 0 | BytePositionInLine: 1332.
at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, Utf8JsonReader& reader, Exception ex)
at System.Text.Json.Serialization.JsonConverter1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state) at System.Text.Json.Serialization.JsonConverter1.ReadCoreAsObject(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan1 utf8Json, JsonTypeInfo jsonTypeInfo, Nullable1 actualByteCount)
at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan1 json, JsonTypeInfo jsonTypeInfo) at System.Text.Json.JsonSerializer.Deserialize(String json, Type returnType, JsonSerializerContext context) at Microsoft.Identity.Client.Utils.JsonHelper.DeserializeFromJson[T](String json) at Microsoft.Identity.Client.OAuth2.OAuth2Client.CreateResponse[T](HttpResponse response, RequestContext requestContext) at Microsoft.Identity.Client.OAuth2.OAuth2Client.ExecuteRequestAsync[T](Uri endPoint, HttpMethod method, RequestContext requestContext, Boolean expectErrorsOn200OK, Boolean addCommonHeaders, Func2 onBeforePostRequestData)
at Microsoft.Identity.Client.OAuth2.OAuth2Client.GetTokenAsync(Uri endPoint, RequestContext requestContext, Boolean addCommonHeaders, Func2 onBeforePostRequestHandler) at Microsoft.Identity.Client.OAuth2.TokenClient.SendHttpAndClearTelemetryAsync(String tokenEndpoint, ILoggerAdapter logger) at Microsoft.Identity.Client.OAuth2.TokenClient.SendTokenRequestAsync(IDictionary2 additionalBodyParameters, String scopeOverride, String tokenEndpointOverride, CancellationToken cancellationToken)
at Microsoft.Identity.Client.Internal.Requests.UsernamePasswordRequest.GetTokenResponseAsync(CancellationToken cancellationToken)
at Microsoft.Identity.Client.Internal.Requests.UsernamePasswordRequest.ExecuteAsync(CancellationToken cancellationToken)
at Microsoft.Identity.Client.Internal.Requests.RequestBase.RunAsync(CancellationToken cancellationToken)
at Microsoft.Identity.Client.ApiConfig.Executors.PublicClientExecutor.ExecuteAsync(AcquireTokenCommonParameters commonParameters, AcquireTokenByUsernamePasswordParameters usernamePasswordParameters, CancellationToken cancellationToken)

Configuration

Regression?

This works correctly when the client app is targeting .NET 6 but fails in .NET 7 with the above error message. We did not see this mentioned in the breaking changes for .NET 7. Is this intended behavior?

Other information

@mairaw mairaw transferred this issue from dotnet/core Jan 13, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 13, 2023
@ghost
Copy link

ghost commented Jan 13, 2023

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

We have an SDK which provides authentication services to client applications and when using Deserialize(string json, Type returnType, JsonSerializerContext context), the functions seems to not honor the supplied JsonSerializerContext configurations. Specifically NumberHandling = JsonNumberHandling.AllowReadingFromString,

We have an int value that we receive from one of our services that is in the form of a string.

 {  
   "expires_in":"3600",  
 }  

When trying to deserialize with JsonSerializer.Deserialize and a JsonSerializerContext configured with NumberHandling = JsonNumberHandling.AllowReadingFromString,. We get the following error:

System.Text.Json.JsonException
The JSON value could not be converted to System.Int64. Path: $.expires_in | LineNumber: 0 | BytePositionInLine: 1332.
at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, Utf8JsonReader& reader, Exception ex)
at System.Text.Json.Serialization.JsonConverter1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state) at System.Text.Json.Serialization.JsonConverter1.ReadCoreAsObject(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan1 utf8Json, JsonTypeInfo jsonTypeInfo, Nullable1 actualByteCount)
at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan1 json, JsonTypeInfo jsonTypeInfo) at System.Text.Json.JsonSerializer.Deserialize(String json, Type returnType, JsonSerializerContext context) at Microsoft.Identity.Client.Utils.JsonHelper.DeserializeFromJson[T](String json) at Microsoft.Identity.Client.OAuth2.OAuth2Client.CreateResponse[T](HttpResponse response, RequestContext requestContext) at Microsoft.Identity.Client.OAuth2.OAuth2Client.ExecuteRequestAsync[T](Uri endPoint, HttpMethod method, RequestContext requestContext, Boolean expectErrorsOn200OK, Boolean addCommonHeaders, Func2 onBeforePostRequestData)
at Microsoft.Identity.Client.OAuth2.OAuth2Client.GetTokenAsync(Uri endPoint, RequestContext requestContext, Boolean addCommonHeaders, Func2 onBeforePostRequestHandler) at Microsoft.Identity.Client.OAuth2.TokenClient.SendHttpAndClearTelemetryAsync(String tokenEndpoint, ILoggerAdapter logger) at Microsoft.Identity.Client.OAuth2.TokenClient.SendTokenRequestAsync(IDictionary2 additionalBodyParameters, String scopeOverride, String tokenEndpointOverride, CancellationToken cancellationToken)
at Microsoft.Identity.Client.Internal.Requests.UsernamePasswordRequest.GetTokenResponseAsync(CancellationToken cancellationToken)
at Microsoft.Identity.Client.Internal.Requests.UsernamePasswordRequest.ExecuteAsync(CancellationToken cancellationToken)
at Microsoft.Identity.Client.Internal.Requests.RequestBase.RunAsync(CancellationToken cancellationToken)
at Microsoft.Identity.Client.ApiConfig.Executors.PublicClientExecutor.ExecuteAsync(AcquireTokenCommonParameters commonParameters, AcquireTokenByUsernamePasswordParameters usernamePasswordParameters, CancellationToken cancellationToken)

Configuration

Regression?

This works correctly when the client app is targeting .NET 6 but fails in .NET 7 with the above error message. We did not see this mentioned in the breaking changes for .NET 7. Is this intended behavior?

Other information

Author: trwalke
Assignees: -
Labels:

area-System.Text.Json

Milestone: -

@eiriktsarpalis
Copy link
Member

Would it be possible to share minimal reproduction showcasing the issue you are seeing? I tried to reconstruct a repro based on your description and it appears to be working as expected:

string json = """{"expires_in":"3600"}""";
MyPoco result = JsonSerializer.Deserialize(json, MyContext.Default.MyPoco);
Console.WriteLine(result.expires_in);

[JsonNumberHandling(JsonNumberHandling.AllowReadingFromString)]
public class MyPoco
{
    public int expires_in { get; set; }
}

[JsonSerializable(typeof(MyPoco))]
public partial class MyContext : JsonSerializerContext
{
}

@eiriktsarpalis eiriktsarpalis added the needs-author-action An issue or pull request that requires more info or actions from the author. label Jan 16, 2023
@ghost
Copy link

ghost commented Jan 16, 2023

This issue has been marked needs-author-action and may be missing some important information.

@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jan 16, 2023
@pmaytak
Copy link

pmaytak commented Jan 21, 2023

@eiriktsarpalis I think the difference with the repro is we're setting the number handling in the context instance not on poco classes and then passing the context to serialize method.

@eiriktsarpalis
Copy link
Member

Ah yes, so presumably this scenario:

string json = """{"expires_in":"3600"}""";
var context = new MyContext(new JsonSerializerOptions()
{
    NumberHandling = JsonNumberHandling.AllowReadingFromString
});

MyPoco result = JsonSerializer.Deserialize(json, context.MyPoco);
Console.WriteLine(result.expires_in);

public class MyPoco
{
    public int expires_in { get; set; }
}

[JsonSerializable(typeof(MyPoco))]
public partial class MyContext : JsonSerializerContext
{

I think we should consider fixing this. @layomia this seems related to #78010, is this a scenario that the PR is taking into account?

@eiriktsarpalis eiriktsarpalis added enhancement Product code improvement that does NOT require public API changes/additions and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels Jan 23, 2023
@eiriktsarpalis eiriktsarpalis added this to the Future milestone Jan 23, 2023
@eiriktsarpalis eiriktsarpalis added the source-generator Indicates an issue with a source generator feature label Jan 23, 2023
@eiriktsarpalis
Copy link
Member

Fixed by #87484.

@eiriktsarpalis eiriktsarpalis modified the milestones: Future, 8.0.0 Jun 15, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json bug enhancement Product code improvement that does NOT require public API changes/additions source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

No branches or pull requests

3 participants