AbstractMockHttpServletRequestBuilder#buildRequest is not idempotent

[reda-alaoui]

I have testing components making use of MockHttpServletRequestBuilder. These components should be able to mutate the builder but also to check the builder state (e.g. "what is the http method currently stored by the builder?").

By design, the only way to access MockHttpServletRequestBuilder state is to invoke buildRequest to retrieve a MockHttpServletRequest that will allow to check the state via its getters.
I am fine with that.

But I discovered that invoking buildRequest mutates the value of MockHttpServletRequestBuilder#pathInfo:

spring-framework/spring-test/src/main/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilder.java

Lines 901 to 909 in 88812ed

	if ("".equals(this.pathInfo)) {
	if (!requestUri.startsWith(this.contextPath + this.servletPath)) {
	throw new IllegalArgumentException(
	"Invalid servlet path [" + this.servletPath + "] for request URI [" + requestUri + "]");
	}
	String extraPath = requestUri.substring(this.contextPath.length() + this.servletPath.length());
	this.pathInfo = (StringUtils.hasText(extraPath) ?
	UrlPathHelper.defaultInstance.decodeRequestString(request, extraPath) : null);
	}

This mutation can break the builder once you try to pass it to MockMvc#perform.
An example of scenario:

1. Make sure MockMvc has a non null defaultRequestBuilder that will be merged in the consumer's provided MockHttpServletRequestBuilder
2. Create MockHttpServletRequestBuilder instance foo
3. Invoke MockHttpServletRequestBuilder#buildRequest on foo (with the injectable ServletContext)
4. Now invoke MockMvc#perform with foo as argument
5. You may end up with a 404 not found

TLDR: calling buildRequest twice on the MockHttpServletRequestBuilder is not safe. This pull request fixes this.

[github-actions]

Fixed via df860fd

[reda-alaoui]

Hello @rstoyanchev ,
Thanks for the merge 🙏

Can I ask why the @author was removed from AbstractMockHttpServletRequestBuilder?
Should I avoid adding this javadoc tag on my next contributions?

[rstoyanchev]

@reda-alaoui it is too minor of an update. We would end up with a very long list if we added everyone who has touched the file if you look at the commit history. There is no clearcut rule I'm afraid for when a change is impactful enough, but in this case it's clearly not.

[reda-alaoui]

I contributed to other projects from the Spring galaxy (e.g. Spring HATEOAS and Spring Data JPA), and this never looked a concern from their maintainers.

Anyway, thanks for your answer 👍

[rstoyanchev]

No problem. A bit of a grey area I suppose.