From d8441fc80c1657929900bbf6069c8b27d36bb87e Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Thu, 9 Nov 2023 12:52:14 +0000 Subject: [PATCH] Invoke handleEmptyBody if there is no content-type Closes gh-30522 --- ...essageConverterMethodArgumentResolver.java | 44 ++++++++++- .../RequestMappingHandlerAdapterTests.java | 77 +++++++++++++++---- 2 files changed, 104 insertions(+), 17 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java index f0039aa242..a3bfda4064 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java @@ -23,6 +23,7 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; @@ -38,6 +39,7 @@ import org.springframework.core.log.LogFormatUtils; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpInputMessage; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpOutputMessage; import org.springframework.http.HttpRequest; import org.springframework.http.InvalidMediaTypeException; import org.springframework.http.MediaType; @@ -170,7 +172,6 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements EmptyBodyCheckingHttpInputMessage message = null; try { message = new EmptyBodyCheckingHttpInputMessage(inputMessage); - for (HttpMessageConverter converter : this.messageConverters) { Class> converterType = (Class>) converter.getClass(); GenericHttpMessageConverter genericConverter = @@ -190,6 +191,10 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements break; } } + if (body == NO_VALUE && noContentType && !message.hasBody()) { + body = getAdvice().handleEmptyBody( + null, message, parameter, targetType, NoContentTypeHttpMessageConverter.class); + } } catch (IOException ex) { throw new HttpMessageNotReadableException("I/O error while reading input message", ex, inputMessage); @@ -201,8 +206,7 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements } if (body == NO_VALUE) { - if (httpMethod == null || !SUPPORTED_METHODS.contains(httpMethod) || - (noContentType && !message.hasBody())) { + if (httpMethod == null || !SUPPORTED_METHODS.contains(httpMethod) || (noContentType && !message.hasBody())) { return null; } throw new HttpMediaTypeNotSupportedException(contentType, @@ -354,4 +358,38 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements } } + + /** + * Placeholder HttpMessageConverter type to pass to RequestBodyAdvice if there + * is no content-type and no content. In that case, we may not find a converter, + * but RequestBodyAdvice have a chance to provide it via handleEmptyBody. + */ + private static class NoContentTypeHttpMessageConverter implements HttpMessageConverter { + + @Override + public boolean canRead(Class clazz, @Nullable MediaType mediaType) { + return false; + } + + @Override + public boolean canWrite(Class clazz, @Nullable MediaType mediaType) { + return false; + } + + @Override + public List getSupportedMediaTypes() { + return Collections.emptyList(); + } + + @Override + public String read(Class clazz, HttpInputMessage inputMessage) { + throw new UnsupportedOperationException(); + } + + @Override + public void write(String s, @Nullable MediaType contentType, HttpOutputMessage outputMessage) { + throw new UnsupportedOperationException(); + } + } + } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterTests.java index cdf6f16b28..b02683151a 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterTests.java @@ -25,6 +25,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import org.apache.groovy.util.Maps; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -45,6 +46,8 @@ import org.springframework.lang.Nullable; import org.springframework.ui.Model; import org.springframework.web.bind.annotation.ControllerAdvice; import org.springframework.web.bind.annotation.ModelAttribute; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.annotation.SessionAttributes; import org.springframework.web.context.support.StaticWebApplicationContext; import org.springframework.web.method.HandlerMethod; @@ -109,7 +112,7 @@ public class RequestMappingHandlerAdapterTests { @Test public void cacheControlWithoutSessionAttributes() throws Exception { - HandlerMethod handlerMethod = handlerMethod(new SimpleController(), "handle"); + HandlerMethod handlerMethod = handlerMethod(new TestController(), "handle"); this.handlerAdapter.setCacheSeconds(100); this.handlerAdapter.afterPropertiesSet(); @@ -197,7 +200,7 @@ public class RequestMappingHandlerAdapterTests { this.webAppContext.registerSingleton("maa", ModelAttributeAdvice.class); this.webAppContext.refresh(); - HandlerMethod handlerMethod = handlerMethod(new SimpleController(), "handle"); + HandlerMethod handlerMethod = handlerMethod(new TestController(), "handle"); this.handlerAdapter.afterPropertiesSet(); ModelAndView mav = this.handlerAdapter.handle(this.request, this.response, handlerMethod); @@ -210,7 +213,7 @@ public class RequestMappingHandlerAdapterTests { this.webAppContext.registerPrototype("maa", ModelAttributeAdvice.class); this.webAppContext.refresh(); - HandlerMethod handlerMethod = handlerMethod(new SimpleController(), "handle"); + HandlerMethod handlerMethod = handlerMethod(new TestController(), "handle"); this.handlerAdapter.afterPropertiesSet(); Map model1 = this.handlerAdapter.handle(this.request, this.response, handlerMethod).getModel(); Map model2 = this.handlerAdapter.handle(this.request, this.response, handlerMethod).getModel(); @@ -226,7 +229,7 @@ public class RequestMappingHandlerAdapterTests { this.webAppContext.setParent(parent); this.webAppContext.refresh(); - HandlerMethod handlerMethod = handlerMethod(new SimpleController(), "handle"); + HandlerMethod handlerMethod = handlerMethod(new TestController(), "handle"); this.handlerAdapter.afterPropertiesSet(); ModelAndView mav = this.handlerAdapter.handle(this.request, this.response, handlerMethod); @@ -240,7 +243,7 @@ public class RequestMappingHandlerAdapterTests { this.webAppContext.registerSingleton("manupa", ModelAttributeNotUsedPackageAdvice.class); this.webAppContext.refresh(); - HandlerMethod handlerMethod = handlerMethod(new SimpleController(), "handle"); + HandlerMethod handlerMethod = handlerMethod(new TestController(), "handle"); this.handlerAdapter.afterPropertiesSet(); ModelAndView mav = this.handlerAdapter.handle(this.request, this.response, handlerMethod); @@ -249,9 +252,7 @@ public class RequestMappingHandlerAdapterTests { assertThat(mav.getModel().get("attr3")).isNull(); } - // SPR-10859 - - @Test + @Test // gh-15486 public void responseBodyAdvice() throws Exception { List> converters = new ArrayList<>(); converters.add(new MappingJackson2HttpMessageConverter()); @@ -263,7 +264,7 @@ public class RequestMappingHandlerAdapterTests { this.request.addHeader("Accept", MediaType.APPLICATION_JSON_VALUE); this.request.setParameter("c", "callback"); - HandlerMethod handlerMethod = handlerMethod(new SimpleController(), "handleBadRequest"); + HandlerMethod handlerMethod = handlerMethod(new TestController(), "handleBadRequest"); this.handlerAdapter.afterPropertiesSet(); this.handlerAdapter.handle(this.request, this.response, handlerMethod); @@ -271,6 +272,19 @@ public class RequestMappingHandlerAdapterTests { assertThat(this.response.getContentAsString()).isEqualTo("{\"status\":400,\"message\":\"body\"}"); } + @Test // gh-30522 + public void responseBodyAdviceWithEmptyBody() throws Exception { + this.webAppContext.registerBean("rba", EmptyBodyAdvice.class); + this.webAppContext.refresh(); + + HandlerMethod handlerMethod = handlerMethod(new TestController(), "handleBody", Map.class); + this.handlerAdapter.afterPropertiesSet(); + this.handlerAdapter.handle(this.request, this.response, handlerMethod); + + assertThat(this.response.getStatus()).isEqualTo(200); + assertThat(this.response.getContentAsString()).isEqualTo("Body: {foo=bar}"); + } + private HandlerMethod handlerMethod(Object handler, String methodName, Class... paramTypes) throws Exception { Method method = handler.getClass().getDeclaredMethod(methodName, paramTypes); return new InvocableHandlerMethod(handler, method); @@ -284,7 +298,7 @@ public class RequestMappingHandlerAdapterTests { @SuppressWarnings("unused") - private static class SimpleController { + private static class TestController { @ModelAttribute public void addAttributes(Model model) { @@ -296,14 +310,17 @@ public class RequestMappingHandlerAdapterTests { } public ResponseEntity> handleWithResponseEntity() { - return new ResponseEntity<>(Collections.singletonMap( - "foo", "bar"), HttpStatus.OK); + return new ResponseEntity<>(Collections.singletonMap("foo", "bar"), HttpStatus.OK); } public ResponseEntity handleBadRequest() { return new ResponseEntity<>("body", HttpStatus.BAD_REQUEST); } + @ResponseBody + public String handleBody(@Nullable @RequestBody Map body) { + return "Body: " + body; + } } @@ -360,6 +377,7 @@ public class RequestMappingHandlerAdapterTests { } } + /** * This class additionally implements {@link RequestBodyAdvice} solely for the purpose * of verifying that controller advice implementing both {@link ResponseBodyAdvice} @@ -368,7 +386,8 @@ public class RequestMappingHandlerAdapterTests { * @see gh-22638 */ @ControllerAdvice - private static class ResponseCodeSuppressingAdvice extends AbstractMappingJacksonResponseBodyAdvice implements RequestBodyAdvice { + private static class ResponseCodeSuppressingAdvice + extends AbstractMappingJacksonResponseBodyAdvice implements RequestBodyAdvice { @Override protected void beforeBodyWriteInternal(MappingJacksonValue bodyContainer, MediaType contentType, @@ -405,12 +424,42 @@ public class RequestMappingHandlerAdapterTests { } @Override - public Object handleEmptyBody(@Nullable Object body, HttpInputMessage inputMessage, MethodParameter parameter, + public Object handleEmptyBody(Object body, HttpInputMessage inputMessage, MethodParameter parameter, Type targetType, Class> converterType) { return "default value for empty body"; } + } + + @ControllerAdvice + private static class EmptyBodyAdvice implements RequestBodyAdvice { + + @Override + public boolean supports(MethodParameter param, Type targetType, Class> type) { + return true; + } + + @Override + public HttpInputMessage beforeBodyRead(HttpInputMessage message, MethodParameter param, + Type targetType, Class> type) { + + throw new UnsupportedOperationException(); + } + + @Override + public Object afterBodyRead(Object body, HttpInputMessage message, MethodParameter param, + Type targetType, Class> type) { + + throw new UnsupportedOperationException(); + } + + @Override + public Object handleEmptyBody(Object body, HttpInputMessage message, MethodParameter param, + Type targetType, Class> type) { + + return Maps.of("foo", "bar"); + } } }