Cwikius-Spring
/
spring-security
mirror of https://github.com/spring-projects/spring-security.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Resolve The matchingRequestParameterName From The Query String 

Prior to this commit, the ServletRequest#getParameter method was used in order to verify if the matchingRequestParameterName was present in the request. That method has some side effects like interfering in the execution of the ServletRequest#getInputStream and ServletRequest#getReader method when the request is an HTTP POST (if those methods are invoked after getParameter, or vice-versa, the content won't be available). This commit makes that we only use the query string to check for the parameter, avoiding draining the request's input stream.

Closes gh-13731
This commit is contained in:
Marcus Da Coregio 2023-09-14 17:09:56 +01:00 committed by Marcus Hert Da Coregio
parent aeafcc1377
commit 18e88366d2
2 changed files with 29 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
web/src/main/java/org/springframework/security/web/savedrequest/HttpSessionRequestCache.java
View File

@ -28,6 +28,8 @@ import org.springframework.security.web.PortResolverImpl;
import org.springframework.security.web.util.UrlUtils;
import org.springframework.security.web.util.matcher.AnyRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher;
import org.springframework.util.StringUtils;
import org.springframework.web.util.UriComponentsBuilder;
/**
* {@code RequestCache} which stores the {@code SavedRequest} in the HttpSession.
@ -100,11 +102,14 @@ public class HttpSessionRequestCache implements RequestCache {
@Override
public HttpServletRequest getMatchingRequest(HttpServletRequest request, HttpServletResponse response) {
if (this.matchingRequestParameterName != null
&& request.getParameter(this.matchingRequestParameterName) == null) {
this.logger.trace(
"matchingRequestParameterName is required for getMatchingRequest to lookup a value, but not provided");
return null;
if (this.matchingRequestParameterName != null) {
if (!StringUtils.hasText(request.getQueryString())
|| !UriComponentsBuilder.fromUriString(UrlUtils.buildRequestUrl(request)).build().getQueryParams()
.containsKey(this.matchingRequestParameterName)) {
this.logger.trace(
"matchingRequestParameterName is required for getMatchingRequest to lookup a value, but not provided");
return null;
}
}
SavedRequest saved = getRequest(request, response);
if (saved == null) {

24
web/src/test/java/org/springframework/security/web/savedrequest/HttpSessionRequestCacheTests.java
View File

@ -32,8 +32,9 @@ import org.springframework.security.web.PortResolverImpl;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.Mockito.mock;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
/**
@ -100,7 +101,7 @@ public class HttpSessionRequestCacheTests {
public void getMatchingRequestWhenMatchingRequestParameterNameSetThenSessionNotAccessed() {
HttpSessionRequestCache cache = new HttpSessionRequestCache();
cache.setMatchingRequestParameterName("success");
HttpServletRequest request = mock(HttpServletRequest.class);
HttpServletRequest request = spy(new MockHttpServletRequest());
HttpServletRequest matchingRequest = cache.getMatchingRequest(request, new MockHttpServletResponse());
assertThat(matchingRequest).isNull();
verify(request, never()).getSession();
@ -115,7 +116,6 @@ public class HttpSessionRequestCacheTests {
cache.saveRequest(request, new MockHttpServletResponse());
MockHttpServletRequest requestToMatch = new MockHttpServletRequest();
requestToMatch.setQueryString("success"); // gh-12665
requestToMatch.setParameter("success", "");
requestToMatch.setSession(request.getSession());
HttpServletRequest matchingRequest = cache.getMatchingRequest(requestToMatch, new MockHttpServletResponse());
assertThat(matchingRequest).isNotNull();
@ -131,7 +131,6 @@ public class HttpSessionRequestCacheTests {
cache.saveRequest(request, new MockHttpServletResponse());
MockHttpServletRequest requestToMatch = new MockHttpServletRequest();
requestToMatch.setQueryString("param=true&success");
requestToMatch.setParameter("success", "");
requestToMatch.setSession(request.getSession());
HttpServletRequest matchingRequest = cache.getMatchingRequest(requestToMatch, new MockHttpServletResponse());
assertThat(matchingRequest).isNotNull();
@ -146,13 +145,28 @@ public class HttpSessionRequestCacheTests {
assertThat(request.getSession().getAttribute(HttpSessionRequestCache.SAVED_REQUEST)).isNotNull();
MockHttpServletRequest requestToMatch = new MockHttpServletRequest();
requestToMatch.setQueryString("success");
requestToMatch.setParameter("success", "");
requestToMatch.setSession(request.getSession());
HttpServletRequest matchingRequest = cache.getMatchingRequest(requestToMatch, new MockHttpServletResponse());
assertThat(matchingRequest).isNotNull();
assertThat(request.getSession().getAttribute(HttpSessionRequestCache.SAVED_REQUEST)).isNull();
}
// gh-13731
@Test
public void getMatchingRequestWhenMatchingRequestParameterNameSetThenDoesNotInvokeGetParameterMethods() {
HttpSessionRequestCache cache = new HttpSessionRequestCache();
cache.setMatchingRequestParameterName("success");
MockHttpServletRequest mockRequest = new MockHttpServletRequest();
mockRequest.setQueryString("success");
HttpServletRequest request = spy(mockRequest);
HttpServletRequest matchingRequest = cache.getMatchingRequest(request, new MockHttpServletResponse());
assertThat(matchingRequest).isNull();
verify(request, never()).getParameter(anyString());
verify(request, never()).getParameterValues(anyString());
verify(request, never()).getParameterNames();
verify(request, never()).getParameterMap();
}
private static final class CustomSavedRequest implements SavedRequest {
private final SavedRequest delegate;