Skip to content

Commit 583a48a

Browse files
committed
Do not process conditional requests for non-GET
Prior to this commit, HttpEntityMethodProcessor would process conditional requests even if those aren't GET requests. This is an issue for POST requests with "If-None-Match: *" headers and many other use cases, which should not receive an HTTP 304 Not Modified status in response. This commit only triggers ETag/Last-Modified conditional requests bits for GET requests. Issue: SPR-13496
1 parent 525d111 commit 583a48a

File tree

2 files changed

+31
-2
lines changed

2 files changed

+31
-2
lines changed

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,8 @@ public void handleReturnValue(Object returnValue, MethodParameter returnType,
172172
Object body = responseEntity.getBody();
173173
if (responseEntity instanceof ResponseEntity) {
174174
outputMessage.setStatusCode(((ResponseEntity<?>) responseEntity).getStatusCode());
175-
if (isResourceNotModified(inputMessage, outputMessage)) {
175+
if (inputMessage.getServletRequest().getMethod() == "GET"
176+
&& isResourceNotModified(inputMessage, outputMessage)) {
176177
outputMessage.setStatusCode(HttpStatus.NOT_MODIFIED);
177178
// Ensure headers are flushed, no body should be written.
178179
outputMessage.flush();

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public void setUp() throws Exception {
112112
returnTypeInt = new MethodParameter(getClass().getMethod("handle3"), -1);
113113

114114
mavContainer = new ModelAndViewContainer();
115-
servletRequest = new MockHttpServletRequest("POST", "/foo");
115+
servletRequest = new MockHttpServletRequest("GET", "/foo");
116116
servletResponse = new MockHttpServletResponse();
117117
webRequest = new ServletWebRequest(servletRequest, servletResponse);
118118
}
@@ -182,6 +182,7 @@ public void resolveArgumentRequestEntity() throws Exception {
182182
@Test(expected = HttpMediaTypeNotSupportedException.class)
183183
public void resolveArgumentNotReadable() throws Exception {
184184
MediaType contentType = MediaType.TEXT_PLAIN;
185+
servletRequest.setMethod("POST");
185186
servletRequest.addHeader("Content-Type", contentType.toString());
186187

187188
given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(contentType));
@@ -194,6 +195,7 @@ public void resolveArgumentNotReadable() throws Exception {
194195

195196
@Test(expected = HttpMediaTypeNotSupportedException.class)
196197
public void resolveArgumentNoContentType() throws Exception {
198+
servletRequest.setMethod("POST");
197199
servletRequest.setContent("some content".getBytes(Charset.forName("UTF-8")));
198200
processor.resolveArgument(paramHttpEntity, mavContainer, webRequest, null);
199201
fail("Expected exception");
@@ -455,6 +457,32 @@ public void handleReturnTypeChangedETagAndLastModified() throws Exception {
455457
assertEquals(0, servletResponse.getContentAsByteArray().length);
456458
}
457459

460+
// SPR-13496
461+
@Test
462+
public void handleReturnTypePostRequestWithIfNotModified() throws Exception {
463+
String wildcardValue = "*";
464+
String etagValue = "\"some-etag\"";
465+
servletRequest.setMethod("POST");
466+
servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, wildcardValue);
467+
HttpHeaders responseHeaders = new HttpHeaders();
468+
responseHeaders.set(HttpHeaders.ETAG, etagValue);
469+
ResponseEntity<String> returnValue = new ResponseEntity<String>("body", responseHeaders, HttpStatus.OK);
470+
471+
given(messageConverter.canWrite(String.class, null)).willReturn(true);
472+
given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN));
473+
given(messageConverter.canWrite(String.class, MediaType.TEXT_PLAIN)).willReturn(true);
474+
475+
476+
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
477+
478+
assertTrue(mavContainer.isRequestHandled());
479+
assertEquals(HttpStatus.OK.value(), servletResponse.getStatus());
480+
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
481+
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
482+
ArgumentCaptor<HttpOutputMessage> outputMessage = ArgumentCaptor.forClass(HttpOutputMessage.class);
483+
verify(messageConverter).write(eq("body"), eq(MediaType.TEXT_PLAIN), outputMessage.capture());
484+
}
485+
458486
@SuppressWarnings("unused")
459487
public ResponseEntity<String> handle1(HttpEntity<String> httpEntity, ResponseEntity<String> entity,
460488
int i, RequestEntity<String> requestEntity) {

0 commit comments

Comments
 (0)