From 1da7a89f5921226d7b87e14f4a6a3d534f57c164 Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Sat, 27 Oct 2018 21:29:28 -0700 Subject: [PATCH] Add `ErrorContext.Member` to `ErrorContext.Path` when clearly needed - #8509 nits: - use `ModelNames.CreatePropertyModelName(...)` - move `exception` assignment up and reuse that variable --- .../JsonInputFormatter.cs | 46 +++++++++++++------ .../JsonInputFormatterTest.cs | 8 ++-- .../InputObjectValidationTests.cs | 8 ++-- 3 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Formatters.Json/JsonInputFormatter.cs b/src/Microsoft.AspNetCore.Mvc.Formatters.Json/JsonInputFormatter.cs index b217ce441e..1d8cdaab45 100644 --- a/src/Microsoft.AspNetCore.Mvc.Formatters.Json/JsonInputFormatter.cs +++ b/src/Microsoft.AspNetCore.Mvc.Formatters.Json/JsonInputFormatter.cs @@ -261,32 +261,48 @@ namespace Microsoft.AspNetCore.Mvc.Formatters { successful = false; - // Handle path combinations such as "" + "Property", "Parent" + "Property", or "Parent" + "[12]". - var key = eventArgs.ErrorContext.Path; - if (!string.IsNullOrEmpty(context.ModelName)) + // When ErrorContext.Path does not include ErrorContext.Member, add Member to form full path. + var path = eventArgs.ErrorContext.Path; + var member = eventArgs.ErrorContext.Member?.ToString(); + var addMember = !string.IsNullOrEmpty(member); + if (addMember) { - if (string.IsNullOrEmpty(eventArgs.ErrorContext.Path)) + // Path.Member case (path.Length < member.Length) needs no further checks. + if (path.Length == member.Length) { - key = context.ModelName; + // Add Member in Path.Memb case but not for Path.Path. + addMember = !string.Equals(path, member, StringComparison.Ordinal); } - else if (eventArgs.ErrorContext.Path[0] == '[') + else if (path.Length > member.Length) { - key = context.ModelName + eventArgs.ErrorContext.Path; - } - else - { - key = context.ModelName + "." + eventArgs.ErrorContext.Path; + // Finally, check whether Path already ends with Member. + if (member[0] == '[') + { + addMember = !path.EndsWith(member, StringComparison.Ordinal); + } + else + { + addMember = !path.EndsWith("." + member, StringComparison.Ordinal); + } } } - var metadata = GetPathMetadata(context.Metadata, eventArgs.ErrorContext.Path); - var modelStateException = WrapExceptionForModelState(eventArgs.ErrorContext.Error); - context.ModelState.TryAddModelError(key, modelStateException, metadata); + if (addMember) + { + path = ModelNames.CreatePropertyModelName(path, member); + } - _logger.JsonInputException(eventArgs.ErrorContext.Error); + // Handle path combinations such as ""+"Property", "Parent"+"Property", or "Parent"+"[12]". + var key = ModelNames.CreatePropertyModelName(context.ModelName, path); exception = eventArgs.ErrorContext.Error; + var metadata = GetPathMetadata(context.Metadata, path); + var modelStateException = WrapExceptionForModelState(exception); + context.ModelState.TryAddModelError(key, modelStateException, metadata); + + _logger.JsonInputException(exception); + // Error must always be marked as handled // Failure to do so can cause the exception to be rethrown at every recursive level and // overflow the stack for x64 CLR processes diff --git a/test/Microsoft.AspNetCore.Mvc.Formatters.Json.Test/JsonInputFormatterTest.cs b/test/Microsoft.AspNetCore.Mvc.Formatters.Json.Test/JsonInputFormatterTest.cs index 30868308ad..801faf62ff 100644 --- a/test/Microsoft.AspNetCore.Mvc.Formatters.Json.Test/JsonInputFormatterTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Formatters.Json.Test/JsonInputFormatterTest.cs @@ -44,7 +44,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/json"; - + var formatterContext = CreateInputFormatterContext(typeof(User), httpContext); // Act @@ -531,7 +531,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters // missing password property here var contentBytes = Encoding.UTF8.GetBytes("{ \"UserName\" : \"John\"}"); var httpContext = GetHttpContext(contentBytes, "application/json;charset=utf-8"); - + var formatterContext = CreateInputFormatterContext(typeof(UserLogin), httpContext); // Act @@ -571,7 +571,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters [InlineData("{\"a\":{\"b\"}}", "a", "Invalid character after parsing property name. Expected ':' but got: }. Path 'a', line 1, position 9.")] [InlineData("{\"age\":\"x\"}", "age", "Could not convert string to decimal: x. Path 'age', line 1, position 10.")] [InlineData("{\"login\":1}", "login", "Error converting value 1 to type 'Microsoft.AspNetCore.Mvc.Formatters.JsonInputFormatterTest+UserLogin'. Path 'login', line 1, position 10.")] - [InlineData("{\"login\":{\"username\":\"somevalue\"}}", "login", "Required property 'Password' not found in JSON. Path 'login', line 1, position 33.")] + [InlineData("{\"login\":{\"username\":\"somevalue\"}}", "login.Password", "Required property 'Password' not found in JSON. Path 'login', line 1, position 33.")] public async Task ReadAsync_WithAllowInputFormatterExceptionMessages_RegistersJsonInputExceptionsAsInputFormatterException( string content, string modelStateKey, @@ -582,7 +582,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters var contentBytes = Encoding.UTF8.GetBytes(content); var httpContext = GetHttpContext(contentBytes); - + var formatterContext = CreateInputFormatterContext(typeof(User), httpContext); // Act diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputObjectValidationTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputObjectValidationTests.cs index a56ee08b8e..a91091265b 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputObjectValidationTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputObjectValidationTests.cs @@ -172,8 +172,10 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public async Task CheckIfExcludedField_IsNotValidatedForNonBodyBoundModels() { // Arrange - var kvps = new List>(); - kvps.Add(new KeyValuePair("Alias", "xyz")); + var kvps = new List> + { + new KeyValuePair("Alias", "xyz"), + }; var content = new FormUrlEncodedContent(kvps); // Act @@ -316,7 +318,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests validationProblemDetails.Errors, error => { - Assert.Empty(error.Key); + Assert.Equal("isbn", error.Key); Assert.Equal(new[] { "Required property 'isbn' not found in JSON. Path '', line 1, position 44." }, error.Value); }); }