Asp.Net Core does not properly handle the error with polymorphic deserialization from STJ when type discriminator is not specified

[ilya-scale]

Description

When type discriminator is not specified in the request, the error is not handled and 500 is returned instead of 400.

Example request 1:

{
	"type": "unknown",
	"someOtherProperty": "value"
}

Example request 2:

{
	"someOtherProperty": "value"
}

Controller action:

[HttpPost("test")]
public void Test(Base request)
{
        
}

Models:

[JsonPolymorphic(TypeDiscriminatorPropertyName = "type")]
[JsonDerivedType(typeof(Derived), "derived")]
public abstract record Base
{
}

public record Derived : Base
{
}

Reproduction Steps

1. Create a controller with the action and models from Description
2. Do a call with the example request 1, response is 400

{
	"type": "https://tools.ietf.org/html/rfc9110#section-15.5.1",
	"title": "One or more validation errors occurred.",
	"status": 400,
	"errors": {
		"$": [
			"Read unrecognized type discriminator id 'unknown'. Path: $ | LineNumber: 2 | BytePositionInLine: 21."
		],
		"request": [
			"The request field is required."
		]
	},
	"traceId": "00-a4e0c60070a4c53222d0622acad8d198-e18068d6fe5d5773-00"
}

3. Do a call with the example request 2, response is 500

System.NotSupportedException: Deserialization of types without a parameterless constructor, a singular parameterized constructor, or a parameterized constructor annotated with 'JsonConstructorAttribute' is not supported.

Expected behavior

Ideally I expect the 400 error that will say clearly that the "type" TypeDiscriminator has to be provided (since the base class is abstract it is clear it cannot be constructed).

At the very least some form of 400 with json that tells that json cannot be deserialized instead of 500 (there was nothing "wrong" with the server, but client provided a bad request which is clearly a 400).

Actual behavior

A 500 is returned and the exception is not handled by asp.net core (unlike example 1 request)

Regression?

No response

Known Workarounds

One can probably make class not abstract and do a validation oneself that the "base" class is not supported.

Configuration

.Net 8
MacOS ARM

Other information

It is somewhat related to the case when type discriminator is not the first field. If it is not the first - it will be the same error I think, which should also be handled ideally, but this can be fixed in .Net 9 hopefully.

[ghost]

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

When type discriminator is not specified in the request, the error is not handled and 500 is returned instead of 400.

Example request 1:

{
	"type": "unknown",
	"someOtherProperty": "value"
}

Example request 2:

{
	"someOtherProperty": "value"
}

Controller action:

[HttpPost("test")]
public void Test(Base request)
{
        
}

Models:

[JsonPolymorphic(TypeDiscriminatorPropertyName = "type")]
[JsonDerivedType(typeof(Derived), "derived")]
public abstract record Base
{
}

public record Derived : Base
{
}

Reproduction Steps

1. Create a controller with the action and models from Description
2. Do a call with the example request 1, response is 400

{
	"type": "https://tools.ietf.org/html/rfc9110#section-15.5.1",
	"title": "One or more validation errors occurred.",
	"status": 400,
	"errors": {
		"$": [
			"Read unrecognized type discriminator id 'unknown'. Path: $ | LineNumber: 2 | BytePositionInLine: 21."
		],
		"request": [
			"The request field is required."
		]
	},
	"traceId": "00-a4e0c60070a4c53222d0622acad8d198-e18068d6fe5d5773-00"
}

3. Do a call with the example request 2, response is 500

System.NotSupportedException: Deserialization of types without a parameterless constructor, a singular parameterized constructor, or a parameterized constructor annotated with 'JsonConstructorAttribute' is not supported.

Expected behavior

Ideally I expect the 400 error that will say clearly that the "type" TypeDiscriminator has to be provided (since the base class is abstract it is clear it cannot be constructed).

At the very least some form of 400 with json that tells that json cannot be deserialized instead of 500 (there was nothing "wrong" with the server, but client provided a bad request which is clearly a 400).

Actual behavior

A 500 is returned and the exception is not handled by asp.net core (unlike example 1 request)

Regression?

No response

Known Workarounds

One can probably make class not abstract and do a validation oneself that the "base" class is not supported.

Configuration

.Net 8
MacOS ARM

Other information

No response

Author:	ilya-scale
Assignees:	-
Labels:	area-System.Text.Json, untriaged
Milestone:	-

[eiriktsarpalis]

This has been addressed by dotnet/runtime#97474 and should become available with .NET 9 Preview 1 to be launched today.

[ilya-scale]

@eiriktsarpalis , this is not directly connected to the type discriminator being out of order. In this case the type discriminator is missing completely. Will that issue fix this case as well? (Because it seems it is asp.net core issue, and not directly STJ)

[eiriktsarpalis]

this is not directly connected to the type discriminator being out of order.

The particular PR also improved the error message in the particular use case that you are citing, cf. dotnet/runtime@4b6b478

At the very least some form of 400 with json that tells that json cannot be deserialized instead of 500 (there was nothing "wrong" with the server, but client provided a bad request which is clearly a 400).

Regarding this concern, it is difficult to map deserialization failure modes to validation errors (there are too many of those, serialization fails on the first occurrence, serialization can fail at arbitrary locations in the object graph). Arguably, every deserialization error might be regarded as a validation error so if you wish, you might want to write an exception filter that intercepts JsonExceptions.

[ilya-scale]

Well, the validation does handle the wrong discriminator type somehow in a nice way, would not it be nice that it will handle all the JsonExceptions to return as 400 (if not with all proper fields at least with some generic message)?

[eiriktsarpalis]

I defer to the @dotnet/aspnet-team on whether such a mapping would be desirable, but my impression is that the current behavior is very much intentional: deserialization != validation.

[gregsdennis]

@ilya-scale you might also consider validating with a JSON Schema prior to deserialization. That will provide more thorough error messaging.

Mine is not the only library for this, but you may enjoy it: https://docs.json-everything.net/schema/basics/

[danroth27]

Moving to the aspnetcore repo for triage.

[ilya-scale]

Yes, deserialization is of course not the same as validation, but deserialization is a part of handling the http input, and since that input is wrong (does not conform to the expected model) so that we cannot deserialize, I expect to have a 400 the same way as if the whole json is not properly formatted, etc. Failing with 500 seems a bit unnecessary since we know for fact this was a bad request: json that cannot be deserialized.