diff --git a/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/RequestRateLimiterGatewayFilterFactory.java b/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/RequestRateLimiterGatewayFilterFactory.java index c653c64f9..a729e1498 100644 --- a/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/RequestRateLimiterGatewayFilterFactory.java +++ b/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/factory/RequestRateLimiterGatewayFilterFactory.java @@ -131,8 +131,14 @@ public GatewayFilter apply(Config config) { } return limiter.isAllowed(routeId, key).flatMap(response -> { - for (Map.Entry header : response.getHeaders().entrySet()) { - exchange.getResponse().getHeaders().add(header.getKey(), header.getValue()); + // The response may already be committed when the filter chain is + // re-subscribed with the same exchange (for example by the retry + // filter). Adding headers to a committed response would throw + // UnsupportedOperationException from ReadOnlyHttpHeaders. + if (!exchange.getResponse().isCommitted()) { + for (Map.Entry header : response.getHeaders().entrySet()) { + exchange.getResponse().getHeaders().add(header.getKey(), header.getValue()); + } } if (response.isAllowed()) { diff --git a/spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/factory/RequestRateLimiterGatewayFilterFactoryTests.java b/spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/factory/RequestRateLimiterGatewayFilterFactoryTests.java index 1df0cedb1..d5a67e6d9 100644 --- a/spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/factory/RequestRateLimiterGatewayFilterFactoryTests.java +++ b/spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/factory/RequestRateLimiterGatewayFilterFactoryTests.java @@ -105,6 +105,36 @@ public void emptyKeyDeniedWithThrowOnLimit() { assertFilterFactory(exchange -> Mono.empty(), null, true, HttpStatus.FORBIDDEN, true, true); } + @Test + public void deniedDoesNotMutateHeadersWhenResponseAlreadyCommitted() { + // gh-4175: when the filter chain is re-subscribed with the same exchange (for + // example by the retry filter), the response may already be committed. Adding the + // rate-limit headers to a committed response throws UnsupportedOperationException + // (ReadOnlyHttpHeaders), so the filter must skip the header mutation. + String key = "notallowedkey"; + Map headers = Collections.singletonMap("X-Tokens-Remaining", "0"); + when(rateLimiter.isAllowed("myroute", key)).thenReturn(Mono.just(new Response(false, headers))); + + MockServerHttpRequest request = MockServerHttpRequest.get("/").build(); + MockServerWebExchange exchange = MockServerWebExchange.from(request); + exchange.getAttributes() + .put(ServerWebExchangeUtils.GATEWAY_ROUTE_ATTR, + Route.async().id("myroute").predicate(ex -> true).uri("http://localhost").build()); + + // Simulate a previous subscription having already committed the response. + exchange.getResponse().setComplete().block(); + assertThat(exchange.getResponse().isCommitted()).isTrue(); + + RequestRateLimiterGatewayFilterFactory factory = this.context + .getBean(RequestRateLimiterGatewayFilterFactory.class); + GatewayFilter filter = factory.apply(config -> { + config.setRouteId("myroute"); + config.setKeyResolver(resolver2); + }); + + StepVerifier.create(filter.filter(exchange, this.filterChain)).expectComplete().verify(); + } + private void assertFilterFactory(KeyResolver keyResolver, String key, boolean allowed, HttpStatus expectedStatus) { assertFilterFactory(keyResolver, key, allowed, expectedStatus, null, false); }