From 66cece6a2c8c378b761d6d03a8b50f4004b06679 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Fri, 26 Jul 2024 15:38:11 +0200 Subject: [PATCH] Support application/graphql with charset information Prior to this commit, gh-948 added a fallback support for the "application/graphql" content-type sent by clients. This media type is not widely used and advised against by the spec group. This fallback checked for an exact match of the content type, not taking into account potential media type parameters such as charset. This commit ensure that a `MediaType#include` comparison is used to trigger the fallback. Fixes gh-1036 --- .../webflux/AbstractGraphQlHttpHandler.java | 24 ++++++++++------ .../webmvc/AbstractGraphQlHttpHandler.java | 28 +++++++++++-------- .../webflux/GraphQlHttpHandlerTests.java | 16 +++++++++++ .../webmvc/GraphQlHttpHandlerTests.java | 9 ++++++ 4 files changed, 57 insertions(+), 20 deletions(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/AbstractGraphQlHttpHandler.java b/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/AbstractGraphQlHttpHandler.java index 6f869254..f0a0631d 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/AbstractGraphQlHttpHandler.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/AbstractGraphQlHttpHandler.java @@ -36,6 +36,7 @@ import org.springframework.http.codec.CodecConfigurer; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; +import org.springframework.util.StringUtils; import org.springframework.web.reactive.function.server.ServerRequest; import org.springframework.web.reactive.function.server.ServerResponse; import org.springframework.web.server.UnsupportedMediaTypeStatusException; @@ -114,15 +115,20 @@ public abstract class AbstractGraphQlHttpHandler { private static Mono applyApplicationGraphQlFallback( UnsupportedMediaTypeStatusException ex, ServerRequest request) { - // Spec requires application/json but some clients still use application/graphql - return "application/graphql".equals(request.headers().firstHeader(HttpHeaders.CONTENT_TYPE)) ? - ServerRequest.from(request) - .headers((headers) -> headers.setContentType(MediaType.APPLICATION_JSON)) - .body(request.bodyToFlux(DataBuffer.class)) - .build() - .bodyToMono(SerializableGraphQlRequest.class) - .log() : - Mono.error(ex); + String contentTypeHeader = request.headers().firstHeader(HttpHeaders.CONTENT_TYPE); + if (StringUtils.hasText(contentTypeHeader)) { + MediaType contentType = MediaType.parseMediaType(contentTypeHeader); + MediaType applicationGraphQl = MediaType.parseMediaType("application/graphql"); + + // Spec requires application/json but some clients still use application/graphql + return applicationGraphQl.includes(contentType) ? ServerRequest.from(request) + .headers((headers) -> headers.setContentType(MediaType.APPLICATION_JSON)) + .body(request.bodyToFlux(DataBuffer.class)) + .build() + .bodyToMono(SerializableGraphQlRequest.class) + .log() : Mono.error(ex); + } + return Mono.error(ex); } /** diff --git a/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/AbstractGraphQlHttpHandler.java b/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/AbstractGraphQlHttpHandler.java index f697dd05..faba51c4 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/AbstractGraphQlHttpHandler.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/AbstractGraphQlHttpHandler.java @@ -49,6 +49,7 @@ import org.springframework.util.CollectionUtils; import org.springframework.util.IdGenerator; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import org.springframework.util.StringUtils; import org.springframework.web.HttpMediaTypeNotSupportedException; import org.springframework.web.server.ServerWebInputException; import org.springframework.web.servlet.ModelAndView; @@ -170,17 +171,22 @@ public abstract class AbstractGraphQlHttpHandler { private static SerializableGraphQlRequest applyApplicationGraphQlFallback( ServerRequest request, HttpMediaTypeNotSupportedException ex) throws HttpMediaTypeNotSupportedException { - // Spec requires application/json but some clients still use application/graphql - if ("application/graphql".equals(request.headers().firstHeader(HttpHeaders.CONTENT_TYPE))) { - try { - request = ServerRequest.from(request) - .headers((headers) -> headers.setContentType(MediaType.APPLICATION_JSON)) - .body(request.body(byte[].class)) - .build(); - return request.body(SerializableGraphQlRequest.class); - } - catch (Throwable ex2) { - // ignore + String contentTypeHeader = request.headers().firstHeader(HttpHeaders.CONTENT_TYPE); + if (StringUtils.hasText(contentTypeHeader)) { + MediaType contentType = MediaType.parseMediaType(contentTypeHeader); + MediaType applicationGraphQl = MediaType.parseMediaType("application/graphql"); + // Spec requires application/json but some clients still use application/graphql + if (applicationGraphQl.includes(contentType)) { + try { + request = ServerRequest.from(request) + .headers((headers) -> headers.setContentType(MediaType.APPLICATION_JSON)) + .body(request.body(byte[].class)) + .build(); + return request.body(SerializableGraphQlRequest.class); + } + catch (Throwable ex2) { + // ignore + } } } throw ex; diff --git a/spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlHttpHandlerTests.java b/spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlHttpHandlerTests.java index 768ac766..e8648e8f 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlHttpHandlerTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlHttpHandlerTests.java @@ -99,6 +99,22 @@ public class GraphQlHttpHandlerTests { .verifyComplete(); } + @Test + void shouldSupportApplicationGraphQlWithCharset() throws Exception { + String document = "{greeting}"; + MockServerHttpRequest httpRequest = MockServerHttpRequest.post("/") + .contentType(MediaType.parseMediaType("application/graphql;charset=UTF-8")) + .accept(MediaType.ALL) + .body(initRequestBody(document)); + + MockServerHttpResponse response = handleRequest(httpRequest, this.greetingHandler); + + assertThat(response.getHeaders().getContentType()).isEqualTo(MediaType.APPLICATION_JSON); + StepVerifier.create(response.getBodyAsString()) + .expectNext("{\"data\":{\"greeting\":\"Hello\"}}") + .verifyComplete(); + } + @Test void shouldProduceApplicationGraphQl() throws Exception { MockServerHttpRequest httpRequest = MockServerHttpRequest.post("/") diff --git a/spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlHttpHandlerTests.java b/spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlHttpHandlerTests.java index 76e05957..d9792f41 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlHttpHandlerTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlHttpHandlerTests.java @@ -80,6 +80,15 @@ public class GraphQlHttpHandlerTests { assertThat(response.getContentAsString()).isEqualTo("{\"data\":{\"greeting\":\"Hello\"}}"); } + @Test + void shouldSupportApplicationGraphQlWithCharset() throws Exception { + MockHttpServletRequest request = createServletRequest("{ greeting }", "*/*"); + request.setContentType("application/graphql;charset=UTF-8"); + + MockHttpServletResponse response = handleRequest(request, this.greetingHandler); + assertThat(response.getContentAsString()).isEqualTo("{\"data\":{\"greeting\":\"Hello\"}}"); + } + @Test void shouldProduceApplicationGraphQl() throws Exception { MockHttpServletRequest request = createServletRequest("{ greeting }", MediaType.APPLICATION_GRAPHQL_RESPONSE_VALUE);