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

Breaking change from 6.0.11 to 6.0.12 if you expect query parameters in @RequestBody #31327

Closed
anderius opened this issue Sep 27, 2023 · 6 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: regression A bug that is also a regression
Milestone

Comments

@anderius
Copy link

anderius commented Sep 27, 2023

I discovered a breaking change between Spring Boot 3.1.3 and 3.1.4. It only affects users that are doing something wrong (imho), but as I discovered it in a test, maybe others have done the same mistake.

I have replicated it here: https://github.com/anderius/spring-requestbody-demo

The tests passes with Spring Boot 3.1.3, but change that to 3.1.4, and the test fails.

It seems query parameters was previously put into a parameter annotated with @RequestBody, but that changed in 3.1.4. The new behavior is the correct one, I believe.

Relevant code snippets:

@PostMapping(value = "/example", consumes = "application/x-www-form-urlencoded", produces = "text/plain")
public String take(@RequestBody String request) {
    return request;
}
@Test
void queryParameterIsRequestBody(@Autowired TestRestTemplate restTemplate) {

HttpHeaders headers = new HttpHeaders();
headers.add("Content-Type", "application/x-www-form-urlencoded");

// Yes, I know this is not the way...
ResponseEntity<String> response = restTemplate.postForEntity("/example?query=123", new HttpEntity<Void>(headers), String.class);
assertThat(response.getBody()).isEqualTo("query=123");
}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 27, 2023
@anderius anderius changed the title Breaking change from 3.1.3 to 3.1.4 (if you do something wrong) Breaking change from 3.1.3 to 3.1.4 if you take query parameters in @RequestBody Sep 27, 2023
@anderius anderius changed the title Breaking change from 3.1.3 to 3.1.4 if you take query parameters in @RequestBody Breaking change from 3.1.3 to 3.1.4 if you expect query parameters in @RequestBody Sep 27, 2023
@scottfrederick
Copy link
Contributor

Thanks for the sample. Spring Boot 3.1.4 upgraded the managed version of Spring Framework to from 6.0.11 to 6.0.12. With your sample, leaving the Spring Boot version at 3.1.3 and adding ext['spring-framework.version'] = '6.0.12' to upgrade the Spring Framework version to the same one used by default with Spring Boot 3.1.4 also reproduces the problem.

@anderius
Copy link
Author

Thank you for that clarification. I suspected the change might be in Spring Framework itself.

Do you want me to move the issue?

@scottfrederick
Copy link
Contributor

We can take care of moving the issue.

@bclozel bclozel transferred this issue from spring-projects/spring-boot Sep 27, 2023
@rstoyanchev rstoyanchev removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 28, 2023
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 28, 2023
@rstoyanchev
Copy link
Contributor

This appears to be a regression as a result of #30942. We have logic for form data that reconstructs the request body from request parameters. This is to ensure consistent access to form data even if some code tries to access request parameters (e.g. from a Filter) which would cause the Servlet container to parse the body. The change in the referenced PR now relies on the Content-Length which in this scenario is 0.

Is your actual scenario sending a form data request with just a query and no body?

@anderius
Copy link
Author

anderius commented Sep 28, 2023

I just had a test that did something wrong, and I have fixed that.

My purpose with this issue was just to raise the awareness of this changed behavior, as it COULD be a breaking change for someone else. For me it would be no problem if you closed this, because the breaking change was only if developers (potentially external clients) did something really wrong in the first place. :-)

@anderius anderius changed the title Breaking change from 3.1.3 to 3.1.4 if you expect query parameters in @RequestBody Breaking change from 6.0.11 to 6.0.12 if you expect query parameters in @RequestBody Sep 28, 2023
@sbrannen sbrannen added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Sep 28, 2023
@sbrannen sbrannen changed the title Breaking change from 6.0.11 to 6.0.12 if you expect query parameters in @RequestBody Breaking change from 6.0.11 to 6.0.12 if you expect query parameters in @RequestBody Sep 28, 2023
@rstoyanchev rstoyanchev self-assigned this Sep 29, 2023
@rstoyanchev rstoyanchev added type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 29, 2023
@rstoyanchev rstoyanchev added this to the 6.0.13 milestone Sep 29, 2023
@rstoyanchev
Copy link
Contributor

We've decided to undo the optimization from #30942 for 6.0.13, while for 6.1 we'll avoid reconstructing the body from request parameters, if the query string is not null.

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: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

5 participants