Skip to content

Commit

Permalink
Return 500 (not 406) if content-type was preset
Browse files Browse the repository at this point in the history
If content-type is preset in the returned ResponseEntity, then any
failure to find a matching converter is a server error.

Closes gh-23205
  • Loading branch information
rstoyanchev committed Jul 5, 2019
1 parent 3d913b8 commit 37f9ce5
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,17 @@ protected Mono<Void> writeBody(@Nullable Object body, MethodParameter bodyParame
}
}

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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ public void handleMonoWithWildcardBodyTypeAndNullBody() throws Exception {
}

@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);
Expand All @@ -351,6 +351,24 @@ public void handleResponseEntityWithExistingResponseHeaders() throws Exception {
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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,8 @@ protected <T> void writeWithMessageConverters(@Nullable T value, MethodParameter

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");
}
Expand Down Expand Up @@ -304,6 +305,10 @@ else if (mediaType.isPresentIn(ALL_APPLICATION_MEDIA_TYPES)) {
}

if (body != null) {
if (isContentTypePreset) {
throw new IllegalStateException(
"No converter for [" + valueType + "] with preset Content-Type '" + contentType + "'");
}
throw new HttpMediaTypeNotAcceptableException(this.allSupportedMediaTypes);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
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;
Expand Down Expand Up @@ -315,6 +316,21 @@ public void shouldFailHandlingWhenContentTypeNotSupported() throws Exception {
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";
Expand Down

0 comments on commit 37f9ce5

Please sign in to comment.