Skip to content

Minimal APIs and controllers treat header arrays differently #54978

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

Closed
1 task done
gurustron opened this issue Apr 5, 2024 · 5 comments · Fixed by #58251
Closed
1 task done

Minimal APIs and controllers treat header arrays differently #54978

gurustron opened this issue Apr 5, 2024 · 5 comments · Fixed by #58251
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc help wanted Up for grabs. We would accept a PR to help resolve this issue
Milestone

Comments

@gurustron
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

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

@dotnet-issue-labeler dotnet-issue-labeler bot added the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Apr 5, 2024
@captainsafia
Copy link
Member

@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:

if (bindingContext.ModelMetadata.IsEnumerableType)
{
values = request.Headers.GetCommaSeparatedValues(headerName);
}
else
{
values = new[] { header.ToString() };
}
}

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.

@captainsafia captainsafia added help wanted Up for grabs. We would accept a PR to help resolve this issue area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Apr 30, 2024
@captainsafia captainsafia added this to the Backlog milestone Apr 30, 2024
@tongsean9807
Copy link

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
Copy link
Member

@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
Copy link
Contributor

danirzv commented May 19, 2024

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

@mikekistler
Copy link
Contributor

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.

smnsht added a commit to smnsht/aspnetcore that referenced this issue Nov 2, 2024
captainsafia pushed a commit that referenced this issue Mar 4, 2025
…#58251)

* #54978

* update EndpointParameterEmitter

* update snapshots

* go back 4 commits in googletest

* imporove BindParameterFromProperty(...) readability

* get coverage for both compile-time and run-time code gen with a single test case.

* merging main

* Revert "go back 4 commits in googletest"

This reverts commit ba2d730.

* Revert "merging main"

This reverts commit b618f3c.

* empty

* add caching for  GetHeaderSplit MethodInfo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc help wanted Up for grabs. We would accept a PR to help resolve this issue
Projects
None yet
6 participants