Skip to content

Commit

Permalink
JSON error handling (#11190)
Browse files Browse the repository at this point in the history 
* Exception handling with SystemTextJsonInputFormatter

* Additional tests

* Update ref package

* PR feedback

* Test fixes and feedback

* Update refs

* Restructure tests

* Cleanup
  • Loading branch information
@pranavkm
pranavkm committed Jun 24, 2019
2 parents 5909583 + f6f1923 commit 151ae52
Show file tree
Hide file tree
Showing 9 changed files with 382 additions and 26 deletions.
2 changes: 1 addition & 1 deletion src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1877,7 +1877,7 @@ public StringOutputFormatter() { }
}
public partial class SystemTextJsonInputFormatter : Microsoft.AspNetCore.Mvc.Formatters.TextInputFormatter, Microsoft.AspNetCore.Mvc.Formatters.IInputFormatterExceptionPolicy
{
public SystemTextJsonInputFormatter(Microsoft.AspNetCore.Mvc.JsonOptions options) { }
public SystemTextJsonInputFormatter(Microsoft.AspNetCore.Mvc.JsonOptions options, Microsoft.Extensions.Logging.ILogger<Microsoft.AspNetCore.Mvc.Formatters.SystemTextJsonInputFormatter> logger) { }
Microsoft.AspNetCore.Mvc.Formatters.InputFormatterExceptionPolicy Microsoft.AspNetCore.Mvc.Formatters.IInputFormatterExceptionPolicy.ExceptionPolicy { get { throw null; } }
public System.Text.Json.JsonSerializerOptions SerializerOptions { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
[System.Diagnostics.DebuggerStepThroughAttribute]
Expand Down
47 changes: 46 additions & 1 deletion src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonInputFormatter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
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
{
Expand All @@ -16,13 +18,19 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
/// </summary>
public class SystemTextJsonInputFormatter : TextInputFormatter, IInputFormatterExceptionPolicy
{
private readonly ILogger<SystemTextJsonInputFormatter> _logger;

/// <summary>
/// Initializes a new instance of <see cref="SystemTextJsonInputFormatter"/>.
/// </summary>
/// <param name="options">The <see cref="JsonOptions"/>.</param>
public SystemTextJsonInputFormatter(JsonOptions options)
/// <param name="logger">The <see cref="ILogger"/>.</param>
public SystemTextJsonInputFormatter(
JsonOptions options,
ILogger<SystemTextJsonInputFormatter> logger)
{
SerializerOptions = options.JsonSerializerOptions;
_logger = logger;

SupportedEncodings.Add(UTF8EncodingWithoutBOM);
SupportedEncodings.Add(UTF16EncodingLittleEndian);
Expand Down Expand Up @@ -67,6 +75,18 @@ public sealed override async Task<InputFormatterResult> ReadRequestBodyAsync(
{
model = await JsonSerializer.ReadAsync(inputStream, context.ModelType, SerializerOptions);
}
catch (JsonException jsonException)
{
var path = jsonException.Path;

var formatterException = new InputFormatterException(jsonException.Message, jsonException);

context.ModelState.TryAddModelError(path, formatterException, context.Metadata);

Log.JsonInputException(_logger, jsonException);

return InputFormatterResult.Failure();
}
finally
{
if (inputStream is TranscodingReadStream transcoding)
Expand All @@ -85,6 +105,7 @@ public sealed override async Task<InputFormatterResult> ReadRequestBodyAsync(
}
else
{
Log.JsonInputSuccess(_logger, context.ModelType);
return InputFormatterResult.Success(model);
}
}
Expand All @@ -98,5 +119,29 @@ private Stream GetInputStream(HttpContext httpContext, Encoding encoding)

return new TranscodingReadStream(httpContext.Request.Body, encoding);
}

private static class Log
{
private static readonly Action<ILogger, string, Exception> _jsonInputFormatterException;
private static readonly Action<ILogger, string, Exception> _jsonInputSuccess;

static Log()
{
_jsonInputFormatterException = LoggerMessage.Define<string>(
LogLevel.Debug,
new EventId(1, "SystemTextJsonInputException"),
"JSON input formatter threw an exception: {Message}");
_jsonInputSuccess = LoggerMessage.Define<string>(
LogLevel.Debug,
new EventId(2, "SystemTextJsonInputSuccess"),
"JSON input formatter succeeded, deserializing to type '{TypeName}'");
}

public static void JsonInputException(ILogger logger, Exception exception)
=> _jsonInputFormatterException(logger, exception.Message, exception);

public static void JsonInputSuccess(ILogger logger, Type modelType)
=> _jsonInputSuccess(logger, modelType.FullName, null);
}
}
}
2 changes: 1 addition & 1 deletion src/Mvc/Mvc.Core/src/Infrastructure/MvcCoreMvcOptionsSetup.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public void Configure(MvcOptions options)
options.Filters.Add(new UnsupportedContentTypeFilter());

// Set up default input formatters.
options.InputFormatters.Add(new SystemTextJsonInputFormatter(_jsonOptions.Value));
options.InputFormatters.Add(new SystemTextJsonInputFormatter(_jsonOptions.Value, _loggerFactory.CreateLogger<SystemTextJsonInputFormatter>()));

// Set up default output formatters.
options.OutputFormatters.Add(new HttpNoContentOutputFormatter());
Expand Down
198 changes: 185 additions & 13 deletions src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,21 @@

using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.IO;
using System.Linq;
using System.Text;
using System.Text.Json;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using Microsoft.Extensions.Logging.Testing;
using Newtonsoft.Json;
using Xunit;

namespace Microsoft.AspNetCore.Mvc.Formatters
{
public abstract class JsonInputFormatterTestBase
public abstract class JsonInputFormatterTestBase : LoggedTest
{
[Theory]
[InlineData("application/json", true)]
Expand Down Expand Up @@ -102,6 +107,61 @@ public async Task JsonFormatterReadsStringValue()
Assert.Equal("abcd", stringValue);
}

[Fact]
public virtual async Task JsonFormatter_EscapedKeys_Bracket()
{
var expectedKey = JsonFormatter_EscapedKeys_Bracket_Expected;

// Arrange
var content = "[{\"It[s a key\":1234556}]";
var formatter = GetInputFormatter();

var contentBytes = Encoding.UTF8.GetBytes(content);
var httpContext = GetHttpContext(contentBytes);

var formatterContext = CreateInputFormatterContext(typeof(IEnumerable<IDictionary<string, short>>), httpContext);

// Act
var result = await formatter.ReadAsync(formatterContext);

// Assert
Assert.True(result.HasError);
Assert.Collection(
formatterContext.ModelState.OrderBy(k => k.Key),
kvp =>
{
Assert.Equal(expectedKey, kvp.Key);
});
}

[Fact]
public virtual async Task JsonFormatter_EscapedKeys()
{
var expectedKey = JsonFormatter_EscapedKeys_Expected;

// Arrange
var content = "[{\"It\\\"s a key\": 1234556}]";
var formatter = GetInputFormatter();

var contentBytes = Encoding.UTF8.GetBytes(content);
var httpContext = GetHttpContext(contentBytes);

var formatterContext = CreateInputFormatterContext(
typeof(IEnumerable<IDictionary<string, short>>), httpContext);

// Act
var result = await formatter.ReadAsync(formatterContext);

// Assert
Assert.True(result.HasError);
Assert.Collection(
formatterContext.ModelState.OrderBy(k => k.Key),
kvp =>
{
Assert.Equal(expectedKey, kvp.Key);
});
}

[Fact]
public virtual async Task JsonFormatterReadsDateTimeValue()
{
Expand Down Expand Up @@ -199,6 +259,33 @@ protected async Task ReadAsync_ReadsValidArray_AsList(Type requestedType)
Assert.Equal(new int[] { 0, 23, 300 }, (IEnumerable<int>)result.Model);
}

[Fact]
public virtual async Task ReadAsync_ArrayOfObjects_HasCorrectKey()
{
// Arrange
var formatter = GetInputFormatter();

var content = "[{\"Age\": 5}, {\"Age\": 3}, {\"Age\": \"Cheese\"} ]";
var contentBytes = Encoding.UTF8.GetBytes(content);
var httpContext = GetHttpContext(contentBytes);

var formatterContext = CreateInputFormatterContext(typeof(List<ComplexModel>), httpContext);

var expectedKey = ReadAsync_ArrayOfObjects_HasCorrectKey_Expected;

// Act
var result = await formatter.ReadAsync(formatterContext);

// Assert
Assert.True(result.HasError, "Model should have had an error!");
Assert.Collection(formatterContext.ModelState.OrderBy(k => k.Key),
kvp =>
{
Assert.Equal(expectedKey, kvp.Key);
Assert.Single(kvp.Value.Errors);
});
}

[Fact]
public virtual async Task ReadAsync_AddsModelValidationErrorsToModelState()
{
Expand All @@ -210,15 +297,19 @@ public virtual async Task ReadAsync_AddsModelValidationErrorsToModelState()
var httpContext = GetHttpContext(contentBytes);

var formatterContext = CreateInputFormatterContext(typeof(ComplexModel), httpContext);
var expectedKey = ReadAsync_AddsModelValidationErrorsToModelState_Expected;

// Act
var result = await formatter.ReadAsync(formatterContext);

// Assert
Assert.True(result.HasError);
Assert.Equal(
"Could not convert string to decimal: not-an-age. Path 'Age', line 1, position 44.",
formatterContext.ModelState["Age"].Errors[0].ErrorMessage);
Assert.True(result.HasError, "Model should have had an error!");
Assert.Collection(formatterContext.ModelState.OrderBy(k => k.Key),
kvp =>
{
Assert.Equal(expectedKey, kvp.Key);
Assert.Single(kvp.Value.Errors);
});
}

[Fact]
Expand All @@ -227,19 +318,23 @@ public virtual async Task ReadAsync_InvalidArray_AddsOverflowErrorsToModelState(
// Arrange
var formatter = GetInputFormatter();

var content = "[0, 23, 300]";
var content = "[0, 23, 33767]";
var contentBytes = Encoding.UTF8.GetBytes(content);
var httpContext = GetHttpContext(contentBytes);

var formatterContext = CreateInputFormatterContext(typeof(byte[]), httpContext);
var formatterContext = CreateInputFormatterContext(typeof(short[]), httpContext);

var expectedValue = ReadAsync_InvalidArray_AddsOverflowErrorsToModelState_Expected;

// Act
var result = await formatter.ReadAsync(formatterContext);

// Assert
Assert.True(result.HasError);
Assert.Equal("The supplied value is invalid.", formatterContext.ModelState["[2]"].Errors[0].ErrorMessage);
Assert.Null(formatterContext.ModelState["[2]"].Errors[0].Exception);
Assert.True(result.HasError, "Model should have produced an error!");
Assert.Collection(formatterContext.ModelState.OrderBy(k => k.Key),
kvp => {
Assert.Equal(expectedValue, kvp.Key);
});
}

[Fact]
Expand All @@ -253,15 +348,19 @@ public virtual async Task ReadAsync_InvalidComplexArray_AddsOverflowErrorsToMode
var httpContext = GetHttpContext(contentBytes);

var formatterContext = CreateInputFormatterContext(typeof(ComplexModel[]), httpContext, modelName: "names");
var expectedKey = ReadAsync_InvalidComplexArray_AddsOverflowErrorsToModelState_Expected;

// Act
var result = await formatter.ReadAsync(formatterContext);

// Assert
Assert.True(result.HasError);
Assert.Equal(
"Error converting value 300 to type 'System.Byte'. Path '[1].Small', line 1, position 69.",
formatterContext.ModelState["names[1].Small"].Errors[0].ErrorMessage);
Assert.Collection(
formatterContext.ModelState.OrderBy(k => k.Key),
kvp => {
Assert.Equal(expectedKey, kvp.Key);
Assert.Single(kvp.Value.Errors);
});
}

[Fact]
Expand Down Expand Up @@ -318,6 +417,65 @@ public async Task ReadAsync_WithInputThatDeserializesToNull_SetsModelOnlyIfAllow
Assert.Null(result.Model);
}

[Fact]
public async Task ReadAsync_ComplexPoco()
{
// Arrange
var formatter = GetInputFormatter();

var content = "{ \"Id\": 5, \"Person\": { \"Name\": \"name\", \"Numbers\": [3, 2, \"Hamburger\"]} }";
var contentBytes = Encoding.UTF8.GetBytes(content);
var httpContext = GetHttpContext(contentBytes);

var formatterContext = CreateInputFormatterContext(typeof(ComplexPoco), httpContext);

var expectedKey = ReadAsync_ComplexPoco_Expected;

// Act
var result = await formatter.ReadAsync(formatterContext);

// Assert
Assert.True(result.HasError, "Model should have had an error!");
Assert.Collection(formatterContext.ModelState.OrderBy(k => k.Key),
kvp => {
Assert.Equal(expectedKey, kvp.Key);
Assert.Single(kvp.Value.Errors);
});
}

[Fact]
public virtual async Task ReadAsync_RequiredAttribute()
{
// Arrange
var formatter = GetInputFormatter();
var content = "{ \"Id\": 5, \"Person\": {\"Numbers\": [3]} }";
var contentBytes = Encoding.UTF8.GetBytes(content);
var httpContext = GetHttpContext(contentBytes);

var formatterContext = CreateInputFormatterContext(typeof(ComplexPoco), httpContext);

// Act
var result = await formatter.ReadAsync(formatterContext);

// Assert
Assert.True(result.HasError, "Model should have had an error!");
Assert.Single(formatterContext.ModelState["Person.Name"].Errors);
}

internal abstract string JsonFormatter_EscapedKeys_Bracket_Expected { get; }

internal abstract string JsonFormatter_EscapedKeys_Expected { get; }

internal abstract string ReadAsync_ArrayOfObjects_HasCorrectKey_Expected { get; }

internal abstract string ReadAsync_AddsModelValidationErrorsToModelState_Expected { get; }

internal abstract string ReadAsync_InvalidArray_AddsOverflowErrorsToModelState_Expected { get; }

internal abstract string ReadAsync_InvalidComplexArray_AddsOverflowErrorsToModelState_Expected { get; }

internal abstract string ReadAsync_ComplexPoco_Expected { get; }

protected abstract TextInputFormatter GetInputFormatter();

protected static HttpContext GetHttpContext(
Expand Down Expand Up @@ -356,6 +514,20 @@ protected static InputFormatterContext CreateInputFormatterContext(
treatEmptyInputAsDefaultValue: treatEmptyInputAsDefaultValue);
}

protected sealed class ComplexPoco
{
public int Id { get; set; }
public Person Person{ get; set; }
}

protected sealed class Person
{
[Required]
[JsonProperty(Required = Required.Always)]
public string Name { get; set; }
public IEnumerable<int> Numbers { get; set; }
}

protected sealed class ComplexModel
{
public string Name { get; set; }
Expand Down
Loading

0 comments on commit 151ae52

Please sign in to comment.