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.36k stars 9.99k forks source link

Minimal APIs and controllers treat header arrays differently #54978

Open gurustron opened 6 months ago

gurustron commented 6 months ago

Is there an existing issue for this?

Describe the bug

Passing comma-separated list of values to the header is treated differently by controller action and Minimal API endpoint during binding to array. For controller action they are parsed as separate values while for Minimal APIs - as single one: Controller action: image

Minimal API:

image

Expected Behavior

Should behave the same way.

Steps To Reproduce

Create controller with action:

[HttpGet]
public IActionResult Get([FromHeader] string[] hs) => Ok(new { Test = hs});

And minimal API endpoint:

app.MapGet("/test_h", ([FromHeader] string[] hs) => new {Test = hs});

And pass request header hs: 1,2,3

Exceptions (if any)

No response

.NET Version

8.0.100

Anything else?

No response

captainsafia commented 5 months ago

@gurustron Thanks for reporting this issue!

Minimal API's binding logic processes all arguments that are annotated with [FromHeader] as string values (see here).

On the other hand, MVC's HeaderModelBinder has some custom handling for enumerable types:

https://github.com/dotnet/aspnetcore/blob/207bc7d00c00bf454dc83789d6824609b1d8b02a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/HeaderModelBinder.cs#L109-L118

I'd classify this as a bug in minimal APIs. I don't see any reason that we shouldn't handle enumerable types correctly, particularly because so many headers pass their values as a comma-seperated string.

I'm going to mark this as help wanted in the backlog. It's not high priority but I'd be happy to review or provide guidance on a PR.

tongsean9807 commented 5 months ago

Hi, I am interested in resolving this issue. But, I am new to the process.

I am able to setup the environment and replicate the problem. I have look into the code, it seems to process string first.

However, I am looking for a document or steps to debug aspnetcore repo. Do I build the code at src/Http after changes? And how can I see the logs?

For example, if I just print "hello world" within aspnetcore repo, where will it appear?

Thank you Regards, Sean

martincostello commented 5 months ago

@tongsean9807 Documentation for building the code can be found here: Build the ASP.NET Core repo

If the instructions for debugging aren't clear from that, then maybe some improvements/clarifications can be made.

My flow on my Windows laptop is typically:

  1. Sync my local repo with the latest code in main;
  2. Create a new branch;
  3. Run restore.cmd;
  4. Run . ./activate.ps1;
  5. Change the directory to where I want to change (e.g. /src/MVC);
  6. Run startvs.cmd;
  7. Work on the code in Visual Studio like I would any other .NET solution to edit, run unit tests and debug via any of the sample projects in the solution that contain the things I've changed.
danirzv commented 5 months ago

it will be a breaking change so, what's the right approach here? should we change how array inputs work in minimal-api?!

mikekistler commented 1 week ago

Here's the relevant section of the HTTP RFC

https://www.rfc-editor.org/rfc/rfc9110.html#section-5.2

which clearly designates comma as the character for separating values in a header.