Aspnetcore: XML Model State Validation

[saibaskaran57]

Feature Request

Description of the problem:

On JSON content-negotiation formatting, all exception are wrapped into a ModelState validation which provides flexibility for each controllers to decide the handling on the bad input models.

Steps to reproduce

1) Configure XML Formatters for content-negotiation on .NET Core Web API

builder.AddMvcOptions(options =>
{
       options.InputFormatters.Add(new XmlSerializerInputFormatter(options));
       options.OutputFormatters.Add(new XmlSerializerOutputFormatter());
});

2) Submit an invalid XML input(e.g. invalid XML root)

C# Model

[DataContract(Namespace = "")]
[XmlRoot("Request")]
public class Request
{
        [Required]
        [DataMember]
        public string Attribute{ get; set; }
}

Client Input(take note on XML root)

<Request1>
 <Attribute>attribute1</Attribute>  
<Request1>

3) XML formatter will throw InvalidOperationException which does not propagate down to filters/controllers and Exception Middlewares will catch the exceptions with a message - _There is an error in XML document_

Question:

Are there any plans in roadmap to have the similar handling for ModelState wrapping on XML Serialization?

[rynowak]

This seems like a bug if this gets through and becomes a 500. It's definitely intended that you get a model state error for this.

[mkArtakMSFT]

@dougbu, can you please look into this? Thanks!

[dougbu]

@mkArtakMSFT looks like this issue is at least PRI: 1. The problem is a regression of the #1184 fix (5bb69e495e) that shipped in 1.0.0. The regression was introduced here https://github.com/aspnet/Mvc/commit/83c3ac62fb9187c75689a26e9749046825cdd04b#diff-366db0fc1964d1d3b1154cfee3b4b4deR180

Basically, we need to restore the unconditional catch block in BodyModelBinder.

[saibaskaran57]

Hi @rynowak & @dougbu,

Thanks for your speedy response and looking into this. Appreciate it.

Will look forward for the updates.

[dougbu]

@saibaskaran57 turns out some the changes here were intentional. But we may be able to improve your experience in the short term.

When using a XmlSerializerInputFormatter or XmlDataContractSerializerInputFormatter, the BodyModelBinder should turn all exceptions into ModelState errors. This has been true since the initial ASP.NET Core release. If that's not happening in your case, could you please provide us with a minimal repro project that illustrates the problem, preferably hosted in a GitHub repo?

If you're using ASP.NET Core version 2.1 or later and a subclass of one of those formatters (or some alternative), there's now an "escape hatch" allowing exceptions such as InvalidOperationException and TypeLoadException to bubble up to an exception handler. Turn this off by setting MvcOptions.InputFormatterExceptionPolicy to InputFormatterExceptionPolicy.AllExceptions. (The default is now MalformedInputExceptions.)

Please let us know which of the above cases applies.

[dougbu]

@danroth27 @mkArtakMSFT, we should revisit the priority after hearing back from @saibaskaran57 This might be more a side effect of our #4920 intentional breaking change and less a regression.

[dougbu]

@saibaskaran57 my earlier comment was incorrect. XmlSerializerInputFormatter and XmlDataContractSerializerInputFormatter use InputFormatterExceptionPolicy.MalformedInputExceptions unless you're using a subclass. And MvcOptions.InputFormatterExceptionPolicy does not apply; it's ignored for input formatters that implement IInputFormatterExceptionPolicy.

The actual workaround is to create a subclass of XmlSerializerInputFormatter and XmlDataContractSerializerInputFormatter and to replace the original in MvcOptions. For example,
``` c#
public void ConfigureServices(IServiceCollection services)
{
services
.AddMvc()
.AddXmlDataContractSerializerFormatters();
services.AddTransient, MyMvcOptionsSetup>();
}

private class MyMvcOptionsSetup : IConfigureOptions
{
public void Configure(MvcOptions options)
{
options.InputFormatters.RemoveType();
options.InputFormatters.Add(new MyXmlDCSInputFormatter(options));
}
}

private class MyXmlDCSInputFormatter : XmlDataContractSerializerInputFormatter
{
public MyXmlDCSInputFormatter(MvcOptions options)
: base(options)
{
}
}
```

[dougbu]

@saibaskaran57 what ASP.NET Core version were you testing? I tried your "invalid XML root" case with XmlSerializerInputFormatter and XmlDataContractSerializerInputFormatter and the de-serializer's Exception was turned into a message ("An error occurred while deserializing input data.") in ModelState when using our current 2.2 code. That doesn't match the behaviour or the message you described.

[dougbu]

Oops, there was an error in the test I was using. The underlying de-serializer wraps an InvalidOperationException inside another InvalidOperationException and the XmlSerializerInputFormatter isn't expecting that. I am looking to see if there's anything about the inner Exception that allows the formatter to catch XML issues but not, say, type-load issues.

[saibaskaran57]

Hi @dougbu,

Thanks for looking at this and appreciate for your response.

I'm using ASP.NET Core 2.1.2. At the moment, I'm extending from XmlSerializerInputFormatter, catching InvalidOperationException specifically and adding the exception into TryAddModelException. It worked well with the approach.

E.g.

try
{
     return base.ReadRequestBodyAsync(context);
}
catch (InvalidOperationException ex)
{
     context.ModelState.TryAddModelException(<Validation Key>, ex);
     return InputFormatterResult.FailureAsync();
}

However, it would be great if the formatter itself able to catch the exception and add it into the ModelState which similar to JsonInputFormatter. On the message with "An error occurred while deserializing input data", that can be reproduced well on my side with not closing the XML tag properly and it will add into the ModelState. The example was more towards not specifying the right XmlRoot.

Sorry for not catching up earlier but feel free to let me know if i could help further. If you need a reproducible working copy, you can refer here.

[dougbu]

Your repro project uses XmlDataContractSerializerInputFormatter (as well as additional subclasses of the built-in TextInputFormatter). In all cases, XmlDataContractSerializerInputFormatter throws a SerializationException and the formatter adds the expected error to ModelState ("An error occurred while deserializing input data.") I've tested multiple Microsoft.AspNetcore.All versions (which includes Microsoft.AspNetcore.Mvc.* packages), with and without services.AddMvc(...).SetCompatibilityVersion(CompatibilityVersion.Latest) for versions 2.1.x and later.

XmlDataContractSerializerInputFormatter consistently does the Right Thing:tm: when the top-most element is incorrect.

---

If I switch the application to use XmlSerializerInputFormatter, behaviours change and the problem reproduces with all 2.1 and later versions of Microsoft.AspNetcore.All. Again, that's with and without services.AddMvc(...).SetCompatibilityVersion(CompatibilityVersion.Latest).

I'm extending from XmlSerializerInputFormatter, catching InvalidOperationException specifically

The second part should not be necessary. My earlier workaround works fine after changing MyXmlDCSInputFormatter to derive from XmlSerializerInputFormatter et cetera.

---

All up, the issue here is specific to XmlSerializerInputFormatter and the workaround I described works fine.

I'm continuing internal conversations about how to distinguish InvalidOperationExceptions about formatting errors from lower-level problems.

[dougbu]

be7dfa30af