diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonInputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonInputFormatter.cs index d93260032f..63dadc82b2 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonInputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonInputFormatter.cs @@ -78,23 +78,11 @@ namespace Microsoft.AspNetCore.Mvc.Formatters catch (JsonException jsonException) { var path = jsonException.Path; - if (path.StartsWith("$.", StringComparison.Ordinal)) - { - path = path.Substring(2); - } - - if (path.StartsWith("$[", StringComparison.Ordinal)) - { - path = path.Substring(1); - } - - // Handle path combinations such as ""+"Property", "Parent"+"Property", or "Parent"+"[12]". - var key = ModelNames.CreatePropertyModelName(context.ModelName, path); var formatterException = new InputFormatterException(jsonException.Message, jsonException); var metadata = ModelNames.GetPathMetadata(context.Metadata, path); - context.ModelState.TryAddModelError(key, formatterException, metadata); + context.ModelState.TryAddModelError(path, formatterException, metadata); Log.JsonInputException(_logger, jsonException); diff --git a/src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs b/src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs index af680361ea..fd76501888 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs @@ -19,14 +19,6 @@ namespace Microsoft.AspNetCore.Mvc.Formatters { public abstract class JsonInputFormatterTestBase : LoggedTest { - internal enum Formatter - { - Newtonsoft, - SystemText - } - - internal abstract Formatter CurrentFormatter { get; } - [Theory] [InlineData("application/json", true)] [InlineData("application/*", false)] @@ -116,8 +108,10 @@ namespace Microsoft.AspNetCore.Mvc.Formatters } [Fact] - public async Task JsonFormatter_EscapedKeys_Bracket() + public virtual async Task JsonFormatter_EscapedKeys_Bracket() { + var expectedKey = JsonFormatter_EscapedKeys_Bracket_Expected; + // Arrange var content = "[{\"It[s a key\":1234556}]"; var formatter = GetInputFormatter(); @@ -136,13 +130,15 @@ namespace Microsoft.AspNetCore.Mvc.Formatters formatterContext.ModelState.OrderBy(k => k.Key), kvp => { - Assert.Equal("[0][\'It[s a key\']", kvp.Key); + Assert.Equal(expectedKey, kvp.Key); }); } [Fact] - public async Task JsonFormatter_EscapedKeys() + public virtual async Task JsonFormatter_EscapedKeys() { + var expectedKey = JsonFormatter_EscapedKeys_Expected; + // Arrange var content = "[{\"It\\\"s a key\": 1234556}]"; var formatter = GetInputFormatter(); @@ -162,17 +158,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters formatterContext.ModelState.OrderBy(k => k.Key), kvp => { - switch(CurrentFormatter) - { - case Formatter.Newtonsoft: - Assert.Equal("[0]['It\"s a key']", kvp.Key); - break; - case Formatter.SystemText: - Assert.Equal("[0][\'It\\u0022s a key\']", kvp.Key); - break; - default: - throw new NotImplementedException(); - } + Assert.Equal(expectedKey, kvp.Key); }); } @@ -285,12 +271,19 @@ namespace Microsoft.AspNetCore.Mvc.Formatters var formatterContext = CreateInputFormatterContext(typeof(List), 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.Single(formatterContext.ModelState["[2].Age"].Errors); + Assert.Collection(formatterContext.ModelState.OrderBy(k => k.Key), + kvp => + { + Assert.Equal(expectedKey, kvp.Key); + Assert.Single(kvp.Value.Errors); + }); } [Fact] @@ -304,13 +297,19 @@ namespace Microsoft.AspNetCore.Mvc.Formatters 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, "Model should have had an error!"); - Assert.Single(formatterContext.ModelState["Age"].Errors); + Assert.Collection(formatterContext.ModelState.OrderBy(k => k.Key), + kvp => + { + Assert.Equal(expectedKey, kvp.Key); + Assert.Single(kvp.Value.Errors); + }); } [Fact] @@ -325,12 +324,17 @@ namespace Microsoft.AspNetCore.Mvc.Formatters var formatterContext = CreateInputFormatterContext(typeof(short[]), httpContext); + var expectedValue = ReadAsync_InvalidArray_AddsOverflowErrorsToModelState_Expected; + // Act var result = await formatter.ReadAsync(formatterContext); // Assert Assert.True(result.HasError, "Model should have produced an error!"); - Assert.True(formatterContext.ModelState.ContainsKey("[2]"), "Should have contained key '[2]'"); + Assert.Collection(formatterContext.ModelState.OrderBy(k => k.Key), + kvp => { + Assert.Equal(expectedValue, kvp.Key); + }); } [Fact] @@ -344,6 +348,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters 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); @@ -353,7 +358,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters Assert.Collection( formatterContext.ModelState.OrderBy(k => k.Key), kvp => { - Assert.Equal("names[1].Small", kvp.Key); + Assert.Equal(expectedKey, kvp.Key); Assert.Single(kvp.Value.Errors); }); } @@ -424,12 +429,18 @@ namespace Microsoft.AspNetCore.Mvc.Formatters 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.Single(formatterContext.ModelState["Person.Numbers[2]"].Errors); + Assert.Collection(formatterContext.ModelState.OrderBy(k => k.Key), + kvp => { + Assert.Equal(expectedKey, kvp.Key); + Assert.Single(kvp.Value.Errors); + }); } [Fact] @@ -451,6 +462,20 @@ namespace Microsoft.AspNetCore.Mvc.Formatters 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( diff --git a/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonInputFormatterTest.cs b/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonInputFormatterTest.cs index 9ef9630a45..411725c7a9 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonInputFormatterTest.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonInputFormatterTest.cs @@ -13,8 +13,6 @@ namespace Microsoft.AspNetCore.Mvc.Formatters { public class SystemTextJsonInputFormatterTest : JsonInputFormatterTestBase { - internal override Formatter CurrentFormatter => Formatter.SystemText; - [Fact] public override Task ReadAsync_AddsModelValidationErrorsToModelState() { @@ -46,6 +44,18 @@ namespace Microsoft.AspNetCore.Mvc.Formatters throw new NotImplementedException(); } + [Fact] + public override Task JsonFormatter_EscapedKeys() + { + return base.JsonFormatter_EscapedKeys(); + } + + [Fact] + public override Task JsonFormatter_EscapedKeys_Bracket() + { + return base.JsonFormatter_EscapedKeys_Bracket(); + } + [Fact] public async Task ReadAsync_SingleError() { @@ -65,7 +75,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters formatterContext.ModelState.OrderBy(k => k), kvp => { - Assert.Equal("[1]", kvp.Key); + Assert.Equal("$[1]", kvp.Key); var error = Assert.Single(kvp.Value.Errors); Assert.StartsWith("''' is an invalid start of a value", error.ErrorMessage); }); @@ -75,5 +85,19 @@ namespace Microsoft.AspNetCore.Mvc.Formatters { return new SystemTextJsonInputFormatter(new JsonOptions(), LoggerFactory.CreateLogger()); } + + internal override string ReadAsync_AddsModelValidationErrorsToModelState_Expected => "$.Age"; + + internal override string JsonFormatter_EscapedKeys_Expected => "$[0]['It\\u0022s a key']"; + + internal override string JsonFormatter_EscapedKeys_Bracket_Expected => "$[0]['It[s a key']"; + + internal override string ReadAsync_ArrayOfObjects_HasCorrectKey_Expected => "$[2].Age"; + + internal override string ReadAsync_InvalidArray_AddsOverflowErrorsToModelState_Expected => "$[2]"; + + internal override string ReadAsync_InvalidComplexArray_AddsOverflowErrorsToModelState_Expected => "$[1].Small"; + + internal override string ReadAsync_ComplexPoco_Expected => "$.Person.Numbers[2]"; } } diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonInputFormatterTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonInputFormatterTest.cs index fa29d0289f..d9b1406a05 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonInputFormatterTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonInputFormatterTest.cs @@ -24,8 +24,6 @@ namespace Microsoft.AspNetCore.Mvc.Formatters private static readonly ObjectPoolProvider _objectPoolProvider = new DefaultObjectPoolProvider(); private static readonly JsonSerializerSettings _serializerSettings = new JsonSerializerSettings(); - internal override Formatter CurrentFormatter => Formatter.Newtonsoft; - [Fact] public async Task Constructor_BuffersRequestBody_UsingDefaultOptions() { @@ -198,6 +196,18 @@ namespace Microsoft.AspNetCore.Mvc.Formatters Assert.Equal(settings.DateTimeZoneHandling, actual.DateTimeZoneHandling); } + [Fact] + public override Task JsonFormatter_EscapedKeys() + { + return base.JsonFormatter_EscapedKeys(); + } + + [Fact] + public override Task JsonFormatter_EscapedKeys_Bracket() + { + return base.JsonFormatter_EscapedKeys_Bracket(); + } + [Theory] [InlineData(" ", true, true)] [InlineData(" ", false, false)] @@ -360,6 +370,20 @@ namespace Microsoft.AspNetCore.Mvc.Formatters }); } + internal override string JsonFormatter_EscapedKeys_Expected => "[0]['It\"s a key']"; + + internal override string JsonFormatter_EscapedKeys_Bracket_Expected => "[0][\'It[s a key\']"; + + internal override string ReadAsync_AddsModelValidationErrorsToModelState_Expected => "Age"; + + internal override string ReadAsync_ArrayOfObjects_HasCorrectKey_Expected => "[2].Age"; + + internal override string ReadAsync_ComplexPoco_Expected => "Person.Numbers[2]"; + + internal override string ReadAsync_InvalidComplexArray_AddsOverflowErrorsToModelState_Expected => "names[1].Small"; + + internal override string ReadAsync_InvalidArray_AddsOverflowErrorsToModelState_Expected => "[2]"; + private class Location { public int Id { get; set; }