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

Update MockHttpServletRequest default method to GET [SPR-16851] #21391

Closed
spring-issuemaster opened this issue May 21, 2018 · 4 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented May 21, 2018

Rob Winch opened SPR-16851 and commented

It would be very nice if the default MockHttpServletRequest method were valid. I'd like to suggest updating the default method to a GET.


Affects: 5.0.6

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 21, 2018

Rossen Stoyanchev commented

Juergen Hoeller, Sam Brannen, given that MockHttpServletRequest has always behaved this way, I'd like to ask for opinions.

My concern is how long this behavior has existed and the potential to break tests that rely on code that checks the HTTP method is not of some specific value.

A second point to make is that MockMvc does enforce the use of a specific HTTP method.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 21, 2018

Rob Winch commented

A second point to make is that MockMvc does enforce the use of a specific HTTP method.

There still a large class of applications that do not use Spring MVC but do use Spring Security. For those applications, MockMvc is not that viable of an option.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 21, 2018

Rob Winch commented

Rossen mentioned that it would help if I elaborated a bit to help better understand the reason for this request.

Spring Security is adding support for providing a white list of valid HTTP methods to better protect users against HTTP Verb tampering and XST attacks. Since Spring Security will reject an HTTP method of empty String and MockHttpServletRequest defaults to an empty string HTTP method this means that there will be lots of our users tests that no longer pass. One could argue Spring Security should not make such a change, but the changes should not impact real world applications in a negative manner.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 24, 2018

Rossen Stoyanchev commented

For MockMvc I only meant that it is a more opinionated layer with a start to end request lifecycle that aims to approximate actual behavior of Servlet containers. The MockHttpServletRequest and response on the other hand have always been just a foundation, it's up to anyone to set them up in whatever way fits the test scenario, possibly for a very fine-grained unit test.

So I'm included to say this should be solved at a different level. Either way tests are going to break and it seems reasonable to expect tests that run with Spring Security, and are more integration than unit tests, to have to set up an HTTP method explicitly, or break.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.