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

String array queries do not use commas to separate each element #46530

Closed JansthcirlU closed 1 year ago

JansthcirlU commented 1 year ago

Is there an existing issue for this?

Describe the bug

Arrays or enumerables of string are incorrectly serialized when using them as a query parameter inside a controller method. For example, the input array string letters = { "d", "g", "o" }; gets serialized to the query string "?letters=d&letters=g&letters=o".

The reason why I'm making an issue here is because someone made a very similar issue on the Swashbuckle repository and they believe it is an upstream issue in the .NET implementation.

Expected Behavior

The query string should be simplified to "?letters=d,g,o" in situations where the OpenAPI style specifications allow it (such as this one).

Steps To Reproduce

  1. Make a new ASP.NET Core 7 Web API named MyApi (dotnet new webapi -f net7.0 -o MyApi)
  2. Add the following CombinationsController
[ApiController]
[Route("api/v1/[controller]")]
public class CombinationsController : Controllerbase
{
    [HttpGet]
    public async Task<IActionResult> Get([FromQuery] string[] letters)
    {
        return NoContent();
    }
}
  1. Make sure Swagger is enabled
  2. Run (or debug) the API and go to the Swagger index page
  3. Try out the api/v1/combinations route on the Combinations controller
  4. Add d, g and o as the input characters and execute

The resulting request URL contains the query ?letters=d&letters=g&letters=o instead of ?letters=d,g,o.

Exceptions (if any)

No response

.NET Version

7.0.101

Anything else?

I'm using ASP.NET Core 7 with Visual Studio Code 1.75.0 as my editor

$ dotnet --info
.NET SDK:
 Version:   7.0.101
 Commit:    bb24aafa11

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22621
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\7.0.101\

Host:
  Version:      7.0.1
  Architecture: x64
  Commit:       97203d38ba

.NET SDKs installed:
  6.0.404 [C:\Program Files\dotnet\sdk]
  7.0.101 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 6.0.12 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  x86   [C:\Program Files (x86)\dotnet]
    registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
  Not set

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download
javiercn commented 1 year ago

@JansthcirlU thanks for contacting us.

I believe this is by design. Arrays are serialized in that fashion for historical reasons and backwards compatibility with older versions of MVC as well as the model binding system.

JansthcirlU commented 1 year ago

@javiercn Thank you for your reply. How could I support the case for users who want to call my API by manually typing out the URL using commas, or for other APIs that want to call the method by visiting a URL with comma-separated values?

There was a workaround in the Swashbuckle issue, but this doesn't seem to be the right solution for de-serializing query strings.

javiercn commented 1 year ago

@JansthcirlU if I understood correctly, the issue you are having is that ?letters=a,b,c is not binding correctly to an array with three elements, is that the case?

JansthcirlU commented 1 year ago

@javiercn correct, using ?letters=a,b,c creates an array with a single element "a,b,c" rather than one element for each letter. I understand that I could manually split the text inside the controller method, but it would be nice if it just worked.

javiercn commented 1 year ago

@JansthcirlU You can author a custom model binder for the parameter and apply it with an attribute.

JansthcirlU commented 1 year ago

@javiercn okay, I'll investigate that approach!

captainsafia commented 1 year ago

@JansthcirlU Where in your OpenAPI definition do you define the format for the letters query value?

ghost commented 1 year ago

Hi @JansthcirlU. 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 1 year ago

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.