aspnet / Mvc

[Archived] ASP.NET Core MVC is a model view controller framework for building dynamic web sites with clean separation of concerns, including the merged MVC, Web API, and Web Pages w/ Razor. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
5.62k stars 2.14k forks source link

400 Bad Request when query parameters are encapsulated in an object using [ApiController] #8726

Closed jameswilddev closed 5 years ago

jameswilddev commented 5 years ago

Is this a Bug or Feature request?:

Bug

Steps to reproduce (preferably a link to a GitHub repo with a repro project):

using Microsoft.AspNetCore;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.DependencyInjection;
using System;
using System.Threading.Tasks;

namespace NetCore400Repro
{
    public sealed class Request
    {
        [FromQuery]
        public string Param { get; set; }
    }

    [ApiController] // Remove this to make it work.
    public sealed class TestController : Controller
    {
        [HttpGet, Route("test")]
        public ActionResult<string> Get(Request request)
        {
            return new ActionResult<string>(request.Param);
        }
    }

    internal static class Program
    {
        internal static async Task Main(string[] args)
        {
            using
            (
                var host = WebHost
                .CreateDefaultBuilder()
                .UseUrls("http://localhost:5000")
                .ConfigureServices(s => s.AddMvc())
                .Configure(app =>
                {
                    app.UseMvc();
                })
                .Build()
            )
            {
                await host.StartAsync();
                Console.WriteLine("Running");
                Console.ReadLine();
            }
        }
    }
}

GET http://localhost:5000/test?param=a

Response:

{
    "": [
        "The input was not valid."
    ]
}

Description of the problem:

Seems it should be valid to me. Removing [ApiController] gets me the expected 200 response with a body of "a".

Version of Microsoft.AspNetCore.Mvc or Microsoft.AspNetCore.App or Microsoft.AspNetCore.All:

pranavkm commented 5 years ago

All complex type parameters in an ApiController are bound from the request body. By design, the parameter inference does not look at attributes on properties on the parameter type. Adding [FromQuery] on the parameter rather than the property should get you the behavior you're looking for here.

jameswilddev commented 5 years ago

Many of our query parameters contain conditional validation which depends upon other query parameters, such as A must be set when B has value C, or only one of A or B can be set.

This can't be done with validation attributes on method parameters AFAIK (no validation context), only when encapsulated in an object as above.

Additionally, we can't write unit tests against validation attributes on method parameters, but writing tests against an encapsulating object is trivial (IObjectModelValidator).

Not supporting encapsulated requests is something of a deal-breaker for validation.

pranavkm commented 5 years ago

@jameswilddev there's a couple of alternatives to annotating the parameter with the attribute:

1) Turn off parameter inference using ApiBehaviorOptions.SuppressInferBindingSourcesForParameters. You get the same behavior as a regular Controller and have to annotate parameters with FromBody if you're attempting to bind it from the request body. 2) Apply a [ModelBinder] attribute to the parameter. This indicates that individual properties on the parameter can come from any source and the FromQuery should work.

jameswilddev commented 5 years ago

Thanks, I'll be sure to give those a try tomorrow when I'm back at the coal face ha!

jameswilddev commented 5 years ago

That seems to work beautifully, thanks, you're a lifesaver