From ef2fb1807b854f450ad468ae7d01ae20c529c298 Mon Sep 17 00:00:00 2001 From: Georgios Andrianakis Date: Mon, 15 Jan 2024 10:14:13 +0200 Subject: [PATCH] Make sure quarkus.http.filter headers The previous behavior would completely replace existing response headers Fixes: #38155 --- .../options/HttpServerCommonHandlers.java | 29 +++++++++++++++++-- integration-tests/vertx-http/pom.xml | 5 ++++ .../src/main/resources/application.properties | 9 ++++++ .../io/quarkus/it/vertx/FilterTestCase.java | 23 +++++++++++++++ 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/options/HttpServerCommonHandlers.java b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/options/HttpServerCommonHandlers.java index 820e6390b9b4a..d14da39ac12cf 100644 --- a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/options/HttpServerCommonHandlers.java +++ b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/options/HttpServerCommonHandlers.java @@ -3,6 +3,8 @@ import static io.quarkus.vertx.core.runtime.context.VertxContextSafetyToggle.setCurrentContextSafe; import static io.quarkus.vertx.http.runtime.TrustedProxyCheck.allowAll; +import java.util.LinkedHashSet; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.function.Supplier; @@ -23,6 +25,7 @@ import io.smallrye.common.vertx.VertxContext; import io.vertx.core.Context; import io.vertx.core.Handler; +import io.vertx.core.MultiMap; import io.vertx.core.Vertx; import io.vertx.core.http.HttpMethod; import io.vertx.core.http.HttpServerRequest; @@ -122,9 +125,10 @@ public static void applyFilters(Map filtersInConfig, Route .handler(new Handler() { @Override public void handle(RoutingContext event) { - event.response().headers().setAll(headers); + addFilterHeaders(event, headers); event.next(); } + }); } else { for (var method : methods.get()) { @@ -133,7 +137,7 @@ public void handle(RoutingContext event) { .handler(new Handler() { @Override public void handle(RoutingContext event) { - event.response().headers().setAll(headers); + addFilterHeaders(event, headers); event.next(); } }); @@ -143,6 +147,27 @@ public void handle(RoutingContext event) { } } + private static void addFilterHeaders(RoutingContext event, Map headers) { + for (var entry : headers.entrySet()) { + String key = entry.getKey(); + String value = entry.getValue(); + MultiMap responseHeaders = event.response().headers(); + List oldValues = responseHeaders.getAll(key); + if (oldValues.isEmpty()) { + responseHeaders.set(key, value); + } else { + // we need to make sure the new value is not duplicated + var newValues = new LinkedHashSet(oldValues); + boolean added = newValues.add(value); + if (added) { + responseHeaders.set(key, newValues); + } else { + // we don't need to do anything here as the value was already in the set + } + } + } + } + public static void applyHeaders(Map headers, Router httpRouteRouter) { if (!headers.isEmpty()) { // Creates a handler for each header entry diff --git a/integration-tests/vertx-http/pom.xml b/integration-tests/vertx-http/pom.xml index ba3cd9d408d75..c78c8565389ee 100644 --- a/integration-tests/vertx-http/pom.xml +++ b/integration-tests/vertx-http/pom.xml @@ -52,6 +52,11 @@ smallrye-mutiny-vertx-web-client test + + org.assertj + assertj-core + test + diff --git a/integration-tests/vertx-http/src/main/resources/application.properties b/integration-tests/vertx-http/src/main/resources/application.properties index 325197e0c00f7..f8f5ad7a1913b 100644 --- a/integration-tests/vertx-http/src/main/resources/application.properties +++ b/integration-tests/vertx-http/src/main/resources/application.properties @@ -29,6 +29,15 @@ quarkus.http.filter.cached.header."Cache-Control"=max-age=31536000 quarkus.http.filter.cached.matches=/filter/(an.*|override) quarkus.http.filter.cached.methods=GET +# See io.quarkus.it.vertx.FilterTestCase.testCorsRequest +quarkus.http.filter.cors.header."Cache-Control"=max-age=31536000 +quarkus.http.filter.cors.header."Access-Control-Allow-Origin"=https://example.org/ +quarkus.http.filter.cors.header."Access-Control-Allow-Methods"=TEST +quarkus.http.filter.cors.matches=/filter/any +quarkus.http.filter.cors.methods=GET +# we want this filter to run after cors, so it can have the chance to see the values the cors filter set +quarkus.http.filter.cors.order=400 + # See io.quarkus.it.vertx.FilterTestCase.testPathOrder quarkus.http.filter.just-order.order=10 quarkus.http.filter.just-order.header."Cache-Control"=max-age=5000 diff --git a/integration-tests/vertx-http/src/test/java/io/quarkus/it/vertx/FilterTestCase.java b/integration-tests/vertx-http/src/test/java/io/quarkus/it/vertx/FilterTestCase.java index fba19b9423d7d..1e017af2cb47f 100644 --- a/integration-tests/vertx-http/src/test/java/io/quarkus/it/vertx/FilterTestCase.java +++ b/integration-tests/vertx-http/src/test/java/io/quarkus/it/vertx/FilterTestCase.java @@ -4,9 +4,13 @@ import static org.hamcrest.Matchers.emptyOrNullString; import static org.hamcrest.Matchers.is; +import java.util.List; + +import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; import io.quarkus.test.junit.QuarkusTest; +import io.restassured.http.Header; @QuarkusTest public class FilterTestCase { @@ -22,6 +26,25 @@ void testAnyPathAdditionalHeadersGet() { } + @Test + void testCorsRequest() { + List
corsMethods = given() + .header("Origin", "https://example.org/") + .header("Access-Control-Request-Method", "GET") + .get("/filter/any") + .then() + .statusCode(200) + .header("Cache-Control", is("max-age=31536000")) + .header("Access-Control-Allow-Origin", + is("https://example.org/")) // this same header was added by cors but also by a property, we should only have one value + .body(is("ok")) + .extract() + .headers().getList("Access-Control-Allow-Methods"); + Assertions.assertThat(corsMethods.stream().map(Header::getValue)).containsExactly("POST,GET,PUT,OPTIONS,DELETE", + "TEST"); + + } + @Test void testAnyPathAdditionalHeadersHead() { // HEAD requests should not include the header