dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.48k stars 10.04k forks source link

[HttpGet("{id}")]public IActionResult GetById(byte[] id) should be BindingSource.Path #39384

Open myunQ opened 2 years ago

myunQ commented 2 years ago

Take the liberty to make two suggestions

  1. Is it possible to hand over the model binding work of [HttpGet("{id}")]public IActionResult GetById(byte[] id) to ByteArrayModelBinder for binding.

I am trying to do

        [HttpGet("{id}")]
        public ActionResult<object> GetById(byte[] id)
        {
            ......
        }

but The value of ControllerContext.ActionDescriptor.Parameters[0].BindingInfo.BindingSource is BindingSource.Body,Resulting in parameter binding using BodyModelBinder, and the binding fails. response http status 415.

I know [FromRoute]byte[] id will do the trick, but I'd like it to be like any other simple type. Because in [HttpGet("{id}")] it is already specifically defined to get the value from the path and bind it to the id parameter.

If adopted, make all [HttpXXX("{id}")] available as such.

  1. In addition to supporting base64 strings in ByteArrayModelBinder, it should also support URL-safe base64 and hex strings.

Added: The framework is asp.net core 6.0

pranavkm commented 2 years ago

Changing at this point would be a breaking change (parameters that would have previously bound from the body would now bind from the route) so I'd be hesitant to consider this.

You could consider authoring a convention similar to https://github.com/dotnet/aspnetcore/blob/main/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs which configures it the way you'd like. https://docs.microsoft.com/aspnet/core/mvc/controllers/application-model?view=aspnetcore-6.0#conventions

ghost commented 2 years ago

Hi @myunQ. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

ghost commented 2 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

myunQ commented 2 years ago

Hi @pranavkm. I verified that the method you said can solve the problem that the id parameter of [HttpGet("{id}")]public IActionResult GetById(byte[] id) is parsed as BindingSource.Body. Thank you so much!

But I compromised and I chose to use [FromRoute] because I don't want to cause code like [HttpPost]public IActionResults XXXX([FromHeader]byte[] bytes) because I implement IActionModelConvention to replace BindingSource Some inexplicable problems have arisen.

Now I just need to implement a model binder that supports converting Base64, URL-safe Base64 and hex to byte[] type.

builder.Services.AddControllers(options =>
{
    //options.Conventions.Add(new ByteArrayParameterBindingInfoConvention());

    // Replaces the default byte[] parameter binding provider.
    for(int i = 0; i < options.ModelBinderProviders.Count; i++)
    {
        if (options.ModelBinderProviders[i] is Microsoft.AspNetCore.Mvc.ModelBinding.Binders.ByteArrayModelBinderProvider)
        {
            options.ModelBinderProviders[i] = new My.ByteArrayModelBinderProvider();
            break;
        }
    }
});

Is there something wrong with my code?