Skip to content

Skip rate-limit header mutation on committed response#4192

Open
seonwooj0810 wants to merge 1 commit into
spring-cloud:mainfrom
seonwooj0810:fix/issue-4175-ratelimiter-committed-headers
Open

Skip rate-limit header mutation on committed response#4192
seonwooj0810 wants to merge 1 commit into
spring-cloud:mainfrom
seonwooj0810:fix/issue-4175-ratelimiter-committed-headers

Conversation

@seonwooj0810
Copy link
Copy Markdown

Fixes gh-4175

Problem

RequestRateLimiterGatewayFilterFactory copies the rate-limiter response headers onto the exchange response via exchange.getResponse().getHeaders().add(...) unconditionally:

return limiter.isAllowed(routeId, key).flatMap(response -> {
    for (Map.Entry<String, String> header : response.getHeaders().entrySet()) {
        exchange.getResponse().getHeaders().add(header.getKey(), header.getValue());
    }
    ...

When the filter chain is re-subscribed with the same ServerWebExchange — most notably when RetryGatewayFilterFactory is placed before the rate limiter and retries on series=CLIENT_ERROR — the response from the first (denied) pass is already committed. getHeaders() then returns a ReadOnlyHttpHeaders instance, and add(...) fails with UnsupportedOperationException.

Change

Guard the header mutation with ServerHttpResponse.isCommitted() so headers are only added while the response can still be modified. This is a minimal, compatibility-preserving fix: the default complete-signal behavior on a denied request is unchanged, and the existing throwOnLimit option keeps working. (Approach suggested by @dongyikuan919 in the issue thread.)

Test evidence

Added deniedDoesNotMutateHeadersWhenResponseAlreadyCommitted to RequestRateLimiterGatewayFilterFactoryTests, which commits the response before invoking the filter and asserts it completes instead of erroring.

  • Without the fix the new test fails with onError(java.lang.UnsupportedOperationException).
  • With the fix the full test class passes: Tests run: 7, Failures: 0, Errors: 0, Skipped: 0.

Built and tested with JDK 21 against spring-cloud-gateway-server-webflux; spring-javaformat:apply reports no formatting changes.

Verification done: (1) no in-flight PR for #4175 (only the previously merged #4072 that added throwOnLimit); (2) issue has no self-claim; (3) fix is in .java files; (4) confirmed the unguarded getHeaders().add() still present on current main; (5) commit signed off for DCO.

RequestRateLimiterGatewayFilterFactory adds the rate-limiter response
headers via getHeaders().add(). When the filter chain is re-subscribed
with the same ServerWebExchange (for example by the retry filter after a
denied request), the response is already committed and getHeaders()
returns ReadOnlyHttpHeaders, so the add() call fails with
UnsupportedOperationException.

Guard the header mutation with ServerHttpResponse.isCommitted() so the
filter no longer throws when re-applied to a committed response. The
default complete-signal behavior is unchanged.

Fixes spring-cloudgh-4175

Signed-off-by: seonwoo_jung <79202163+seonwooj0810@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RequestRateLimiterGatewayFilterFactory commits 429 response instead of throwing

2 participants