Skip to content

Commit

Permalink
Make sure quarkus.http.filter headers
Browse files Browse the repository at this point in the history
The previous behavior would completely replace
existing response headers

Fixes: quarkusio#38155
  • Loading branch information
geoand committed Jan 15, 2024
1 parent e3dd81b commit ef2fb18
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -122,9 +125,10 @@ public static void applyFilters(Map<String, FilterConfig> filtersInConfig, Route
.handler(new Handler<RoutingContext>() {
@Override
public void handle(RoutingContext event) {
event.response().headers().setAll(headers);
addFilterHeaders(event, headers);
event.next();
}

});
} else {
for (var method : methods.get()) {
Expand All @@ -133,7 +137,7 @@ public void handle(RoutingContext event) {
.handler(new Handler<RoutingContext>() {
@Override
public void handle(RoutingContext event) {
event.response().headers().setAll(headers);
addFilterHeaders(event, headers);
event.next();
}
});
Expand All @@ -143,6 +147,27 @@ public void handle(RoutingContext event) {
}
}

private static void addFilterHeaders(RoutingContext event, Map<String, String> headers) {
for (var entry : headers.entrySet()) {
String key = entry.getKey();
String value = entry.getValue();
MultiMap responseHeaders = event.response().headers();
List<String> 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<String>(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<String, HeaderConfig> headers, Router httpRouteRouter) {
if (!headers.isEmpty()) {
// Creates a handler for each header entry
Expand Down
5 changes: 5 additions & 0 deletions integration-tests/vertx-http/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@
<artifactId>smallrye-mutiny-vertx-web-client</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>

<!-- Minimal test dependencies to *-deployment artifacts for consistent build order -->
<dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -22,6 +26,25 @@ void testAnyPathAdditionalHeadersGet() {

}

@Test
void testCorsRequest() {
List<Header> 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
Expand Down

0 comments on commit ef2fb18

Please sign in to comment.