From 008a78d1c5a5a5a66535067fe4760bbdcd198d7d Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 11 Mar 2025 15:39:59 +0000 Subject: [PATCH] Handle invalid media types as client errors Closes gh-1145 --- .../webflux/AbstractGraphQlHttpHandler.java | 13 +++++++-- .../server/webflux/GraphQlHttpHandler.java | 16 +++++++++-- .../webflux/GraphQlRequestPredicates.java | 21 ++++++++++++-- .../webmvc/AbstractGraphQlHttpHandler.java | 14 ++++++++-- .../server/webmvc/GraphQlHttpHandler.java | 16 +++++++++-- .../webmvc/GraphQlRequestPredicates.java | 21 ++++++++++++-- .../GraphQlRequestPredicatesTests.java | 28 +++++++++++++++++-- .../webmvc/GraphQlRequestPredicatesTests.java | 25 +++++++++++++++-- 8 files changed, 137 insertions(+), 17 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 f0a0631d..5c562374 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 @@ -1,5 +1,5 @@ /* - * Copyright 2020-2024 the original author or authors. + * Copyright 2020-2025 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. @@ -31,6 +31,7 @@ import org.springframework.graphql.server.WebGraphQlRequest; import org.springframework.graphql.server.WebGraphQlResponse; import org.springframework.graphql.server.support.SerializableGraphQlRequest; import org.springframework.http.HttpHeaders; +import org.springframework.http.InvalidMediaTypeException; import org.springframework.http.MediaType; import org.springframework.http.codec.CodecConfigurer; import org.springframework.lang.Nullable; @@ -101,7 +102,15 @@ public abstract class AbstractGraphQlHttpHandler { private Mono readRequest(ServerRequest serverRequest) { if (this.codecDelegate != null) { - MediaType contentType = serverRequest.headers().contentType().orElse(MediaType.APPLICATION_JSON); + ServerRequest.Headers headers = serverRequest.headers(); + MediaType contentType; + try { + contentType = headers.contentType().orElse(MediaType.APPLICATION_OCTET_STREAM); + } + catch (InvalidMediaTypeException ex) { + throw new UnsupportedMediaTypeStatusException("Could not parse " + + "Content-Type [" + headers.firstHeader(HttpHeaders.CONTENT_TYPE) + "]: " + ex.getMessage()); + } return this.codecDelegate.decode(serverRequest.bodyToFlux(DataBuffer.class), contentType); } else { diff --git a/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlHttpHandler.java b/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlHttpHandler.java index a3cc3d33..b956c9cb 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlHttpHandler.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlHttpHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2024 the original author or authors. + * Copyright 2020-2025 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. @@ -22,10 +22,13 @@ import reactor.core.publisher.Mono; import org.springframework.graphql.server.WebGraphQlHandler; import org.springframework.graphql.server.WebGraphQlResponse; +import org.springframework.http.HttpHeaders; +import org.springframework.http.InvalidMediaTypeException; import org.springframework.http.MediaType; import org.springframework.http.codec.CodecConfigurer; import org.springframework.web.reactive.function.server.ServerRequest; import org.springframework.web.reactive.function.server.ServerResponse; +import org.springframework.web.server.NotAcceptableStatusException; /** * WebFlux.fn Handler for GraphQL over HTTP requests. @@ -67,7 +70,16 @@ public class GraphQlHttpHandler extends AbstractGraphQlHttpHandler { } private static MediaType selectResponseMediaType(ServerRequest serverRequest) { - for (MediaType accepted : serverRequest.headers().accept()) { + ServerRequest.Headers headers = serverRequest.headers(); + List acceptedMediaTypes; + try { + acceptedMediaTypes = headers.accept(); + } + catch (InvalidMediaTypeException ex) { + throw new NotAcceptableStatusException("Could not parse " + + "Accept header [" + headers.firstHeader(HttpHeaders.ACCEPT) + "]: " + ex.getMessage()); + } + for (MediaType accepted : acceptedMediaTypes) { if (SUPPORTED_MEDIA_TYPES.contains(accepted)) { return accepted; } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlRequestPredicates.java b/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlRequestPredicates.java index 3f80729b..0d692f9f 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlRequestPredicates.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlRequestPredicates.java @@ -24,6 +24,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; +import org.springframework.http.InvalidMediaTypeException; import org.springframework.http.MediaType; import org.springframework.http.server.PathContainer; import org.springframework.lang.Nullable; @@ -33,6 +34,8 @@ import org.springframework.web.cors.reactive.CorsUtils; import org.springframework.web.reactive.function.server.RequestPredicate; import org.springframework.web.reactive.function.server.RouterFunctions; import org.springframework.web.reactive.function.server.ServerRequest; +import org.springframework.web.server.NotAcceptableStatusException; +import org.springframework.web.server.UnsupportedMediaTypeStatusException; import org.springframework.web.util.pattern.PathPattern; import org.springframework.web.util.pattern.PathPatternParser; @@ -119,7 +122,14 @@ public final class GraphQlRequestPredicates { return true; } ServerRequest.Headers headers = request.headers(); - MediaType actual = headers.contentType().orElse(MediaType.APPLICATION_OCTET_STREAM); + MediaType actual; + try { + actual = headers.contentType().orElse(MediaType.APPLICATION_OCTET_STREAM); + } + catch (InvalidMediaTypeException ex) { + throw new UnsupportedMediaTypeStatusException("Could not parse " + + "Content-Type [" + headers.firstHeader(HttpHeaders.CONTENT_TYPE) + "]: " + ex.getMessage()); + } boolean contentTypeMatch = false; for (MediaType contentType : contentTypes) { contentTypeMatch = contentType.includes(actual); @@ -136,7 +146,14 @@ public final class GraphQlRequestPredicates { return true; } ServerRequest.Headers headers = request.headers(); - List acceptedMediaTypes = acceptedMediaTypes(headers); + List acceptedMediaTypes; + try { + acceptedMediaTypes = acceptedMediaTypes(headers); + } + catch (InvalidMediaTypeException ex) { + throw new NotAcceptableStatusException("Could not parse " + + "Accept header [" + headers.firstHeader(HttpHeaders.ACCEPT) + "]: " + ex.getMessage()); + } boolean match = false; outer: for (MediaType acceptedMediaType : acceptedMediaTypes) { 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 faba51c4..88e9c4da 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 @@ -1,5 +1,5 @@ /* - * Copyright 2020-2024 the original author or authors. + * Copyright 2020-2025 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. @@ -37,6 +37,7 @@ import org.springframework.graphql.server.WebGraphQlResponse; import org.springframework.graphql.server.support.SerializableGraphQlRequest; import org.springframework.http.HttpCookie; import org.springframework.http.HttpHeaders; +import org.springframework.http.InvalidMediaTypeException; import org.springframework.http.MediaType; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.server.ServerHttpRequest; @@ -52,6 +53,7 @@ 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.server.UnsupportedMediaTypeStatusException; import org.springframework.web.servlet.ModelAndView; import org.springframework.web.servlet.function.ServerRequest; import org.springframework.web.servlet.function.ServerResponse; @@ -146,7 +148,15 @@ public abstract class AbstractGraphQlHttpHandler { private GraphQlRequest readBody(ServerRequest request) throws ServletException { try { if (this.messageConverter != null) { - MediaType contentType = request.headers().contentType().orElse(MediaType.APPLICATION_JSON); + ServerRequest.Headers headers = request.headers(); + MediaType contentType; + try { + contentType = headers.contentType().orElse(MediaType.APPLICATION_OCTET_STREAM); + } + catch (InvalidMediaTypeException ex) { + throw new UnsupportedMediaTypeStatusException("Could not parse " + + "Content-Type [" + headers.firstHeader(HttpHeaders.CONTENT_TYPE) + "]: " + ex.getMessage()); + } if (this.messageConverter.canRead(SerializableGraphQlRequest.class, contentType)) { ServerHttpRequest httpRequest = new ServletServerHttpRequest(request.servletRequest()); return (GraphQlRequest) this.messageConverter.read(SerializableGraphQlRequest.class, httpRequest); diff --git a/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlHttpHandler.java b/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlHttpHandler.java index 80a59655..ae04ef74 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlHttpHandler.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlHttpHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2024 the original author or authors. + * Copyright 2020-2025 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. @@ -25,9 +25,12 @@ import reactor.core.publisher.Mono; import org.springframework.graphql.server.WebGraphQlHandler; import org.springframework.graphql.server.WebGraphQlResponse; +import org.springframework.http.HttpHeaders; +import org.springframework.http.InvalidMediaTypeException; import org.springframework.http.MediaType; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.lang.Nullable; +import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.servlet.function.ServerRequest; import org.springframework.web.servlet.function.ServerResponse; @@ -97,7 +100,16 @@ public class GraphQlHttpHandler extends AbstractGraphQlHttpHandler { } private static MediaType selectResponseMediaType(ServerRequest request) { - for (MediaType mediaType : request.headers().accept()) { + ServerRequest.Headers headers = request.headers(); + List acceptedMediaTypes; + try { + acceptedMediaTypes = headers.accept(); + } + catch (InvalidMediaTypeException ex) { + throw new NotAcceptableStatusException("Could not parse " + + "Accept header [" + headers.firstHeader(HttpHeaders.ACCEPT) + "]: " + ex.getMessage()); + } + for (MediaType mediaType : acceptedMediaTypes) { if (SUPPORTED_MEDIA_TYPES.contains(mediaType)) { return mediaType; } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlRequestPredicates.java b/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlRequestPredicates.java index a6667e3a..1eb1ed88 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlRequestPredicates.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlRequestPredicates.java @@ -24,12 +24,15 @@ import org.apache.commons.logging.LogFactory; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; +import org.springframework.http.InvalidMediaTypeException; import org.springframework.http.MediaType; import org.springframework.http.server.PathContainer; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.MimeTypeUtils; import org.springframework.web.cors.CorsUtils; +import org.springframework.web.server.NotAcceptableStatusException; +import org.springframework.web.server.UnsupportedMediaTypeStatusException; import org.springframework.web.servlet.function.RequestPredicate; import org.springframework.web.servlet.function.RouterFunctions; import org.springframework.web.servlet.function.ServerRequest; @@ -119,7 +122,14 @@ public final class GraphQlRequestPredicates { return true; } ServerRequest.Headers headers = request.headers(); - MediaType actual = headers.contentType().orElse(MediaType.APPLICATION_OCTET_STREAM); + MediaType actual; + try { + actual = headers.contentType().orElse(MediaType.APPLICATION_OCTET_STREAM); + } + catch (InvalidMediaTypeException ex) { + throw new UnsupportedMediaTypeStatusException("Could not parse " + + "Content-Type [" + headers.firstHeader(HttpHeaders.CONTENT_TYPE) + "]: " + ex.getMessage()); + } boolean contentTypeMatch = false; for (MediaType contentType : contentTypes) { contentTypeMatch = contentType.includes(actual); @@ -136,7 +146,14 @@ public final class GraphQlRequestPredicates { return true; } ServerRequest.Headers headers = request.headers(); - List acceptedMediaTypes = acceptedMediaTypes(headers); + List acceptedMediaTypes; + try { + acceptedMediaTypes = acceptedMediaTypes(headers); + } + catch (InvalidMediaTypeException ex) { + throw new NotAcceptableStatusException("Could not parse " + + "Accept header [" + headers.firstHeader(HttpHeaders.ACCEPT) + "]: " + ex.getMessage()); + } boolean match = false; outer: for (MediaType acceptedMediaType : acceptedMediaTypes) { diff --git a/spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlRequestPredicatesTests.java b/spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlRequestPredicatesTests.java index 1f20a1f6..01c8acb9 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlRequestPredicatesTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlRequestPredicatesTests.java @@ -18,6 +18,7 @@ package org.springframework.graphql.server.webflux; import java.util.Collections; +import java.util.List; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -30,10 +31,13 @@ import org.springframework.mock.web.server.MockServerWebExchange; import org.springframework.web.reactive.function.server.RequestPredicate; import org.springframework.web.reactive.function.server.RouterFunctions; import org.springframework.web.reactive.function.server.ServerRequest; +import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.server.UnsupportedMediaTypeStatusException; import org.springframework.web.util.pattern.PathPatternParser; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; /** * Tests for {@link GraphQlRequestPredicates}. @@ -92,23 +96,41 @@ class GraphQlRequestPredicatesTests { } @Test - void shouldRejectRequestWithDifferentContentType() { + void shouldRejectRequestWithIncompatibleContentType() { ServerWebExchange exchange = createMatchingHttpExchange() - .mutate().request(req -> req.headers(headers -> headers.setContentType(MediaType.TEXT_HTML))) + .mutate().request(request -> request.headers(h -> h.setContentType(MediaType.TEXT_HTML))) .build(); ServerRequest serverRequest = ServerRequest.create(exchange, Collections.emptyList()); assertThat(httpPredicate.test(serverRequest)).isFalse(); } + @Test + void shouldRejectRequestWithInvalidContentType() { + ServerWebExchange exchange = createMatchingHttpExchange() + .mutate().request(request -> request.headers(h -> h.set("Content-Type", "bogus"))) + .build(); + ServerRequest request = ServerRequest.create(exchange, Collections.emptyList()); + assertThatThrownBy(() -> httpPredicate.test(request)).isInstanceOf(UnsupportedMediaTypeStatusException.class); + } + @Test void shouldRejectRequestWithIncompatibleAccept() { ServerWebExchange exchange = createMatchingHttpExchange() - .mutate().request(req -> req.headers(headers -> headers.setAccept(Collections.singletonList(MediaType.TEXT_HTML)))) + .mutate().request(request -> request.headers(h -> h.setAccept(List.of(MediaType.TEXT_HTML)))) .build(); ServerRequest serverRequest = ServerRequest.create(exchange, Collections.emptyList()); assertThat(httpPredicate.test(serverRequest)).isFalse(); } + @Test + void shouldRejectRequestWithInvalidAccept() { + ServerWebExchange exchange = createMatchingHttpExchange() + .mutate().request(request -> request.headers(h -> h.set("Accept", "bogus"))) + .build(); + ServerRequest request = ServerRequest.create(exchange, Collections.emptyList()); + assertThatThrownBy(() -> httpPredicate.test(request)).isInstanceOf(NotAcceptableStatusException.class); + } + @Test void shouldSetMatchingPatternAttribute() { ServerWebExchange exchange = createMatchingHttpExchange(); diff --git a/spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlRequestPredicatesTests.java b/spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlRequestPredicatesTests.java index fb9171dd..645bdba8 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlRequestPredicatesTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlRequestPredicatesTests.java @@ -18,18 +18,22 @@ package org.springframework.graphql.server.webmvc; import java.util.Collections; +import java.util.List; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.http.HttpHeaders; import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.web.server.NotAcceptableStatusException; +import org.springframework.web.server.UnsupportedMediaTypeStatusException; import org.springframework.web.servlet.function.RequestPredicate; import org.springframework.web.servlet.function.RouterFunctions; import org.springframework.web.servlet.function.ServerRequest; import org.springframework.web.util.pattern.PathPatternParser; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; /** * Tests for {@link GraphQlRequestPredicates}. @@ -85,13 +89,21 @@ class GraphQlRequestPredicatesTests { } @Test - void shouldRejectRequestWithDifferentContentType() { + void shouldRejectRequestWithIncompatibleContentType() { MockHttpServletRequest request = createMatchingHttpRequest(); request.setContentType("text/xml"); ServerRequest serverRequest = ServerRequest.create(request, Collections.emptyList()); assertThat(httpPredicate.test(serverRequest)).isFalse(); } + @Test // gh-1145 + void shouldRejectRequestWithInvalidContentType() { + MockHttpServletRequest servletRequest = createMatchingHttpRequest(); + servletRequest.setContentType("bogus"); + ServerRequest request = ServerRequest.create(servletRequest, List.of()); + assertThatThrownBy(() -> httpPredicate.test(request)).isInstanceOf(UnsupportedMediaTypeStatusException.class); + } + @Test void shouldRejectRequestWithIncompatibleAccept() { MockHttpServletRequest request = createMatchingHttpRequest(); @@ -101,6 +113,15 @@ class GraphQlRequestPredicatesTests { assertThat(httpPredicate.test(serverRequest)).isFalse(); } + @Test + void shouldRejectRequestWithInvalidAccept() { + MockHttpServletRequest servletRequest = createMatchingHttpRequest(); + servletRequest.removeHeader("Accept"); + servletRequest.addHeader("Accept", "bogus"); + ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList()); + assertThatThrownBy(() -> httpPredicate.test(request)).isInstanceOf(NotAcceptableStatusException.class); + } + @Test void shouldSetMatchingPatternAttribute() { MockHttpServletRequest request = createMatchingHttpRequest(); @@ -168,7 +189,7 @@ class GraphQlRequestPredicatesTests { } @Test - void shouldRejectRequestWithDifferentContentType() { + void shouldRejectRequestWithIncmopatibleContentType() { MockHttpServletRequest request = createMatchingSseRequest(); request.setContentType("text/xml"); ServerRequest serverRequest = ServerRequest.create(request, Collections.emptyList());