From 164d14064cccbeeafe79713eba0b307bdfb1ad38 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Mon, 1 Oct 2018 15:05:56 -0700 Subject: [PATCH] Use casing for ProblemDetails that specified by RFC * Use JsonProperty.MemberName to specify lowercase casing for ProblemDetails properties - https://tools.ietf.org/html/rfc7807#section-3 * Use XML NS and lowercase for Xml elements specified by RFC - https://tools.ietf.org/html/rfc7807#appendix-A Fixes https://github.com/aspnet/Mvc/issues/8501 --- .../ProblemDetails.cs | 10 ++--- .../ValidationProblemDetails.cs | 2 + .../ProblemDetailsWrapper.cs | 24 +++++----- .../ValidationProblemDetailsWrapper.cs | 2 +- .../ProblemDetailsWrapperTest.cs | 18 ++++---- .../ValidationProblemDetailsWrapperTest.cs | 44 +++++++++---------- ...ontractSerializerFormattersWrappingTest.cs | 39 ++++++++++------ .../XmlSerializerFormattersWrappingTest.cs | 39 ++++++++++------ 8 files changed, 104 insertions(+), 74 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ProblemDetails.cs b/src/Microsoft.AspNetCore.Mvc.Core/ProblemDetails.cs index 44b816aa05..9d07004103 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ProblemDetails.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ProblemDetails.cs @@ -18,7 +18,7 @@ namespace Microsoft.AspNetCore.Mvc /// (e.g., using HTML [W3C.REC-html5-20141028]). When this member is not present, its value is assumed to be /// "about:blank". /// - [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] + [JsonProperty(NullValueHandling = NullValueHandling.Ignore, PropertyName = "type")] public string Type { get; set; } /// @@ -26,25 +26,25 @@ namespace Microsoft.AspNetCore.Mvc /// of the problem, except for purposes of localization(e.g., using proactive content negotiation; /// see[RFC7231], Section 3.4). /// - [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] + [JsonProperty(NullValueHandling = NullValueHandling.Ignore, PropertyName = "title")] public string Title { get; set; } /// /// The HTTP status code([RFC7231], Section 6) generated by the origin server for this occurrence of the problem. /// - [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] + [JsonProperty(NullValueHandling = NullValueHandling.Ignore, PropertyName = "status")] public int? Status { get; set; } /// /// A human-readable explanation specific to this occurrence of the problem. /// - [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] + [JsonProperty(NullValueHandling = NullValueHandling.Ignore, PropertyName = "detail")] public string Detail { get; set; } /// /// A URI reference that identifies the specific occurrence of the problem.It may or may not yield further information if dereferenced. /// - [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] + [JsonProperty(NullValueHandling = NullValueHandling.Ignore, PropertyName = "instance")] public string Instance { get; set; } /// diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ValidationProblemDetails.cs b/src/Microsoft.AspNetCore.Mvc.Core/ValidationProblemDetails.cs index 2332367b95..da104c9a9b 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ValidationProblemDetails.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ValidationProblemDetails.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Newtonsoft.Json; namespace Microsoft.AspNetCore.Mvc { @@ -64,6 +65,7 @@ namespace Microsoft.AspNetCore.Mvc /// /// Gets or sets the validation errors associated with this instance of . /// + [JsonProperty(PropertyName = "errors")] public IDictionary Errors { get; } = new Dictionary(StringComparer.Ordinal); } } diff --git a/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/ProblemDetailsWrapper.cs b/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/ProblemDetailsWrapper.cs index 30775bd1b1..f7d2056806 100644 --- a/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/ProblemDetailsWrapper.cs +++ b/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/ProblemDetailsWrapper.cs @@ -12,9 +12,11 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml /// /// Wrapper class for to enable it to be serialized by the xml formatters. /// - [XmlRoot(nameof(ProblemDetails))] + [XmlRoot("problem", Namespace = Namespace)] public class ProblemDetailsWrapper : IXmlSerializable, IUnwrappable { + internal const string Namespace = "urn:ietf:rfc:7807"; + /// /// Key used to represent dictionary elements with empty keys /// @@ -83,25 +85,25 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml switch (name) { - case nameof(ProblemDetails.Detail): + case "detail": ProblemDetails.Detail = value; break; - case nameof(ProblemDetails.Instance): + case "instance": ProblemDetails.Instance = value; break; - case nameof(ProblemDetails.Status): + case "status": ProblemDetails.Status = string.IsNullOrEmpty(value) ? (int?)null : int.Parse(value, CultureInfo.InvariantCulture); break; - case nameof(ProblemDetails.Title): + case "title": ProblemDetails.Title = value; break; - case nameof(ProblemDetails.Type): + case "type": ProblemDetails.Type = value; break; @@ -122,20 +124,20 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml if (!string.IsNullOrEmpty(ProblemDetails.Detail)) { writer.WriteElementString( - XmlConvert.EncodeLocalName(nameof(ProblemDetails.Detail)), + XmlConvert.EncodeLocalName("detail"), ProblemDetails.Detail); } if (!string.IsNullOrEmpty(ProblemDetails.Instance)) { writer.WriteElementString( - XmlConvert.EncodeLocalName(nameof(ProblemDetails.Instance)), + XmlConvert.EncodeLocalName("instance"), ProblemDetails.Instance); } if (ProblemDetails.Status.HasValue) { - writer.WriteStartElement(XmlConvert.EncodeLocalName(nameof(ProblemDetails.Status))); + writer.WriteStartElement(XmlConvert.EncodeLocalName("status")); writer.WriteValue(ProblemDetails.Status.Value); writer.WriteEndElement(); } @@ -143,14 +145,14 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml if (!string.IsNullOrEmpty(ProblemDetails.Title)) { writer.WriteElementString( - XmlConvert.EncodeLocalName(nameof(ProblemDetails.Title)), + XmlConvert.EncodeLocalName("title"), ProblemDetails.Title); } if (!string.IsNullOrEmpty(ProblemDetails.Type)) { writer.WriteElementString( - XmlConvert.EncodeLocalName(nameof(ProblemDetails.Type)), + XmlConvert.EncodeLocalName("type"), ProblemDetails.Type); } diff --git a/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/ValidationProblemDetailsWrapper.cs b/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/ValidationProblemDetailsWrapper.cs index b8787ee0e0..a454fb5d0a 100644 --- a/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/ValidationProblemDetailsWrapper.cs +++ b/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/ValidationProblemDetailsWrapper.cs @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml /// /// Wrapper class for to enable it to be serialized by the xml formatters. /// - [XmlRoot(nameof(ValidationProblemDetails))] + [XmlRoot("problem", Namespace = "urn:ietf:rfc:7807")] public class ValidationProblemDetailsWrapper : ProblemDetailsWrapper, IUnwrappable { private static readonly string ErrorKey = "MVC-Errors"; diff --git a/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/ProblemDetailsWrapperTest.cs b/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/ProblemDetailsWrapperTest.cs index b6760f0f15..88db6289c9 100644 --- a/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/ProblemDetailsWrapperTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/ProblemDetailsWrapperTest.cs @@ -17,14 +17,14 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml { // Arrange var xml = "" + - "" + - "Some title" + - "403" + - "Some instance" + + "" + + "Some title" + + "403" + + "Some instance" + "Test Value 1" + "<_x005B_key2_x005D_>Test Value 2" + "Test Value 3" + - ""; + ""; var serializer = new DataContractSerializer(typeof(ProblemDetailsWrapper)); // Act @@ -76,13 +76,13 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml var wrapper = new ProblemDetailsWrapper(problemDetails); var outputStream = new MemoryStream(); var expectedContent = "" + - "" + - "Some detail" + - "Some title" + + "" + + "Some detail" + + "Some title" + "Test Value 1" + "<_x005B_Key2_x005D_>Test Value 2" + "Test Value 3" + - ""; + ""; // Act using (var xmlWriter = XmlWriter.Create(outputStream)) diff --git a/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/ValidationProblemDetailsWrapperTest.cs b/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/ValidationProblemDetailsWrapperTest.cs index 50630a0082..53bae5afaf 100644 --- a/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/ValidationProblemDetailsWrapperTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/ValidationProblemDetailsWrapperTest.cs @@ -17,10 +17,10 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml { // Arrange var xml = "" + - "" + - "Some title" + - "400" + - "Some instance" + + "" + + "Some title" + + "400" + + "Some instance" + "Test Value 1" + "<_x005B_key2_x005D_>Test Value 2" + "" + @@ -28,7 +28,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml "<_x005B_error2_x005D_>Test error 3" + "Test error 4" + "" + - ""; + ""; var serializer = new DataContractSerializer(typeof(ValidationProblemDetailsWrapper)); // Act @@ -78,13 +78,13 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml { // Arrange var xml = "" + - "" + - "Some title" + - "400" + - "Some instance" + + "" + + "Some title" + + "400" + + "Some instance" + "Test Value 1" + "<_x005B_key2_x005D_>Test Value 2" + - ""; + ""; var serializer = new DataContractSerializer(typeof(ValidationProblemDetailsWrapper)); // Act @@ -118,11 +118,11 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml { // Arrange var xml = "" + - "" + - "Some title" + - "400" + + "" + + "Some title" + + "400" + "" + - ""; + ""; var serializer = new DataContractSerializer(typeof(ValidationProblemDetailsWrapper)); // Act @@ -160,9 +160,9 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml var wrapper = new ValidationProblemDetailsWrapper(problemDetails); var outputStream = new MemoryStream(); var expectedContent = "" + - "" + - "Some detail" + - "Some title" + + "" + + "Some detail" + + "Some title" + "Test Value 1" + "<_x005B_Key2_x005D_>Test Value 2" + "" + @@ -170,7 +170,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml "<_x005B_error2_x005D_>Test error 3" + "Test error 4" + "" + - ""; + ""; // Act using (var xmlWriter = XmlWriter.Create(outputStream)) @@ -203,12 +203,12 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml var wrapper = new ValidationProblemDetailsWrapper(problemDetails); var outputStream = new MemoryStream(); var expectedContent = "" + - "" + - "Some detail" + - "Some title" + + "" + + "Some detail" + + "Some title" + "Test Value 1" + "<_x005B_Key2_x005D_>Test Value 2" + - ""; + ""; // Act using (var xmlWriter = XmlWriter.Create(outputStream)) diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/XmlDataContractSerializerFormattersWrappingTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/XmlDataContractSerializerFormattersWrappingTest.cs index 2e34fbbe27..d85de4c3c7 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/XmlDataContractSerializerFormattersWrappingTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/XmlDataContractSerializerFormattersWrappingTest.cs @@ -216,12 +216,12 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Arrange using (new ActivityReplacer()) { - var expected = "" + - "404" + - "Not Found" + - "https://tools.ietf.org/html/rfc7231#section-6.5.4" + + var expected = "" + + "404" + + "Not Found" + + "https://tools.ietf.org/html/rfc7231#section-6.5.4" + $"{Activity.Current.Id}" + - ""; + ""; // Act var response = await Client.GetAsync("/api/XmlDataContractApi/ActionReturningClientErrorStatusCodeResult"); @@ -237,8 +237,13 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public async Task ProblemDetails_WithExtensionMembers_IsSerialized() { // Arrange - var expected = @"instance404title -correlationAccount1 Account2"; + var expected = "" + + "instance" + + "404" + + "title" + + "correlation" + + "Account1 Account2" + + ""; // Act var response = await Client.GetAsync("/api/XmlDataContractApi/ActionReturningProblemDetails"); @@ -255,14 +260,14 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Arrange using (new ActivityReplacer()) { - var expected = "" + - "400" + - "One or more validation errors occurred." + + var expected = "" + + "400" + + "One or more validation errors occurred." + $"{Activity.Current.Id}" + "" + "The State field is required." + "" + - ""; + ""; // Act var response = await Client.GetAsync("/api/XmlDataContractApi/ActionReturningValidationProblem"); @@ -278,8 +283,16 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public async Task ValidationProblemDetails_WithExtensionMembers_IsSerialized() { // Arrange - var expected = @"some detail400One or more validation errors occurred. -some typecorrelationErrorValue"; + var expected = "" + + "some detail" + + "400" + + "One or more validation errors occurred." + + "some type" + + "correlation" + + "" + + "ErrorValue" + + "" + + ""; // Act var response = await Client.GetAsync("/api/XmlDataContractApi/ActionReturningValidationDetailsWithMetadata"); diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/XmlSerializerFormattersWrappingTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/XmlSerializerFormattersWrappingTest.cs index b8b41c5f0a..33c76b98e8 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/XmlSerializerFormattersWrappingTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/XmlSerializerFormattersWrappingTest.cs @@ -191,12 +191,12 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Arrange using (new ActivityReplacer()) { - var expected = "" + - "404" + - "Not Found" + - "https://tools.ietf.org/html/rfc7231#section-6.5.4" + + var expected = "" + + "404" + + "Not Found" + + "https://tools.ietf.org/html/rfc7231#section-6.5.4" + $"{Activity.Current.Id}" + - ""; + ""; // Act var response = await Client.GetAsync("/api/XmlSerializerApi/ActionReturningClientErrorStatusCodeResult"); @@ -212,8 +212,13 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public async Task ProblemDetails_WithExtensionMembers_IsSerialized() { // Arrange - var expected = @"instance404title -correlationAccount1 Account2"; + var expected = "" + + "instance" + + "404" + + "title" + + "correlation" + + "Account1 Account2" + + ""; // Act var response = await Client.GetAsync("/api/XmlSerializerApi/ActionReturningProblemDetails"); @@ -230,14 +235,14 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Arrange using (new ActivityReplacer()) { - var expected = "" + - "400" + - "One or more validation errors occurred." + + var expected = "" + + "400" + + "One or more validation errors occurred." + $"{Activity.Current.Id}" + "" + "The State field is required." + "" + - ""; + ""; // Act var response = await Client.GetAsync("/api/XmlSerializerApi/ActionReturningValidationProblem"); @@ -253,8 +258,16 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public async Task ValidationProblemDetails_WithExtensionMembers_IsSerialized() { // Arrange - var expected = @"some detail400One or more validation errors occurred. -some typecorrelationErrorValue"; + var expected = "" + + "some detail" + + "400" + + "One or more validation errors occurred." + + "some type" + + "correlation" + + "" + + "ErrorValue" + + "" + + ""; // Act var response = await Client.GetAsync("/api/XmlSerializerApi/ActionReturningValidationDetailsWithMetadata");