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

@RequestParam binding does not support params with an empty array "[]" suffix #32577

Closed
panovst opened this issue Apr 4, 2024 · 5 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@panovst
Copy link

panovst commented Apr 4, 2024

Affects: 2.5.2

Many http clients pass an array to queryString in this form:
?attributes[]=field1&attributes[]=field2

I'm trying to get Controller to work with this format through RequestParam:

@RestController
public class FooController {
   
   @RequestMapping(method = RequestMethod.GET,
    value = "/foo",
    produces = "application/json")
   ResponseEntity<FooDto> getFoo(@RequestParam(value = "attributes", required = false) List<String> attributes) {
   }
}

However, when executing a request, null is passed to the controller method as the attributes value.

I found that the "RequestParamMethodArgumentResolver.resolveName" method receives "attributes" as the value of the parameter name, while this parameter is stored in the request object using the key "attributes[]". Therefore, the argument of the controller method is null, because resolver cannot find a match.

How would you like to add such support to RequestParamMethodArgumentResolver? The code could look something like this:

@Override
    protected Object resolveName(String name, MethodParameter parameter, NativeWebRequest request) throws Exception {
      if (
          (
              parameterTypeIs(List.class, parameter) || parameterTypeIs(Set.class, parameter)
          )
          && nameIsIndexParameter(name, request)) {
        return super.resolveName(name + "[]", parameter, request);

      }
      return super.resolveName(name, parameter, request);
    }

    private boolean parameterTypeIs(Class<?> parameterType, MethodParameter parameter) {
      return parameterType.isAssignableFrom(parameter.getParameterType());
    }

    private boolean nameIsIndexParameter(String name, NativeWebRequest request) {
      return request.getParameterMap().containsKey(name + "[]");
    }

In my project, I replaced the built-in RequestParamMethodArgumentResolver with a custom one. An example can be found in the description of the question on stackoverflow

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 4, 2024
@snicoll snicoll changed the title enhancements: support for a parameter with an index in the query String when bound to @RequestParam Support for a parameter with an index in the query String when bound to @RequestParam Apr 4, 2024
@snicoll
Copy link
Member

snicoll commented Apr 4, 2024

Many http clients pass an array to queryString in this form

Do they? I can see this being used in PHP but that's an implementation detail that's not backed by anything I am aware of in a spec. If you need to pass multiple values, you should use the attribute as requested and let the server rebuild the list for you.

@snicoll snicoll closed this as not planned Won't fix, can't repro, duplicate, stale Apr 4, 2024
@snicoll snicoll added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Apr 4, 2024
@panovst
Copy link
Author

panovst commented Apr 5, 2024

then why do you support this format here?

Special DataBinder for data binding from web request parameters to JavaBean objects

org.springframework.web.bind.WebDataBinder#adaptEmptyArrayIndices

Check for property values with names that end on "[]". This is used by some clients for array syntax without an explicit index value. If such values are found, drop the brackets to adapt to the expected way of expressing the same for data binding purposes.

@snicoll
Copy link
Member

snicoll commented Apr 5, 2024

I wasn't aware we did. That's a question for @rstoyanchev then

See #25836 (comment)

@panovst
Copy link
Author

panovst commented Apr 5, 2024

Our situation is further complicated by the following: we use the open api to describe the contract, and the openapi-generator to generate the client and controller interfaces.

this means that even if we consider this option (from the link to the issue that you provided)

fun getAllEntities(@RequestParam(name="types[]") types: List<String>) {}

there are two disadvantages to this solution:

  1. we have to use the attributes[] format in the openapi itself
  2. only clients sending "attributes[]=" will be able to work with this method

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Apr 9, 2024

Indeed I think this is the the equivalent of #25836 but for @RequestParam.

@rstoyanchev rstoyanchev reopened this Apr 9, 2024
@rstoyanchev rstoyanchev changed the title Support for a parameter with an index in the query String when bound to @RequestParam @RequestParam binding does not support params with an empty array "[]" suffix Apr 9, 2024
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: declined A suggestion or change that we don't feel we should currently apply labels Apr 9, 2024
@rstoyanchev rstoyanchev added this to the 6.1.6 milestone Apr 9, 2024
@rstoyanchev rstoyanchev self-assigned this Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants