Skip to content

Commit

Permalink
Return 500 if producible attribute present
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rstoyanchev committed Jan 9, 2020
1 parent 1ec15ba commit 34d3240
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 7 deletions.
@@ -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.
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -163,7 +166,9 @@ protected Mono<Void> 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<MediaType> 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 + "'"));
}
Expand Down
@@ -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.
Expand All @@ -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;
Expand Down Expand Up @@ -371,6 +372,28 @@ public void handleWithPresetContentTypeShouldFailWithServerError() {
.verify();
}

@Test // gh-23287
public void handleWithProducibleContentTypeShouldFailWithServerError() {
ResponseEntity<String> value = ResponseEntity.ok().body("<foo/>");
MethodParameter returnType = on(TestController.class).resolveReturnType(entity(String.class));
HandlerResult result = handlerResult(value, returnType);

MockServerWebExchange exchange = MockServerWebExchange.from(get("/path"));
Set<MediaType> 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"));
Expand Down
@@ -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.
Expand Down Expand Up @@ -309,7 +309,11 @@ else if (mediaType.isPresentIn(ALL_APPLICATION_MEDIA_TYPES)) {
}

if (body != null) {
if (isContentTypePreset) {
Set<MediaType> producibleMediaTypes =
(Set<MediaType>) 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 + "'");
}
Expand Down
@@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -323,7 +324,24 @@ public void shouldFailWithServerErrorIfContentTypeFromResponseEntity() {
.contentType(MediaType.APPLICATION_XML)
.body("<foo/>");

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<MediaType> mediaTypes = Collections.singleton(MediaType.APPLICATION_XML);
servletRequest.setAttribute(PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE, mediaTypes);

ResponseEntity<String> returnValue = ResponseEntity.ok().body("<foo/>");

given(stringHttpMessageConverter.canWrite(String.class, TEXT_PLAIN)).willReturn(true);
given(stringHttpMessageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(TEXT_PLAIN));

assertThatThrownBy(() ->
Expand Down

0 comments on commit 34d3240

Please sign in to comment.