Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ProblemDetails serialization to Xml uses incorrect casing #7715

Closed
kingmotley opened this issue Feb 19, 2019 · 6 comments
Closed

ProblemDetails serialization to Xml uses incorrect casing #7715

kingmotley opened this issue Feb 19, 2019 · 6 comments
Assignees
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates investigate

Comments

@kingmotley
Copy link

kingmotley commented Feb 19, 2019

Describe the bug

ValidationProblemDetails/ProblemDetails serialization to Xml uses incorrect casing

To Reproduce

Steps to reproduce the behavior:
using ASP.NET Core 2.2
return a ValidationProblemDetails and you get something similar to the following:

<?xml version="1.0" encoding="utf-8"?>
<ValidationProblemDetails xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
    <Type>https://asp.net/core</Type>
    <Title>One or more validation errors occurred.</Title>
    <Status>400</Status>
    <Detail>Please refer to the errors property for additional details.</Detail>
    <Instance>/api/v1/SimpleOrders</Instance>
</ValidationProblemDetails>

Expected behavior

Conform to the example https://tools.ietf.org/html/rfc7807#page-15

<?xml version="1.0" encoding="utf-8"?>
<problem xmlns="urn:ietf:rfc:7807">
    <type>https://asp.net/core</Type>
    <title>One or more validation errors occurred.</Title>
    <status>400</Status>
    <detail>Please refer to the errors property for additional details.</Detail>
    <instance>/api/v1/SimpleOrders</Instance>
</problem>

Additional context

Xml is a case-sensitive format, so casing here is important. Additionally, the errors node is not serialized by default which leaves a poor user experience for anyone expecting Web API to work for both JSON and XML formats.

@pranavkm
Copy link
Contributor

Is your CompatiblityVersion set to 2.2? We made some changes to the xml serialization to make the serialization closer to the spec. See https://docs.microsoft.com/en-us/aspnet/core/mvc/compatibility-version?view=aspnetcore-2.2 for documentation on specifying the compat version.

@Eilon Eilon added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Feb 19, 2019
@kingmotley
Copy link
Author

kingmotley commented Feb 19, 2019

@pranavkm Yes, it is already set to 2.2.

            services.AddMvc(config =>
            {
                config.ReturnHttpNotAcceptable = true;
                config.SuppressBindingUndefinedValueToEnumType = true;

                // Remove the text/plaintext response format type
                config.OutputFormatters.RemoveType<StringOutputFormatter>();

                config.EnableEndpointRouting = true;

                // Add XML Content Negotiation
                config.RespectBrowserAcceptHeader = true;
                config.InputFormatters.Add(new XmlSerializerInputFormatter(config));
                config.OutputFormatters.Add(new XmlSerializerOutputFormatter(new XmlWriterSettings { Indent = true, NamespaceHandling = NamespaceHandling.OmitDuplicates }));

            }).SetCompatibilityVersion(CompatibilityVersion.Version_2_2);

@pranavkm
Copy link
Contributor

You have to DI in the formatters rather than add the formatters manually for it to be configured correctly. Something along the lines of

services
   .AddMvc(config => { ...})
   .AddXmlSerializerFormatters()
   .SetCompatibilityVersion(CompatibilityVersion.Version_2_2);

services.Configure<MvcOptions>(options =>
{
   var outputFormatter = options.OutputFormatters.OfType<XmlSerializerOutputFormatter>().First();
   outputFormatter.WriterSettings.Indent = true;
   outputFormatter.WriterSettings.NamespaceHandling = NamespaceHandling.OmitDuplicates;

});

@pranavkm pranavkm self-assigned this Feb 19, 2019
@kingmotley
Copy link
Author

kingmotley commented Feb 19, 2019

Ouch. OK, that does look better. Now I get responses like this for xml:

<problem xmlns="urn:ietf:rfc:7807">
    <detail>Please refer to the errors property for additional details.</detail>
    <instance>https://localhost:44379/api/v1/SimpleOrders</instance>
    <status>400</status>
    <title>One or more validation errors occurred.</title>
    <MVC-Errors>
        <Recipient.Zip>Zip '6009' is not valid for country 'UNITED STATES'.</Recipient.Zip>
        <Recipient.LastName>The fields FirstName and LastName must be strings with a combined maximum length of '39'.</Recipient.LastName>
        <Recipient.FirstName>The fields FirstName and LastName must be strings with a combined maximum length of '39'. This is the second error message for FirstName</Recipient.FirstName>
    </MVC-Errors>
</problem>

and this for json:

{
    "errors": {
        "Recipient.Zip": [
            "Zip '6009' is not valid for country 'UNITED STATES'."
        ],
        "Recipient.LastName": [
            "The fields FirstName and LastName must be strings with a combined maximum length of '39'."
        ],
        "Recipient.FirstName": [
            "The fields FirstName and LastName must be strings with a combined maximum length of '39'.",
            "This is the second error message for FirstName"
        ]
    },
    "title": "One or more validation errors occurred.",
    "status": 400,
    "detail": "Please refer to the errors property for additional details.",
    "instance": "https://localhost:44379/api/v1/SimpleOrders"
}

Now I just have to get swagger to agree that is what it is going to send back.
If I put [ProducesResponseType((int)HttpStatusCode.BadRequest)] on my action, then swagger says this is the response:

<?xml version="1.0" encoding="UTF-8"?>
<ProblemDetails>
	<type>string</type>
	<title>string</title>
	<status>0</status>
	<detail>string</detail>
	<instance>string</instance>
	<additionalProp>Unknown Type: object</additionalProp>
</ProblemDetails>

and I put [ProducesResponseType(typeof(ValidationProblemDetails), (int)HttpStatusCode.BadRequest)] then it says this is the response:

<?xml version="1.0" encoding="UTF-8"?>
<ValidationProblemDetails>
	<errors>
		<additionalProp>Unknown Type: array</additionalProp>
	</errors>
	<type>string</type>
	<title>string</title>
	<status>0</status>
	<detail>string</detail>
	<instance>string</instance>
	<additionalProp>Unknown Type: object</additionalProp>
</ValidationProblemDetails>

@kingmotley
Copy link
Author

kingmotley commented Feb 19, 2019

It does seem odd that in XML, the messages are joined together by a space so that it returns a single string like this <Recipient.FirstName>The fields FirstName and LastName must be strings with a combined maximum length of '39'. This is the second error message for FirstName</Recipient.FirstName>, but with JSON, the messages are returned as an array of string like this "Recipient.FirstName": [ "The fields FirstName and LastName must be strings with a combined maximum length of '39'.", "This is the second error message for FirstName" ].

Makes creating a unified model rather difficult. Pick one and stick with it for both formats. Either return a string with all the messages combined, or return an array of strings, but don't change it based on the serializer.

@pranavkm
Copy link
Contributor

The RFC has a very specific format for representing arrays in XML - https://tools.ietf.org/html/rfc7807#appendix-A - something that skipped due to time constraints when initially implementing this feature. Error values would need to be represented using this format since it's an array of strings:

<MVC-Errors>
    <Recipient.Zip>
        <i>Zip '6009' is not valid for country 'UNITED STATES'.</i>
    </Recipient.Zip>
    <Recipient.LastName>
        <i>The fields FirstName and LastName must be strings with a combined maximum length of '39'.</i>
    </Recipient.LastName>
    <Recipient.FirstName>
        <i>The fields FirstName and LastName must be strings with a combined maximum length of '39'.</i>
        <i>This is the second error message for FirstName</i>
    </Recipient.FirstName>
</MVC-Errors>

We'd be happy to accept a PR if this is something you would be interested in improving. Here are the relevant types:

I'm closing this issue since the initial question was resolved.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates investigate
Projects
None yet
Development

No branches or pull requests

3 participants