huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Treat FormatExceptions and OverflowExceptions to be treated as model 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 https://github.com/aspnet/AspNetCore/issues/14504
This commit is contained in:
Pranav K 2019-10-17 10:06:57 -07:00 committed by GitHub
parent 1b2c44313b
commit c76df96c70
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 190 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonInputFormatter.cs
View File

@ -8,7 +8,6 @@ using System.Text.Json;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.Formatters.Json;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using Microsoft.Extensions.Logging;
namespace Microsoft.AspNetCore.Mvc.Formatters
@ -87,6 +86,16 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
return InputFormatterResult.Failure();
}
catch (Exception exception) when (exception is FormatException || exception is OverflowException)
{
// The code in System.Text.Json never throws these exceptions. However a custom converter could produce these errors for instance when
// parsing a value. These error messages are considered safe to report to users using ModelState.
context.ModelState.TryAddModelError(string.Empty, exception, context.Metadata);
Log.JsonInputException(_logger, exception);
return InputFormatterResult.Failure();
}
finally
{
if (inputStream is TranscodingReadStream transcoding)

3
src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs
View File

@ -334,7 +334,8 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
// Assert
Assert.True(result.HasError, "Model should have produced an error!");
Assert.Collection(formatterContext.ModelState.OrderBy(k => k.Key),
kvp => {
kvp =>
{
Assert.Equal(expectedValue, kvp.Key);
});
}

79
src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonInputFormatterTest.cs
View File

@ -5,6 +5,8 @@ using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
using Xunit;
@ -81,6 +83,48 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
});
}
[Fact]
public async Task ReadAsync_DoesNotThrowFormatException()
{
// Arrange
var formatter = GetInputFormatter();
var contentBytes = Encoding.UTF8.GetBytes("{\"dateValue\":\"not-a-date\"}");
var httpContext = GetHttpContext(contentBytes);
var formatterContext = CreateInputFormatterContext(typeof(TypeWithBadConverters), httpContext);
// Act
await formatter.ReadAsync(formatterContext);
Assert.False(formatterContext.ModelState.IsValid);
var kvp = Assert.Single(formatterContext.ModelState);
Assert.Empty(kvp.Key);
var error = Assert.Single(kvp.Value.Errors);
Assert.Equal("The supplied value is invalid.", error.ErrorMessage);
}
[Fact]
public async Task ReadAsync_DoesNotThrowOverflowException()
{
// Arrange
var formatter = GetInputFormatter();
var contentBytes = Encoding.UTF8.GetBytes("{\"shortValue\":\"32768\"}");
var httpContext = GetHttpContext(contentBytes);
var formatterContext = CreateInputFormatterContext(typeof(TypeWithBadConverters), httpContext);
// Act
await formatter.ReadAsync(formatterContext);
Assert.False(formatterContext.ModelState.IsValid);
var kvp = Assert.Single(formatterContext.ModelState);
Assert.Empty(kvp.Key);
var error = Assert.Single(kvp.Value.Errors);
Assert.Equal("The supplied value is invalid.", error.ErrorMessage);
}
protected override TextInputFormatter GetInputFormatter()
{
return new SystemTextJsonInputFormatter(new JsonOptions(), LoggerFactory.CreateLogger<SystemTextJsonInputFormatter>());
@ -99,5 +143,40 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
internal override string ReadAsync_InvalidComplexArray_AddsOverflowErrorsToModelState_Expected => "$[1].Small";
internal override string ReadAsync_ComplexPoco_Expected => "$.Person.Numbers[2]";
private class TypeWithBadConverters
{
[JsonConverter(typeof(DateTimeConverter))]
public DateTime DateValue { get; set; }
[JsonConverter(typeof(ShortConverter))]
public short ShortValue { get; set; }
}
private class ShortConverter : JsonConverter<short>
{
public override short Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
return short.Parse(reader.GetString());
}
public override void Write(Utf8JsonWriter writer, short value, JsonSerializerOptions options)
{
throw new NotImplementedException();
}
}
private class DateTimeConverter : JsonConverter<DateTime>
{
public override DateTime Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
return DateTime.Parse(reader.GetString());
}
public override void Write(Utf8JsonWriter writer, DateTime value, JsonSerializerOptions options)
{
throw new NotImplementedException();
}
}
}
}

9
src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonInputFormatter.cs
View File

@ -196,8 +196,15 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
}
}
if (!(exception is JsonException || exception is OverflowException))
if (!(exception is JsonException || exception is OverflowException || exception is FormatException))
{
// At this point we've already recorded all exceptions as an entry in the ModelStateDictionary.
// We only need to rethrow an exception if we believe it needs to be handled by something further up
// the stack.
// JsonException, OverflowException, and FormatException are assumed to be only encountered when
// parsing the JSON and are consequently "safe" to be exposed as part of ModelState. Everything else
// needs to be rethrown.
var exceptionDispatchInfo = ExceptionDispatchInfo.Capture(exception);
exceptionDispatchInfo.Throw();
}

98
src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonInputFormatterTest.cs
View File

@ -14,6 +14,7 @@ using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.ObjectPool;
using Moq;
using Newtonsoft.Json;
using Newtonsoft.Json.Converters;
using Newtonsoft.Json.Serialization;
using Xunit;
@ -21,8 +22,8 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
{
public class NewtonsoftJsonInputFormatterTest : JsonInputFormatterTestBase
{
private static readonly ObjectPoolProvider _objectPoolProvider = new DefaultObjectPoolProvider();
private static readonly JsonSerializerSettings _serializerSettings = new JsonSerializerSettings();
private readonly ObjectPoolProvider _objectPoolProvider = new DefaultObjectPoolProvider();
private readonly JsonSerializerSettings _serializerSettings = new JsonSerializerSettings();
[Fact]
public async Task Constructor_BuffersRequestBody_UsingDefaultOptions()
@ -144,7 +145,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
var serializerSettings = new JsonSerializerSettings();
// Act
var formatter = new TestableJsonInputFormatter(serializerSettings);
var formatter = new TestableJsonInputFormatter(serializerSettings, _objectPoolProvider);
// Assert
Assert.Same(serializerSettings, formatter.SerializerSettings);
@ -185,7 +186,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
MaxDepth = 2,
DateTimeZoneHandling = DateTimeZoneHandling.RoundtripKind,
};
var formatter = new TestableJsonInputFormatter(settings);
var formatter = new TestableJsonInputFormatter(settings, _objectPoolProvider);
// Act
var actual = formatter.CreateJsonSerializer(null);
@ -304,7 +305,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
}
[Fact]
public async Task ReadAsync_lowInputFormatterExceptionMessages_DoesNotWrapJsonInputExceptions()
public async Task ReadAsync_AllowInputFormatterExceptionMessages_DoesNotWrapJsonInputExceptions()
{
// Arrange
var formatter = new NewtonsoftJsonInputFormatter(
@ -336,10 +337,72 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
Assert.NotEmpty(modelError.ErrorMessage);
}
[Fact]
public async Task ReadAsync_DoesNotRethrowFormatExceptions()
{
// Arrange
_serializerSettings.Converters.Add(new IsoDateTimeConverter());
var formatter = new NewtonsoftJsonInputFormatter(
GetLogger(),
_serializerSettings,
ArrayPool<char>.Shared,
_objectPoolProvider,
new MvcOptions(),
new MvcNewtonsoftJsonOptions());
var contentBytes = Encoding.UTF8.GetBytes("{\"dateValue\":\"not-a-date\"}");
var httpContext = GetHttpContext(contentBytes);
var formatterContext = CreateInputFormatterContext(typeof(TypeWithPrimitives), httpContext);
// Act
var result = await formatter.ReadAsync(formatterContext);
// Assert
Assert.True(result.HasError);
Assert.False(formatterContext.ModelState.IsValid);
var modelError = Assert.Single(formatterContext.ModelState["dateValue"].Errors);
Assert.Null(modelError.Exception);
Assert.Equal("The supplied value is invalid.", modelError.ErrorMessage);
}
[Fact]
public async Task ReadAsync_DoesNotRethrowOverflowExceptions()
{
// Arrange
_serializerSettings.Converters.Add(new IsoDateTimeConverter());
var formatter = new NewtonsoftJsonInputFormatter(
GetLogger(),
_serializerSettings,
ArrayPool<char>.Shared,
_objectPoolProvider,
new MvcOptions(),
new MvcNewtonsoftJsonOptions());
var contentBytes = Encoding.UTF8.GetBytes("{\"shortValue\":\"32768\"}");
var httpContext = GetHttpContext(contentBytes);
var formatterContext = CreateInputFormatterContext(typeof(TypeWithPrimitives), httpContext);
// Act
var result = await formatter.ReadAsync(formatterContext);
// Assert
Assert.True(result.HasError);
Assert.False(formatterContext.ModelState.IsValid);
var modelError = Assert.Single(formatterContext.ModelState["shortValue"].Errors);
Assert.Null(modelError.Exception);
Assert.Equal("The supplied value is invalid.", modelError.ErrorMessage);
}
private class TestableJsonInputFormatter : NewtonsoftJsonInputFormatter
{
public TestableJsonInputFormatter(JsonSerializerSettings settings)
: base(GetLogger(), settings, ArrayPool<char>.Shared, _objectPoolProvider, new MvcOptions(), new MvcNewtonsoftJsonOptions())
public TestableJsonInputFormatter(JsonSerializerSettings settings, ObjectPoolProvider objectPoolProvider)
: base(GetLogger(), settings, ArrayPool<char>.Shared, objectPoolProvider, new MvcOptions(), new MvcNewtonsoftJsonOptions())
{
}
@ -418,5 +481,26 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
[JsonProperty(Required = Required.Always)]
public string Password { get; set; }
}
public class TypeWithPrimitives
{
public DateTime DateValue { get; set; }
[JsonConverter(typeof(IncorrectShortConverter))]
public short ShortValue { get; set; }
}
private class IncorrectShortConverter : JsonConverter<short>
{
public override short ReadJson(JsonReader reader, Type objectType, short existingValue, bool hasExistingValue, JsonSerializer serializer)
{
return short.Parse(reader.Value.ToString());
}
public override void WriteJson(JsonWriter writer, short value, JsonSerializer serializer)
{
throw new NotImplementedException();
}
}
}
}