Return 500 (not 406) if content-type was preset
If content-type is preset in the returned ResponseEntity, then any failure to find a matching converter is a server error. Closes gh-23205
This commit is contained in:
@@ -161,10 +161,17 @@ public abstract class AbstractMessageWriterResultHandler extends HandlerResultHa
|
||||
}
|
||||
}
|
||||
|
||||
MediaType contentType = exchange.getResponse().getHeaders().getContentType();
|
||||
if (contentType != null && contentType.equals(bestMediaType)) {
|
||||
return Mono.error(new IllegalStateException(
|
||||
"No Encoder for [" + elementType + "] with preset Content-Type '" + contentType + "'"));
|
||||
}
|
||||
|
||||
List<MediaType> mediaTypes = getMediaTypesFor(elementType);
|
||||
if (bestMediaType == null && mediaTypes.isEmpty()) {
|
||||
return Mono.error(new IllegalStateException("No HttpMessageWriter for " + elementType));
|
||||
}
|
||||
|
||||
return Mono.error(new NotAcceptableStatusException(mediaTypes));
|
||||
}
|
||||
|
||||
|
||||
@@ -337,7 +337,7 @@ public class ResponseEntityResultHandlerTests {
|
||||
}
|
||||
|
||||
@Test // SPR-17082
|
||||
public void handleResponseEntityWithExistingResponseHeaders() throws Exception {
|
||||
public void handleWithPresetContentType() {
|
||||
ResponseEntity<Void> value = ResponseEntity.ok().contentType(MediaType.APPLICATION_JSON).build();
|
||||
MethodParameter returnType = on(TestController.class).resolveReturnType(entity(Void.class));
|
||||
HandlerResult result = handlerResult(value, returnType);
|
||||
@@ -351,6 +351,24 @@ public class ResponseEntityResultHandlerTests {
|
||||
assertResponseBodyIsEmpty(exchange);
|
||||
}
|
||||
|
||||
@Test // gh-23205
|
||||
public void handleWithPresetContentTypeShouldFailWithServerError() {
|
||||
ResponseEntity<String> value = ResponseEntity.ok().contentType(MediaType.APPLICATION_XML).body("<foo/>");
|
||||
MethodParameter returnType = on(TestController.class).resolveReturnType(entity(String.class));
|
||||
HandlerResult result = handlerResult(value, returnType);
|
||||
|
||||
MockServerWebExchange exchange = MockServerWebExchange.from(get("/path"));
|
||||
ResponseEntityResultHandler resultHandler = new ResponseEntityResultHandler(
|
||||
Collections.singletonList(new EncoderHttpMessageWriter<>(CharSequenceEncoder.textPlainOnly())),
|
||||
new RequestedContentTypeResolverBuilder().build()
|
||||
);
|
||||
|
||||
StepVerifier.create(resultHandler.handleResult(exchange, result))
|
||||
.consumeErrorWith(ex -> assertThat(ex)
|
||||
.isInstanceOf(IllegalStateException.class)
|
||||
.hasMessageContaining("with preset Content-Type"))
|
||||
.verify();
|
||||
}
|
||||
|
||||
|
||||
private void testHandle(Object returnValue, MethodParameter returnType) {
|
||||
|
||||
@@ -218,7 +218,8 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
|
||||
|
||||
MediaType selectedMediaType = null;
|
||||
MediaType contentType = outputMessage.getHeaders().getContentType();
|
||||
if (contentType != null && contentType.isConcrete()) {
|
||||
boolean isContentTypePreset = contentType != null && contentType.isConcrete();
|
||||
if (isContentTypePreset) {
|
||||
if (logger.isDebugEnabled()) {
|
||||
logger.debug("Found 'Content-Type:" + contentType + "' in response");
|
||||
}
|
||||
@@ -304,6 +305,10 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
|
||||
}
|
||||
|
||||
if (body != null) {
|
||||
if (isContentTypePreset) {
|
||||
throw new IllegalStateException(
|
||||
"No converter for [" + valueType + "] with preset Content-Type '" + contentType + "'");
|
||||
}
|
||||
throw new HttpMediaTypeNotAcceptableException(this.allSupportedMediaTypes);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -59,6 +59,7 @@ import static java.time.Instant.ofEpochMilli;
|
||||
import static java.time.format.DateTimeFormatter.RFC_1123_DATE_TIME;
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
|
||||
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.anyCollection;
|
||||
import static org.mockito.ArgumentMatchers.argThat;
|
||||
@@ -315,6 +316,21 @@ public class HttpEntityMethodProcessorMockTests {
|
||||
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest));
|
||||
}
|
||||
|
||||
@Test // gh-23205
|
||||
public void shouldFailWithServerErrorIfContentTypeFromResponseEntity() {
|
||||
ResponseEntity<String> returnValue = ResponseEntity.ok()
|
||||
.contentType(MediaType.APPLICATION_XML)
|
||||
.body("<foo/>");
|
||||
|
||||
given(stringHttpMessageConverter.canWrite(String.class, null)).willReturn(true);
|
||||
given(stringHttpMessageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(TEXT_PLAIN));
|
||||
|
||||
assertThatIllegalStateException()
|
||||
.isThrownBy(() ->
|
||||
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest))
|
||||
.withMessageContaining("with preset Content-Type");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldFailHandlingWhenConverterCannotWrite() throws Exception {
|
||||
String body = "Foo";
|
||||
|
||||
Reference in New Issue
Block a user