spring-restdocs icon indicating copy to clipboard operation
spring-restdocs copied to clipboard

Request body is empty when using MockMvc, application/x-www-form-urlencoded and param

Open unlimitedsola opened this issue 6 years ago • 6 comments

Reproducible repository at: https://github.com/unlimitedsola/restdocs-empty-body-issue

The generated asciidoc doesn't contain any request body/parameter

unlimitedsola avatar Aug 23 '18 08:08 unlimitedsola

Thanks for the sample. On closer inspection, I'm not sure that this is a bug in REST Docs. When MockMvc is used to build an application/x-www-form-url-encoded request, the parameters that are form encoded aren't reflected in the request body. This means that the request's body is empty, its content length is incorrect, etc. As a result, the following test will fail:

@Test
public void formUrlEncodedRequestBuiltWithParam() throws Exception {
	MockHttpServletRequest mockRequest = MockMvcRequestBuilders.patch("/foo")
			.contentType(MediaType.APPLICATION_FORM_URLENCODED).param("a", "alpha")
			.param("b", "bravo").buildRequest(new MockServletContext());
	assertThat(mockRequest.getContentLength()).isGreaterThan(0);
	assertThat(mockRequest.getContentAsString()).isNotEmpty();
}

Interestingly, MockHttpServletRequestBuilder contains logic for the reverse where it will parse the application/x-www-form-urlencoded request's body and set the parameters. In this case things works as you'd expect. This, hopefully, provides a workaround for the time being at least until we see where, if anywhere, a change should be made for this.

wilkinsona avatar Sep 03 '18 14:09 wilkinsona

@rstoyanchev do you think MockHttpServletRequestBuilder should set the request's content for an application/x-www-form-urlencoded request with request parameters but no content?

wilkinsona avatar Sep 03 '18 14:09 wilkinsona

FWIW, PATCH also won't have httpie and curl output, but PUT will.

I thought it is impacted by these.

CliOperationRequest.java#L56-L59

CurlRequestSnippet.java#L190-L192

HttpieRequestSnippet.java#L206-L209

Also I've pushed some test case to prove.

unlimitedsola avatar Sep 03 '18 22:09 unlimitedsola

Indeed; I'm aware of that code. I'd like to explore it becoming unnecessary though. At the moment, my opinion is that this should be handled in MockMvc (and REST Assured and WebTestClient if necessary) or, failing that, in REST Docs request converters.

wilkinsona avatar Sep 04 '18 06:09 wilkinsona

@rstoyanchev If you have a minute, I'd appreciate your input on my question above.

wilkinsona avatar Oct 03 '18 17:10 wilkinsona

In a Servlet container, query and form data are the source of request parameters. So if source data is provided, we'll parse that into request params. Or you can add request params directly, if you don't care about the source, but we won't go the other way around from the parsed representation to the source.

There is ambiguity in trying to do that since technically a request could have both a query and form data. That said given that query parameters are specified through the URI in the MockHttpServletRequestBuilder constructor, we could back-fill any params provided thereafter into the request body. If we do that however, a case could be made that for any other other request (not form data), we should also back-fill parameters into the query, which brings back ambiguity and inconsistency.

So I'm not in favor of trying to interpret request parameters that way, but we could consider additional methods to explicitly add form data, something like formField(String, String...).

For the WebTestClient this is not an issue since it's an actual HTTP client and has no knowledge of "request parameters" which are a Servlet API construct.

rstoyanchev avatar Oct 03 '18 19:10 rstoyanchev