From 3f1a59968281a3d354635318d36e967ced22bfee Mon Sep 17 00:00:00 2001 From: Georgios Andrianakis Date: Fri, 26 Nov 2021 20:21:31 +0200 Subject: [PATCH] Take multiple Accept headers case into account in RESTEasy Reactive Fixes: #21559 --- .../server/handlers/ClassRoutingHandler.java | 130 ++++++++++-------- .../server/handlers/FixedProducesHandler.java | 34 +++-- .../resteasy-reactive/server/vertx/pom.xml | 5 + .../framework/ResteasyReactiveUnitTest.java | 16 ++- .../simple/MultipleAcceptHeadersTest.java | 59 ++++++++ 5 files changed, 168 insertions(+), 76 deletions(-) create mode 100644 independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/simple/MultipleAcceptHeadersTest.java diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/ClassRoutingHandler.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/ClassRoutingHandler.java index 610f8c14960f5..05296f9e0cbbc 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/ClassRoutingHandler.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/ClassRoutingHandler.java @@ -124,66 +124,21 @@ public void handle(ResteasyReactiveRequestContext requestContext) throws Excepti } // according to the spec we need to return HTTP 406 when Accept header doesn't match what is specified in @Produces if (target.value.getProduces() != null) { - String accepts = serverRequest.getRequestHeader(HttpHeaders.ACCEPT); - if ((accepts != null) && !accepts.equals(MediaType.WILDCARD)) { - int commaIndex = accepts.indexOf(','); - boolean multipleAcceptsValues = commaIndex >= 0; - MediaType[] producesMediaTypes = target.value.getProduces().getSortedOriginalMediaTypes(); - if (!multipleAcceptsValues && (producesMediaTypes.length == 1)) { - // the point of this branch is to eliminate any list creation or string indexing as none is needed - MediaType acceptsMediaType = MediaType.valueOf(accepts.trim()); - MediaType providedMediaType = producesMediaTypes[0]; - if (!providedMediaType.isCompatible(acceptsMediaType)) { - throw new NotAcceptableException(INVALID_ACCEPT_HEADER_MESSAGE); - } - } else if (multipleAcceptsValues && (producesMediaTypes.length == 1)) { - // this is fairly common case, so we want it to be as fast as possible - // we do that by manually splitting the accepts header and immediately checking - // if the value is compatible with the produces media type - boolean compatible = false; - int begin = 0; - - do { - String acceptPart; - if (commaIndex == -1) { // this is the case where we are checking the remainder of the string - acceptPart = accepts.substring(begin); - } else { - acceptPart = accepts.substring(begin, commaIndex); - } - if (producesMediaTypes[0].isCompatible(toMediaType(acceptPart.trim()))) { - compatible = true; - break; - } else if (commaIndex == -1) { // we have reached the end and not found any compatible media types - break; - } - begin = commaIndex + 1; // the next part will start at the character after the comma - if (begin >= (accepts.length() - 1)) { // if we have reached this point, then are no compatible media types - break; - } - commaIndex = accepts.indexOf(',', begin); - } while (true); - - if (!compatible) { - throw new NotAcceptableException(INVALID_ACCEPT_HEADER_MESSAGE); - } - } else { - // don't use any of the JAX-RS stuff from the various MediaType helper as we want to be as performant as possible - List acceptsMediaTypes; - if (accepts.contains(",")) { - String[] parts = accepts.split(","); - acceptsMediaTypes = new ArrayList<>(parts.length); - for (int i = 0; i < parts.length; i++) { - String part = parts[i]; - acceptsMediaTypes.add(toMediaType(part.trim())); - } - } else { - acceptsMediaTypes = Collections.singletonList(toMediaType(accepts)); - } - if (MediaTypeHelper.getFirstMatch(Arrays.asList(producesMediaTypes), - acceptsMediaTypes) == null) { - throw new NotAcceptableException(INVALID_ACCEPT_HEADER_MESSAGE); + // there could potentially be multiple Accept headers and we need to response with 406 + // if none match the method's @Produces + List accepts = serverRequest.getAllRequestHeaders(HttpHeaders.ACCEPT); + if (!accepts.isEmpty()) { + boolean hasAtLeastOneMatch = false; + for (int i = 0; i < accepts.size(); i++) { + boolean matches = acceptHeaderMatches(target, accepts.get(i)); + if (matches) { + hasAtLeastOneMatch = true; + break; } } + if (!hasAtLeastOneMatch) { + throw new NotAcceptableException(INVALID_ACCEPT_HEADER_MESSAGE); + } } } @@ -198,6 +153,65 @@ public void handle(ResteasyReactiveRequestContext requestContext) throws Excepti } } + private boolean acceptHeaderMatches(RequestMapper.RequestMatch target, String accepts) { + if ((accepts != null) && !accepts.equals(MediaType.WILDCARD)) { + int commaIndex = accepts.indexOf(','); + boolean multipleAcceptsValues = commaIndex >= 0; + MediaType[] producesMediaTypes = target.value.getProduces().getSortedOriginalMediaTypes(); + if (!multipleAcceptsValues && (producesMediaTypes.length == 1)) { + // the point of this branch is to eliminate any list creation or string indexing as none is needed + MediaType acceptsMediaType = MediaType.valueOf(accepts.trim()); + MediaType providedMediaType = producesMediaTypes[0]; + return providedMediaType.isCompatible(acceptsMediaType); + } else if (multipleAcceptsValues && (producesMediaTypes.length == 1)) { + // this is fairly common case, so we want it to be as fast as possible + // we do that by manually splitting the accepts header and immediately checking + // if the value is compatible with the produces media type + boolean compatible = false; + int begin = 0; + + do { + String acceptPart; + if (commaIndex == -1) { // this is the case where we are checking the remainder of the string + acceptPart = accepts.substring(begin); + } else { + acceptPart = accepts.substring(begin, commaIndex); + } + if (producesMediaTypes[0].isCompatible(toMediaType(acceptPart.trim()))) { + compatible = true; + break; + } else if (commaIndex == -1) { // we have reached the end and not found any compatible media types + break; + } + begin = commaIndex + 1; // the next part will start at the character after the comma + if (begin >= (accepts.length() - 1)) { // if we have reached this point, then are no compatible media types + break; + } + commaIndex = accepts.indexOf(',', begin); + } while (true); + + return compatible; + } else { + // don't use any of the JAX-RS stuff from the various MediaType helper as we want to be as performant as possible + List acceptsMediaTypes; + if (accepts.contains(",")) { + String[] parts = accepts.split(","); + acceptsMediaTypes = new ArrayList<>(parts.length); + for (int i = 0; i < parts.length; i++) { + String part = parts[i]; + acceptsMediaTypes.add(toMediaType(part.trim())); + } + } else { + acceptsMediaTypes = Collections.singletonList(toMediaType(accepts)); + } + return MediaTypeHelper.getFirstMatch(Arrays.asList(producesMediaTypes), + acceptsMediaTypes) != null; + } + } + + return true; + } + private MediaType toMediaType(String mediaTypeStr) { return MediaTypeHeaderDelegate.parse(mediaTypeStr); } diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/FixedProducesHandler.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/FixedProducesHandler.java index 579550e43eefe..0626066d3c823 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/FixedProducesHandler.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/FixedProducesHandler.java @@ -1,5 +1,6 @@ package org.jboss.resteasy.reactive.server.handlers; +import java.util.List; import java.util.Locale; import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.HttpHeaders; @@ -31,26 +32,35 @@ public FixedProducesHandler(MediaType mediaType, EntityWriter writer) { @Override public void handle(ResteasyReactiveRequestContext requestContext) throws Exception { - String accept = requestContext.serverRequest().getRequestHeader(HttpHeaders.ACCEPT); - if (accept == null) { + List acceptValues = requestContext.serverRequest().getAllRequestHeaders(HttpHeaders.ACCEPT); + if (acceptValues.isEmpty()) { requestContext.setResponseContentType(mediaType); requestContext.setEntityWriter(writer); } else { - //TODO: this needs to be optimised - if (accept.contains(mediaTypeString) || accept.contains("*/*") || accept.contains(mediaTypeSubstring)) { - requestContext.setResponseContentType(mediaType); - requestContext.setEntityWriter(writer); - } else { - // some clients might be sending the header with incorrect casing... - String lowercaseAccept = accept.toLowerCase(Locale.ROOT); - if (lowercaseAccept.contains(mediaTypeString) || lowercaseAccept.contains(mediaTypeSubstring)) { + boolean handled = false; + for (int i = 0; i < acceptValues.size(); i++) { + String accept = acceptValues.get(i); + //TODO: this needs to be optimised + if (accept.contains(mediaTypeString) || accept.contains("*/*") || accept.contains(mediaTypeSubstring)) { requestContext.setResponseContentType(mediaType); requestContext.setEntityWriter(writer); + handled = true; + break; } else { - throw new WebApplicationException( - Response.notAcceptable(Variant.mediaTypes(mediaType.getMediaType()).build()).build()); + // some clients might be sending the header with incorrect casing... + String lowercaseAccept = accept.toLowerCase(Locale.ROOT); + if (lowercaseAccept.contains(mediaTypeString) || lowercaseAccept.contains(mediaTypeSubstring)) { + requestContext.setResponseContentType(mediaType); + requestContext.setEntityWriter(writer); + handled = true; + break; + } } } + if (!handled) { + throw new WebApplicationException( + Response.notAcceptable(Variant.mediaTypes(mediaType.getMediaType()).build()).build()); + } } } } diff --git a/independent-projects/resteasy-reactive/server/vertx/pom.xml b/independent-projects/resteasy-reactive/server/vertx/pom.xml index 6b6c920a956fd..d3842eb70ba07 100644 --- a/independent-projects/resteasy-reactive/server/vertx/pom.xml +++ b/independent-projects/resteasy-reactive/server/vertx/pom.xml @@ -74,6 +74,11 @@ jakarta.validation-api test + + io.vertx + vertx-web-client + test + org.jboss.logging diff --git a/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/framework/ResteasyReactiveUnitTest.java b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/framework/ResteasyReactiveUnitTest.java index 28578620d13a9..c066b32050c76 100644 --- a/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/framework/ResteasyReactiveUnitTest.java +++ b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/framework/ResteasyReactiveUnitTest.java @@ -60,11 +60,14 @@ public class ResteasyReactiveUnitTest implements BeforeAllCallback, AfterAllCallback { + public static final int SERVER_PORT = 8080; + public static final DotName HTTP_SERVER_REQUEST = DotName.createSimple(HttpServerRequest.class.getName()); public static final DotName HTTP_SERVER_RESPONSE = DotName.createSimple(HttpServerResponse.class.getName()); public static final DotName ROUTING_CONTEXT = DotName.createSimple(RoutingContext.class.getName()); private static final Logger rootLogger; public static final String EXECUTOR_THREAD_NAME = "blocking executor thread"; + private Handler[] originalHandlers; static { @@ -224,12 +227,13 @@ public void init(Vertx vertx, Context context) { @Override public void start(Promise startPromise) throws Exception { server = vertx.createHttpServer(); - server.requestHandler(router).listen(8080).onComplete(new io.vertx.core.Handler>() { - @Override - public void handle(AsyncResult event) { - startPromise.complete(); - } - }); + server.requestHandler(router).listen(SERVER_PORT) + .onComplete(new io.vertx.core.Handler>() { + @Override + public void handle(AsyncResult event) { + startPromise.complete(); + } + }); } @Override diff --git a/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/simple/MultipleAcceptHeadersTest.java b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/simple/MultipleAcceptHeadersTest.java new file mode 100644 index 0000000000000..84ffa58a4a3d2 --- /dev/null +++ b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/simple/MultipleAcceptHeadersTest.java @@ -0,0 +1,59 @@ +package org.jboss.resteasy.reactive.server.vertx.test.simple; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.vertx.core.Vertx; +import io.vertx.ext.web.client.WebClient; +import java.util.List; +import java.util.concurrent.TimeUnit; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import org.jboss.resteasy.reactive.server.vertx.test.framework.ResteasyReactiveUnitTest; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +public class MultipleAcceptHeadersTest { + + private static final String BODY = "{\"message\": \"hello world\"}"; + + @RegisterExtension + static ResteasyReactiveUnitTest test = new ResteasyReactiveUnitTest() + .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) + .addClasses(HelloResource.class)); + + @Test + public void matchingHeaderIsFirst() throws Exception { + // the WebClient is used because RestAssured can't seem to send the 'Accept' header multiple times... + WebClient client = WebClient.create(Vertx.vertx()); + + var response = client.get(ResteasyReactiveUnitTest.SERVER_PORT, "localhost", "/hello") + .putHeader("Accept", List.of("application/xml", "application/json")).send().toCompletionStage() + .toCompletableFuture().get(10, TimeUnit.SECONDS); + assertThat(response.statusCode()).isEqualTo(200); + assertThat(response.bodyAsString()).isEqualTo(BODY); + } + + @Test + public void matchingHeaderIsLast() throws Exception { + WebClient client = WebClient.create(Vertx.vertx()); + + var response = client.get(ResteasyReactiveUnitTest.SERVER_PORT, "localhost", "/hello") + .putHeader("Accept", List.of("application/json", "application/xml")).send().toCompletionStage() + .toCompletableFuture().get(10, TimeUnit.SECONDS); + assertThat(response.statusCode()).isEqualTo(200); + assertThat(response.bodyAsString()).isEqualTo(BODY); + } + + @Path("/hello") + public static class HelloResource { + + @GET + @Produces("application/json") + public String hello() { + return BODY; + } + } +}