Improve documentation on reading form data via Servlet request parameters vs @RequestBody

[membersound]

I upgraded from spring-boot v2 -> v3. My tests could not detect the following issue, even though I tested the scenario.
The following problem does only occur in live, but not in tests:

In the following example, the @RequestBody parameters are missing in case when using additional query parameters.

@RestController
public class ExampleServlet {
    @PostMapping(value = "/example", consumes = APPLICATION_FORM_URLENCODED_VALUE)
    public Mono<String> namedsqlPostForm(
            @RequestParam(name = "metadata", required = false) String metadata,
            @RequestBody(required = false) MultiValueMap<String, String> params) {
        if (params == null || params.isEmpty())
            throw new RuntimeException("params missing");

        return Mono.just("OK");
    }
}

The following works (body params are filled in controller):

curl --location --request POST 'localhost:8080/example' \
--header 'Content-Type: application/x-www-form-urlencoded' \
--data-urlencode 'test=junit'

The following fails (body params are missing in controller):

curl --location --request POST 'localhost:8080/example?format=json' \
--header 'Content-Type: application/x-www-form-urlencoded' \
--data-urlencode 'test=junit'

Result LIVE:

{
    "timestamp": "2024-08-20T09:37:04.078+00:00",
    "status": 500,
    "error": "Internal Server Error",
    "path": "/example"
}

I attached a sample, which fails in live mode, but succeeds in junit test mode.
Switching the example back to spring-boot 2.x, also the live mode works.

Test:

@SpringBootTest
@AutoConfigureMockMvc
public class ExampleServletTest {
    @Autowired
    private WebTestClient webTestClient;

    //this is NOT true in real world! this only succeeds in junit test!
    @Test
    public void testBodyParamsWithQueryParam() throws JsonProcessingException {
        webTestClient.post()
                .uri("/example?metaadat=v1")
                .contentType(MediaType.APPLICATION_FORM_URLENCODED)
                .bodyValue(new ObjectMapper().writeValueAsString(Map.of("Test", "junit")))
                .exchange()
                .expectStatus().isEqualTo(200)
                .expectBody(String.class).isEqualTo("OK");
    }
}

spring-web-example.zip

[sdeleuze]

The change of behavior seems to have been introduced in Spring Framework 6.1:

• With Spring Boot 3.1, params contains both the ones defines in query parameters and request body
• With Spring Boot 3.2, it is empty.

[sdeleuze]

In AbstractMessageConverterMethodArgumentResolver#readWithMessageConverters:

Spring Boot 3.1.12
((ServletServerHttpRequest)inputMessage).getBody() is a ByteArrayInputStream
((ServletServerHttpRequest)inputMessage).getBody().readAllBytes() returns 22 bytes as expected.

Spring Boot 3.2.8
((ServletServerHttpRequest)inputMessage).getBody() returns a CoyoteInputStream
((ServletServerHttpRequest)inputMessage).getBody().readAllBytes() returns 0 bytes

Both uses Tomcat 10.1.x.

[sdeleuze]

Likely a regression caused by gh-31327 (via 8fa428f) as you have already found @membersound (could have been nice to mention it in the issue description, please share all your findings next time please).

[sdeleuze]

@rstoyanchev As I lack a bit of context on those changes, could you please provide guidance on how to handle this when you are back?

[rstoyanchev]

In the Servlet API, request parameter access causes the request body to be parsed, and it can't be read again, e.g. via @RequestBody. In ServletServerHttpRequest#getBody we try to make up for that by re-constructing the body from the request parameter map, but this is fragile.

In 6.0.x we accepted an optimization in StringHttpMessageConverter to read the body via readNBytes if there is a content-length, but the content-length can get out of sync if the reconstructed body ends up picking request parameters from the query. This is why the decision in #31327 was to back out of the feature if there is both a request body and a query string as we can't properly implement it in that case. We could try to exclude request parameters from the query when reconstructing the body, but that will likely cause regressions elsewhere, and could also prove fragile, e.g. if the same parameter name is present in both the body and the query.

I think the best thing is to not rely on the feature, which can only be properly implemented at the level of the Servlet container. In other words don't expect to be able to read form data via @RequestBody reliably. Instead, for form data specifically, stick to using request parameters. For example for the above:

    @PostMapping(value = "/example", consumes = APPLICATION_FORM_URLENCODED_VALUE)
    public Mono<String> namedsqlPostForm(@RequestParam MultiValueMap<String, String> params) {
            // ...
    }

[sdeleuze]

Based on @rstoyanchev feedback, I am turning this regression issue to a documentation one.

@membersound Please follow Rossen's guidance for your use case.