From 34d32402d352fe7bca2f45d3bc5c7243459ec556 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 8 Jan 2020 17:55:23 +0000 Subject: [PATCH] Return 500 if producible attribute present When a request is mapped through a producible condition on an @RequestMapping, then a failure to find a converter/decoder should be a 500 because the return type + media type pair were declared by the controller and that should be possible to render. Closes gh-23287 --- .../AbstractMessageWriterResultHandler.java | 9 +++++-- .../ResponseEntityResultHandlerTests.java | 25 ++++++++++++++++++- ...stractMessageConverterMethodProcessor.java | 8 ++++-- .../HttpEntityMethodProcessorMockTests.java | 22 ++++++++++++++-- 4 files changed, 57 insertions(+), 7 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java index 68919bf82ac9..8d13a1f46e0c 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,7 @@ import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; +import java.util.Set; import kotlin.reflect.KFunction; import kotlin.reflect.jvm.ReflectJvmMapping; @@ -36,6 +37,8 @@ import org.springframework.http.converter.HttpMessageNotWritableException; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; +import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.reactive.accept.RequestedContentTypeResolver; import org.springframework.web.reactive.result.HandlerResultHandlerSupport; import org.springframework.web.server.NotAcceptableStatusException; @@ -163,7 +166,9 @@ protected Mono writeBody(@Nullable Object body, MethodParameter bodyParame } MediaType contentType = exchange.getResponse().getHeaders().getContentType(); - if (contentType != null && contentType.equals(bestMediaType)) { + boolean isPresentMediaType = (contentType != null && contentType.equals(bestMediaType)); + Set producibleTypes = exchange.getAttribute(HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE); + if (isPresentMediaType || !CollectionUtils.isEmpty(producibleTypes)) { return Mono.error(new HttpMessageNotWritableException( "No Encoder for [" + elementType + "] with preset Content-Type '" + contentType + "'")); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandlerTests.java index a000cf08587f..f7a5f5afa792 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandlerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandlerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,6 +26,7 @@ import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.CompletableFuture; import org.junit.jupiter.api.BeforeEach; @@ -371,6 +372,28 @@ public void handleWithPresetContentTypeShouldFailWithServerError() { .verify(); } + @Test // gh-23287 + public void handleWithProducibleContentTypeShouldFailWithServerError() { + ResponseEntity value = ResponseEntity.ok().body(""); + MethodParameter returnType = on(TestController.class).resolveReturnType(entity(String.class)); + HandlerResult result = handlerResult(value, returnType); + + MockServerWebExchange exchange = MockServerWebExchange.from(get("/path")); + Set mediaTypes = Collections.singleton(MediaType.APPLICATION_XML); + exchange.getAttributes().put(PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE, mediaTypes); + + ResponseEntityResultHandler resultHandler = new ResponseEntityResultHandler( + Collections.singletonList(new EncoderHttpMessageWriter<>(CharSequenceEncoder.textPlainOnly())), + new RequestedContentTypeResolverBuilder().build() + ); + + StepVerifier.create(resultHandler.handleResult(exchange, result)) + .consumeErrorWith(ex -> assertThat(ex) + .isInstanceOf(HttpMessageNotWritableException.class) + .hasMessageContaining("with preset Content-Type")) + .verify(); + } + private void testHandle(Object returnValue, MethodParameter returnType) { MockServerWebExchange exchange = MockServerWebExchange.from(get("/path")); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java index c19c519d122d..61eaeac00d73 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -309,7 +309,11 @@ else if (mediaType.isPresentIn(ALL_APPLICATION_MEDIA_TYPES)) { } if (body != null) { - if (isContentTypePreset) { + Set producibleMediaTypes = + (Set) inputMessage.getServletRequest() + .getAttribute(HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE); + + if (isContentTypePreset || !CollectionUtils.isEmpty(producibleMediaTypes)) { throw new HttpMessageNotWritableException( "No converter for [" + valueType + "] with preset Content-Type '" + contentType + "'"); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java index b2e85cdf567a..dde74a7ceb39 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,6 +28,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.Date; +import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -323,7 +324,24 @@ public void shouldFailWithServerErrorIfContentTypeFromResponseEntity() { .contentType(MediaType.APPLICATION_XML) .body(""); - given(stringHttpMessageConverter.canWrite(String.class, null)).willReturn(true); + given(stringHttpMessageConverter.canWrite(String.class, TEXT_PLAIN)).willReturn(true); + given(stringHttpMessageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(TEXT_PLAIN)); + + assertThatThrownBy(() -> + processor.handleReturnValue( + returnValue, returnTypeResponseEntity, mavContainer, webRequest)) + .isInstanceOf(HttpMessageNotWritableException.class) + .hasMessageContaining("with preset Content-Type"); + } + + @Test // gh-23287 + public void shouldFailWithServerErrorIfContentTypeFromProducibleAttribute() { + Set mediaTypes = Collections.singleton(MediaType.APPLICATION_XML); + servletRequest.setAttribute(PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE, mediaTypes); + + ResponseEntity returnValue = ResponseEntity.ok().body(""); + + given(stringHttpMessageConverter.canWrite(String.class, TEXT_PLAIN)).willReturn(true); given(stringHttpMessageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(TEXT_PLAIN)); assertThatThrownBy(() ->