Skip to content

Treat FormatExceptions and OverflowExceptions to be treated as model state errors #15035

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

Merged
merged 1 commit into from
Oct 17, 2019

Conversation

pranavkm
Copy link
Contributor

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

…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
@@ -196,8 +196,15 @@ public virtual InputFormatterExceptionPolicy ExceptionPolicy
}
}

if (!(exception is JsonException || exception is OverflowException))
if (!(exception is JsonException || exception is OverflowException || exception is FormatException))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dug around and OverflowException made the list because there was a test that broke if it wasn't allowed. Since it looks like an oversight, I don't think there's a compelling reason to put this behind a compat flag. The SystemTextJson one was a similar oversight.

Thoughts?

@javiercn javiercn added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Oct 16, 2019
@pranavkm
Copy link
Contributor Author

I spoke to @dougbu offline about this and he agreed this looked like the right change. Merging this in

@pranavkm pranavkm merged commit c76df96 into release/3.1 Oct 17, 2019
@pranavkm pranavkm deleted the prkrishn/14504 branch October 17, 2019 17:06
@pranavkm pranavkm added this to the 3.1.0-preview2 milestone Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants