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

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

Open
unlimitedsola opened this issue Aug 23, 2018 · 6 comments

Comments

@unlimitedsola
Copy link

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

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

@wilkinsona wilkinsona changed the title Request bodies are not documented when using path parameters Request body is empty when using MockMvc, application/x-www-form-urlencoded and param Sep 3, 2018
@wilkinsona
Copy link
Member

wilkinsona commented Sep 3, 2018

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
Copy link
Member

@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?

@unlimitedsola
Copy link
Author

unlimitedsola commented Sep 3, 2018

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.

@wilkinsona
Copy link
Member

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
Copy link
Member

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

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Oct 3, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants