Document request header behavior of ModifyRequestBody

docs/modules/ROOT/pages/spring-cloud-gateway-server-webflux/gatewayfilter-factories/modifyrequestbody-factory.adoc

@@ -40,5 +40,38 @@ static class Hello {
 
 NOTE: If the request has no body, the `RewriteFilter` is passed `null`. `Mono.empty()` should be returned to assign a missing body in the request.
 
+[[modifyrequestbody-gatewayfilter-factory-headers]]
+== Modifying Request Headers
 
+This filter replaces the downstream request with a decorator whose headers are a *copy* of the request headers.
+The copy is taken before the `RewriteFunction` runs, and the decorator returns a new `HttpHeaders` instance on every call to `getHeaders()`.
+Two consequences follow.
+
+First, the `RewriteFunction` cannot change the request.
+Calling `exchange.mutate()` inside the rewrite function returns a new `ServerWebExchange` that this filter does not use, so headers set there are not sent downstream.
+Use a separate filter to change headers.
+
+Second, mutating the `HttpHeaders` returned by `exchange.getRequest().getHeaders()` in a filter placed after `ModifyRequestBody` has no effect, because the change is written to a copy that is then discarded.
+Unlike the underlying request, whose headers are read-only, this does not throw `UnsupportedOperationException`.
+
+To change headers after `ModifyRequestBody`, mutate the request:
+
+[source,java]
+----
+ServerHttpRequest request = exchange.getRequest()
+    .mutate()
+    .header("X-Example", "value")
+    .build();
+
+return chain.filter(exchange.mutate().request(request).build());
+----
+
+The following form is equivalent:
+
+[source,java]
+----
+return chain.filter(exchange.mutate().request(r -> r.header("X-Example", "value")).build());
+----
+
+Filter factories such as xref:spring-cloud-gateway-server-webflux/gatewayfilter-factories/addrequestheader-factory.adoc[`AddRequestHeader`] already mutate the request this way, so ordering them after `ModifyRequestBody` works as expected.
 

spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/factory/rewrite/ModifyRequestBodyGatewayFilterFactoryUnitTests.java

@@ -16,14 +16,29 @@
 
 package org.springframework.cloud.gateway.filter.factory.rewrite;
 
+import java.nio.charset.StandardCharsets;
+import java.util.concurrent.atomic.AtomicReference;
+
 import org.junit.jupiter.api.Test;
+import reactor.core.publisher.Mono;
 
 import org.springframework.cloud.gateway.filter.GatewayFilter;
+import org.springframework.cloud.gateway.filter.GatewayFilterChain;
 import org.springframework.cloud.gateway.filter.factory.rewrite.ModifyRequestBodyGatewayFilterFactory.Config;
 import org.springframework.core.ParameterizedTypeReference;
+import org.springframework.core.io.buffer.DataBufferUtils;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.MediaType;
+import org.springframework.http.server.reactive.ServerHttpRequest;
+import org.springframework.mock.http.server.reactive.MockServerHttpRequest;
+import org.springframework.mock.web.server.MockServerWebExchange;
+import org.springframework.web.server.ServerWebExchange;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+/**
+ * @author hyungzin0309
+ */
 public class ModifyRequestBodyGatewayFilterFactoryUnitTests {
 
 	@Test
@@ -48,4 +63,100 @@ public void toStringFormatWithParameterizedTypeReferences() {
 		assertThat(filter.toString()).contains("String").contains("Integer").contains("mycontenttype");
 	}
 
+	@Test
+	public void headersAddedByMutatingTheRequestAreVisibleDownstream() {
+		AtomicReference<HttpHeaders> downstreamHeaders = new AtomicReference<>();
+		AtomicReference<String> downstreamBody = new AtomicReference<>();
+		GatewayFilterChain chain = exchange -> {
+			ServerHttpRequest request = exchange.getRequest().mutate().header("X-Added", "added").build();
+			ServerWebExchange mutated = exchange.mutate().request(request).build();
+			downstreamHeaders.set(mutated.getRequest().getHeaders());
+			return readBody(mutated, downstreamBody);
+		};
+
+		modifyBodyFilter().filter(postExchange(), chain).block();
+
+		assertThat(downstreamHeaders.get().getFirst("X-Added")).isEqualTo("added");
+		assertThat(downstreamHeaders.get().getFirst("X-Original")).isEqualTo("original");
+		// the rewritten body must survive the mutation
+		assertThat(downstreamBody.get()).isEqualTo("REWRITTEN");
+	}
+
+	@Test
+	public void headersAddedByMutatingTheExchangeAreVisibleDownstream() {
+		AtomicReference<HttpHeaders> downstreamHeaders = new AtomicReference<>();
+		AtomicReference<String> downstreamBody = new AtomicReference<>();
+		GatewayFilterChain chain = exchange -> {
+			ServerWebExchange mutated = exchange.mutate().request(r -> r.header("X-Added", "added")).build();
+			downstreamHeaders.set(mutated.getRequest().getHeaders());
+			return readBody(mutated, downstreamBody);
+		};
+
+		modifyBodyFilter().filter(postExchange(), chain).block();
+
+		assertThat(downstreamHeaders.get().getFirst("X-Added")).isEqualTo("added");
+		assertThat(downstreamBody.get()).isEqualTo("REWRITTEN");
+	}
+
+	@Test
+	public void headersAreCopiedSoInPlaceChangesAreNotVisibleDownstream() {
+		AtomicReference<HttpHeaders> downstreamHeaders = new AtomicReference<>();
+		AtomicReference<String> downstreamBody = new AtomicReference<>();
+		GatewayFilterChain chain = exchange -> {
+			// the decorator returns a fresh copy on every call, so this write is lost
+			exchange.getRequest().getHeaders().set("X-InPlace", "ignored");
+			downstreamHeaders.set(exchange.getRequest().getHeaders());
+			return readBody(exchange, downstreamBody);
+		};
+
+		modifyBodyFilter().filter(postExchange(), chain).block();
+
+		assertThat(downstreamBody.get()).isEqualTo("REWRITTEN");
+		assertThat(downstreamHeaders.get().containsHeader("X-InPlace")).isFalse();
+	}
+
+	@Test
+	public void headersSetByTheRewriteFunctionAreNotVisibleDownstream() {
+		Config config = new Config();
+		config.setContentType(MediaType.TEXT_PLAIN_VALUE);
+		config.setRewriteFunction(String.class, String.class, (exchange, body) -> {
+			// the exchange returned here is discarded by the filter
+			exchange.mutate().request(r -> r.header("X-Rewrite", "ignored")).build();
+			return Mono.just("REWRITTEN");
+		});
+
+		AtomicReference<HttpHeaders> downstreamHeaders = new AtomicReference<>();
+		AtomicReference<String> downstreamBody = new AtomicReference<>();
+		GatewayFilterChain chain = exchange -> {
+			downstreamHeaders.set(exchange.getRequest().getHeaders());
+			return readBody(exchange, downstreamBody);
+		};
+
+		new ModifyRequestBodyGatewayFilterFactory().apply(config).filter(postExchange(), chain).block();
+
+		assertThat(downstreamBody.get()).isEqualTo("REWRITTEN");
+		assertThat(downstreamHeaders.get().containsHeader("X-Rewrite")).isFalse();
+	}
+
+	private GatewayFilter modifyBodyFilter() {
+		Config config = new Config();
+		config.setContentType(MediaType.TEXT_PLAIN_VALUE);
+		config.setRewriteFunction(String.class, String.class, (exchange, body) -> Mono.just("REWRITTEN"));
+		return new ModifyRequestBodyGatewayFilterFactory().apply(config);
+	}
+
+	private Mono<Void> readBody(ServerWebExchange exchange, AtomicReference<String> sink) {
+		return DataBufferUtils.join(exchange.getRequest().getBody()).doOnNext(buffer -> {
+			sink.set(buffer.toString(StandardCharsets.UTF_8));
+			DataBufferUtils.release(buffer);
+		}).then();
+	}
+
+	private MockServerWebExchange postExchange() {
+		return MockServerWebExchange.from(MockServerHttpRequest.post("http://localhost/post")
+			.header("X-Original", "original")
+			.contentType(MediaType.TEXT_PLAIN)
+			.body("body"));
+	}
+
 }