micronaut-projects / micronaut-core

Micronaut Application Framework
http://micronaut.io
Apache License 2.0
6.03k stars 1.05k forks source link

Optional list query parameter defaults to list with a single element #10804

Open mancze opened 4 months ago

mancze commented 4 months ago

Expected Behavior

Query parameter defined as @QueryValue(value = "v", defaultValue = "") List<String> values should default to an empty list when parameter is not present. This was working in Micronaut 3, but doesn't in Micronaut 4.

Alternatively, there should be a documented way how to declare such parameter if this syntax turns out to be insufficient (e.g. to be able to distinguish between ?v= (parameter specified, but without value) vs ? (no parameter specified).

Actual Behaviour

List with a single element of an empty string is passed as an argument.

Steps To Reproduce

  1. Run example app
  2. Execute curl http://localhost:8080/list-nullable
  3. Expected output (empty list is passed to the method):
    values == null: false
    values: []
    values.size(): 0

Environment Information

No response

Example Application

https://github.com/mancze/micronaut-sandbox/tree/repro/optional-list-query-parameter

Version

4.3.8

yawkat commented 1 month ago

When did this work exactly? I tried your test case with 3.10.4 and it behaves the same way

yawkat commented 1 month ago

And fwiw, you can distinguish the missing parameter from ?v= by removing the defaultValue and adding @Nullable

mancze commented 1 month ago

When did this work exactly? I tried your test case with 3.10.4 and it behaves the same way

We were using 3.8.12.


Sorry, I've incorrectly identified the source problem. I made an error while isolating the issue from our project. We apply validation annotations to the parameter. In the isolated project it'd be something like:

@QueryValue(value = "v", defaultValue = "") @Size(max=100) List<@NotBlank String> values

After upgrading to 4.3.8, I noticed the validation error that we didn't get before. I mistakenly assumed it was due to the difference in argument parsing and did not check. In fact, it was a difference in validation - 3.8.12 passes a list with an empty string despite the fact that it breaks validation constraints.

So after checking I think you are right - the described difference in argument parsing is not present.


But there is another strange thing I discovered which I'm not sure is expected in Micronaut 4. In the example application:

> curl "http://localhost:8080/list-nullable"
values == null: false
values: []
values.size(): 1

> curl "http://localhost:8080/list-nullable?v="
values == null: false
values: []
values.size(): 1

> curl "http://localhost:8080/list-nullable?v=%2C"
values == null: false
values: []
values.size(): 0

> curl "http://localhost:8080/list-nullable?v=,"
values == null: false
values: []
values.size(): 0

> curl "http://localhost:8080/list-nullable?v=%2C%2Cfoo%2C%2Cbar%2C%2C"
values == null: false
values: [, , foo, , bar]
values.size(): 5

It seems strange that ?v= is parsed to a list with empty string and ?v=, is parsed to an empty list. It seems that trailing commas are trimmed together with empty elements preceding them. I'm not sure if it's a bug or not, though.