Spring Cannot remove headers from httpHeaders.keySet() in WebFlux

Affects: v5.3.2

I have ExchangeFilterFunction which should retain only allowed response headers:

private static final Collection<String> ALLOWED_RESPONSE_HEADERS = Arrays.asList(HttpHeaders.CONTENT_TYPE, HttpHeaders.CONTENT_DISPOSITION);

private ExchangeFilterFunction filterResponseHeaders() {
    return ExchangeFilterFunction.ofResponseProcessor(clientResponse -> Mono.just(ClientResponse.from(clientResponse)
            .headers(httpHeaders -> httpHeaders.keySet().removeIf(headerName -> !ALLOWED_RESPONSE_HEADERS.contains(headerName)))
            .body(clientResponse.bodyToFlux(DataBuffer.class))
            .build()));
}

This one is working correctly.

Because of ClientResponse.from() deprecation, the method is rewritten to:

private ExchangeFilterFunction filterResponseHeaders() {
    return ExchangeFilterFunction.ofResponseProcessor(clientResponse -> Mono.just(clientResponse.mutate()
            .headers(httpHeaders -> httpHeaders.keySet().removeIf(headerName -> !ALLOWED_RESPONSE_HEADERS.contains(headerName)))
            .build()));
}

That one is not working. Response headers are not affected.

Comment From: rstoyanchev

I've tried to reproduce this but couldn't as follows:

@Test
public void mutateCanRemoveHeaders() {
    MockClientHttpResponse httpResponse = new MockClientHttpResponse(HttpStatus.OK);
    httpResponse.getHeaders().add("foo", "bar");
    httpResponse.getHeaders().add("bar", "baz");

    DefaultClientResponse otherResponse = new DefaultClientResponse(
            httpResponse, ExchangeStrategies.withDefaults(), "", "",
            () -> new MockClientHttpRequest(HttpMethod.GET, "/path"));

    ClientResponse result = otherResponse.mutate()
            .statusCode(HttpStatus.BAD_REQUEST)
            .headers(headers -> headers.remove("foo"))
            .build();

    assertThat(result.headers().asHttpHeaders().size()).isEqualTo(1);
    assertThat(result.headers().asHttpHeaders().getFirst("bar")).isEqualTo("baz");
}

Comment From: zeljko-mirovic

I added two tests matching scenario in my application. RemoveHeaderClientResponseFromIT.java.txt RemoveHeaderClientResponseMutateIT.java.txt

Comment From: bclozel

I think this issue is in fact in NettyHeadersAdapter. Its keySet() method is using io.netty.handler.codec.http.HttpHeaders.names(), which claims:

Note that modifying the returned Set will not affect the state of this object.

Comment From: zeljko-mirovic

If original keySet() can not be modified, we should always use a copy. Changing DefaultClientResponseBuilder(ClientResponse, boolean) constructor like this is solving the issue:

DefaultClientResponseBuilder(ClientResponse other, boolean mutate) {
    Assert.notNull(other, "ClientResponse must not be null");
    this.strategies = other.strategies();
    this.statusCode = other.rawStatusCode();
    this.headers = new HttpHeaders();
    this.headers.addAll(other.headers().asHttpHeaders());
    if (mutate) {
        this.body = other.bodyToFlux(DataBuffer.class);
    }
    this.originalResponse = other;
    this.request = (other instanceof DefaultClientResponse ?
            ((DefaultClientResponse) other).request() : EMPTY_REQUEST);
}