Skip to content

NewtonsoftJsonInputFormatter does not threat FormatException as MalformedInputException #14504

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

Closed
ig-sinicyn opened this issue Sep 27, 2019 · 7 comments
Assignees
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one

Comments

@ig-sinicyn
Copy link

ig-sinicyn commented Sep 27, 2019

Describe the bug

NewtonsoftJsonInputFormatter does not handle malformed input at least for DateTime fields.
So, malformed request results in unhandled exception and http 500 response.

To Reproduce

Net core 3, controller:

	public class SomeRequest
	{
		[Required]
		public DateTime RequestData { get; set; }
	}

	public class SomeResponse
	{
		[Required]
		public DateTime RequestData { get; set; }
	}

	[ApiController]
	[Route("testCalls")]
	public class TestCallsController : ControllerBase
	{
		[HttpPost]
		[Route("echo")]
		[Consumes("application/json")]
		public SomeResponse EchoPost([Required] SomeRequest request) // (1)
		{
			return new SomeResponse
			{
				RequestData = request.RequestData
			};
		}
	}

setup:

	services
		.AddControllers()
		.ConfigureApiBehaviorOptions(
			o =>
			{
				o.InvalidModelStateResponseFactory = c =>
				{
					Debugger.Break(); // Did not get called
					return new BadRequestObjectResult(c.ModelState);
				};
			})
		.AddNewtonsoftJson();

UPD, the

		.AddNewtonsoftJson(
			options =>
			{
				options.AllowInputFormatterExceptionMessages = true;
			});

does not help.

request:

curl -X POST "https://localhost:44310/testCalls/echo" -H "accept: text/plain" -H "Content-Type: application/json" -d "{\"requestData\":\"invalidDate\"}"

Actual behavior

http 500, exception stack:

System.FormatException: The string 'invalidDate' was not recognized as a valid DateTime. There is an unknown word starting at index '0'.
   at System.DateTimeParse.Parse(ReadOnlySpan`1 s, DateTimeFormatInfo dtfi, DateTimeStyles styles)
   at System.DateTime.Parse(String s, IFormatProvider provider, DateTimeStyles styles)
   at Newtonsoft.Json.Converters.IsoDateTimeConverter.ReadJson(JsonReader reader, Type objectType, Object existingValue, JsonSerializer serializer)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.DeserializeConvertable(JsonConverter converter, JsonReader reader, Type objectType, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.SetPropertyValue(JsonProperty property, JsonConverter propertyConverter, JsonContainerContract containerContract, JsonProperty containerProperty, JsonReader reader, Object target)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id)
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.AspNetCore.Mvc.Formatters.NewtonsoftJsonInputFormatter.ReadRequestBodyAsync(InputFormatterContext context, Encoding encoding)
   at Microsoft.AspNetCore.Mvc.ModelBinding.Binders.BodyModelBinder.BindModelAsync(ModelBindingContext bindingContext)
   at Microsoft.AspNetCore.Mvc.ModelBinding.ParameterBinder.BindModelAsync(ActionContext actionContext, IModelBinder modelBinder, IValueProvider valueProvider, ParameterDescriptor parameter, ModelMetadata metadata, Object value)
   at Microsoft.AspNetCore.Mvc.Controllers.ControllerBinderDelegateProvider.<>c__DisplayClass0_0.<<CreateBinderDelegate>g__Bind|0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeInnerFilterAsync>g__Awaited|13_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResourceFilter>g__Awaited|24_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResourceExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeFilterPipelineAsync()
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Logged|17_1(ResourceInvoker invoker)
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger)
   at Swashbuckle.AspNetCore.SwaggerUI.SwaggerUIMiddleware.Invoke(HttpContext httpContext)
   at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

Expected behavior

Invalid input should be handled by InvalidModelStateResponseFactory call (http 400, by default).

Additional context

As a hack the formatter may be replaced with any derived class, explanation:

// Microsoft.AspNetCore.Mvc.Formatters.NewtonsoftJsonInputFormatter
public virtual InputFormatterExceptionPolicy ExceptionPolicy
{
	get
	{
		if (GetType() == typeof(NewtonsoftJsonInputFormatter))
		{
			return InputFormatterExceptionPolicy.MalformedInputExceptions;
		}
		return InputFormatterExceptionPolicy.AllExceptions; // FormatException will be handled too
	}
}
@pranavkm pranavkm added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Sep 27, 2019
@rynowak rynowak added the bug This issue describes a behavior which is not expected - a bug. label Sep 27, 2019
@rynowak
Copy link
Member

rynowak commented Sep 27, 2019

This looks like a good 3.1 candidate. @pranavkm - do you know if this is a regression?

@mkArtakMSFT mkArtakMSFT added this to the 3.1.0-preview3 milestone Sep 27, 2019
@mkArtakMSFT
Copy link
Contributor

@pranavkm if this is not a regression, please move to 5.0.

@pranavkm
Copy link
Contributor

pranavkm commented Oct 2, 2019

@ig-sinicyn I can't seem to reproduce your issue. The error is correctly reported as a model state error. Could you share a minimal repro?

@pranavkm pranavkm added Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. and removed bug This issue describes a behavior which is not expected - a bug. labels Oct 2, 2019
@pranavkm
Copy link
Contributor

pranavkm commented Oct 9, 2019

Feel free to reopen if you're able to share a minimal app that reproduces the error.

@pranavkm pranavkm closed this as completed Oct 9, 2019
@pranavkm pranavkm removed this from the 3.1.0-preview2 milestone Oct 9, 2019
@ig-sinicyn
Copy link
Author

@pranavkm sorry for delay. I'll add a repro today.

@ig-sinicyn
Copy link
Author

ig-sinicyn commented Oct 11, 2019

Well, the case was

				services.AddNewtonsoftJson(o =>
				{
					o.SerializerSettings.Converters = new List<JsonConverter>
					{
						new IsoDateTimeConverter()
					};
				});

Repro: ReproJsonContent.zip

Technically it's Newtonsoft.Json issue caused by using a legacy converter.
However there may be third-party converters that throw custom exceptions.

@ig-sinicyn
Copy link
Author

@pranavkm Github does not allow to reopen issues that was closed by repo collaborator, can you reopen it for me?

@pranavkm pranavkm reopened this Oct 11, 2019
@mkArtakMSFT mkArtakMSFT added investigate and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Oct 11, 2019
@mkArtakMSFT mkArtakMSFT added this to the 3.1.0-preview2 milestone Oct 14, 2019
pranavkm added a commit that referenced this issue Oct 15, 2019
…state errors

In SystemTextJsonInputFormatter and NewtonsoftJsonInputFormatter, suppress
FormatExceptions and OverflowExceptions and instead report them as model state errors.

This makes the behavior more consistent between these formatters, model binders, and
the XML formatters

Fixes #14504
@pranavkm pranavkm added enhancement This issue represents an ask for new feature or an enhancement to an existing one Working and removed investigate labels Oct 15, 2019
pranavkm added a commit that referenced this issue Oct 17, 2019
…state errors (#15035)

In SystemTextJsonInputFormatter and NewtonsoftJsonInputFormatter, suppress
FormatExceptions and OverflowExceptions and instead report them as model state errors.

This makes the behavior more consistent between these formatters, model binders, and
the XML formatters

Fixes #14504
@pranavkm pranavkm added Done This issue has been fixed and removed Working labels Oct 17, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one
Projects
None yet
Development

No branches or pull requests

4 participants