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

ignore Multipart requests in HttpSessionRequestCache.requestMatcher #7167

Conversation

a-sayyed
Copy link
Contributor

references: #7060

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 31, 2019
@a-sayyed
Copy link
Contributor Author

I have a question regarding testing, since I'm not very familiar with the test setup.
I found the RequestCacheConfigurerTests class, and thought about writing an extra test method,
something like:

// SEC-7060
@Test
public void postWhenRequestIsMultipartThenRequestShouldNotBeCached() throws Exception {
	this.spring.register(DefaultSecurityConfig.class, MultipartFileController.class).autowire();
	MockMultipartFile aFile = new MockMultipartFile("aFile", "aFile", MediaType.TEXT_PLAIN_VALUE, 
RandomStringUtils.random(1024).getBytes());

	this.mvc.perform(multipart("/upload")
		.file(aFile))
		.andDo(print())
		.andExpect(status().isUnauthorized())
		.andReturn();
	}

@Controller
static class MultipartFileController {
	@PostMapping("/upload")
	public ResponseEntity<String> upload(MultipartFile multipartFile){
		return ResponseEntity.ok().build();
	}
}

I have a few questions:

  • is this the way to go?
  • do I need to register the controller, or it doesn't matter in this context?
  • how do I really check if the request parameters are not cached?

@a-sayyed a-sayyed marked this pull request as ready for review August 5, 2019 12:07
@rwinch
Copy link
Member

rwinch commented Aug 5, 2019

Thanks for working this issue. See if this test helps https://github.com/spring-projects/spring-security/blob/5.2.0.M4/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurerTests.java#L126

The gist is:

  • make a multipart request before authenticating and save that session object
  • Authenticate with the session object from before
  • ensure you are not redirected to the URL from the multipart request

@a-sayyed
Copy link
Contributor Author

Thanks for working this issue. See if this test helps https://github.com/spring-projects/spring-security/blob/5.2.0.M4/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurerTests.java#L126

The gist is:

* make a multipart request before authenticating and save that session object

* Authenticate with the session object from before

* ensure you are not redirected to the URL from the multipart request

Thanks @rwinch for the hint, I have added the test as you suggested.

@rwinch rwinch merged commit 1ab05da into spring-projects:master Aug 15, 2019
@rwinch rwinch self-assigned this Aug 15, 2019
@rwinch rwinch added in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 15, 2019
@rwinch rwinch added this to the 5.2.0.RC1 milestone Aug 15, 2019
@rwinch
Copy link
Member

rwinch commented Aug 15, 2019

Thanks for the PR @a-sayyed! This is now merged into master

@rwinch rwinch added the status: duplicate A duplicate of another issue label Aug 15, 2019
@a-sayyed a-sayyed deleted the bugfix/ignore_multipart_requests_in_HttpSessionRequestCache branch April 27, 2020 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants