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 web parameters resolution when injected via constructor #25200

Conversation

encircled
Copy link
Contributor

This fixes the problem described in #25182

Comma-separated multi value parameters are wrongly injected if constructor injection is used. Only spring-web is affected, webflux works correctly.

For example ...?listOfStrings=1,2 becomes List<String> listOfStrings = ["1,2"], instead of expected List<String> listOfStrings = ["1", "2"]. The reason is that WebRequest#getParameterValues exposes single item params as an array which breaks conversion service later on.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 6, 2020
@sbrannen sbrannen added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jun 9, 2020
@encircled
Copy link
Contributor Author

@sbrannen just a kind reminder. Lets review&merge or close it. This bug should be addressed somehow though.

@sbrannen sbrannen added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels May 5, 2021
@sbrannen sbrannen added this to the 5.3.7 milestone May 5, 2021
@sbrannen
Copy link
Member

sbrannen commented May 5, 2021

Thanks for the reminder.

Since this appears to be a bug, we'll see about getting it merged into main for inclusion in 5.3.7 and potentially for a backport to 5.2.x.

@sbrannen sbrannen self-assigned this May 5, 2021
@encircled
Copy link
Contributor Author

Alright, I will rebase it on the latest master then

@encircled encircled force-pushed the 25182-fix-web-params-resolution-via-constructor branch from 4ec48de to cadfe32 Compare May 5, 2021 20:48
@sbrannen
Copy link
Member

sbrannen commented May 8, 2021

Alright, I will rebase it on the latest master then

Thanks for rebasing it on main. 😉

@sbrannen sbrannen closed this in 9ddab9e May 8, 2021
sbrannen added a commit that referenced this pull request May 8, 2021
@sbrannen
Copy link
Member

sbrannen commented May 8, 2021

This has been merged into main in 9ddab9e and revised in 355d394.

Thanks for providing the fix!

@sbrannen sbrannen added the for: backport-to-5.2.x Marks an issue as a candidate for backport to 5.2.x label May 10, 2021
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.2.x Marks an issue as a candidate for backport to 5.2.x labels May 10, 2021
sbrannen pushed a commit to sbrannen/spring-framework that referenced this pull request May 10, 2021
…tors

Prior to this commit, when Web MVC attempted to resolve a constructor
argument for a data class constructor with @ModelAttribute binding,
ModelAttributeMethodProcessor failed to unwrap the array returned by
WebRequest.getParameter(String).

According to the Javadoc for WebRequest.getParameter(String), "a
single-value parameter will be exposed as an array with a single
element."

This commit fixes this issue by extracting the single value from such
an array and using that as the constructor argument (potentially
converted by the WebDataBinder).

Closes spring-projectsgh-25200
sbrannen added a commit that referenced this pull request May 10, 2021
Zoran0104 pushed a commit to Zoran0104/spring-framework that referenced this pull request Aug 20, 2021
…tors

Prior to this commit, when Web MVC attempted to resolve a constructor
argument for a data class constructor with @ModelAttribute binding,
ModelAttributeMethodProcessor failed to unwrap the array returned by
WebRequest.getParameter(String).

According to the Javadoc for WebRequest.getParameter(String), "a
single-value parameter will be exposed as an array with a single
element."

This commit fixes this issue by extracting the single value from such
an array and using that as the constructor argument (potentially
converted by the WebDataBinder).

Closes spring-projectsgh-25200
Zoran0104 pushed a commit to Zoran0104/spring-framework that referenced this pull request Aug 20, 2021
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
…tors

Prior to this commit, when Web MVC attempted to resolve a constructor
argument for a data class constructor with @ModelAttribute binding,
ModelAttributeMethodProcessor failed to unwrap the array returned by
WebRequest.getParameter(String).

According to the Javadoc for WebRequest.getParameter(String), "a
single-value parameter will be exposed as an array with a single
element."

This commit fixes this issue by extracting the single value from such
an array and using that as the constructor argument (potentially
converted by the WebDataBinder).

Closes spring-projectsgh-25200
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
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) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants