Skip to content
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

fix: check array values #2738

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

kolaente
Copy link
Contributor

TL;DR: This fixes an issue where query bindings would not work for slices due to the way browsers would pass them.

Consider a struct like this:

type Foo struct {
    Values []string `query:"values"`
}

Binding that struct using Echo's ctx.Bind function would work, as long as the request only contains query parameters which exactly match the query tag, like this:

?values=foo&values=bar

This is expected and works as it should. The HTTP spec even says that it's completely fine to pass the same query value multiple times.
However, major browsers (I've tested Firefox and Chromium), when asked to construct a query from an array, will create a query string like this:

?values[]=foo&values[]=bar

Notice the additional [] in the string. The spec is somewhat murky on this and it seems there are differences between languages. Echo only supports the first version and will treat [] suffixes on query parameters as literal values. Creating a struct like values[] will pass that in, but then I need more custom logic to parse that if I want to support both. This PR fixes that.

The fix I'm proposing here is the simplest I could come up with. It might not be the best or optimal.

@aldas
Copy link
Contributor

aldas commented Jan 24, 2025

please add tests

@kolaente
Copy link
Contributor Author

@aldas added tests, please have a look again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants