CompositeServerHttpHeadersWriter Executes Sequentially

Fixes gh-7731

Author: rwinch

web/src/main/java/org/springframework/security/web/server/header/CompositeServerHttpHeadersWriter.java

@@ -15,15 +15,13 @@
  */
 package  org.springframework.security.web.server.header;
 
-import java.util.Arrays;
-import java.util.List;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
-
 import org.springframework.web.server.ServerWebExchange;
-
+import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 
+import java.util.Arrays;
+import java.util.List;
+
 /**
  * Combines multiple {@link ServerHttpHeadersWriter} instances into a single instance.
  *
@@ -43,8 +41,9 @@ public CompositeServerHttpHeadersWriter(List<ServerHttpHeadersWriter> writers) {
 
 	@Override
 	public Mono<Void> writeHttpHeaders(ServerWebExchange exchange) {
-		Stream<Mono<Void>> results = writers.stream().map( writer -> writer.writeHttpHeaders(exchange));
-		return Mono.when(results.collect(Collectors.toList()));
+		return Flux.fromIterable(this.writers)
+				.concatMap(w -> w.writeHttpHeaders(exchange))
+				.then();
 	}
 
 }

web/src/test/java/org/springframework/security/web/server/header/CompositeServerHttpHeadersWriterTests.java

@@ -15,11 +15,6 @@
  */
 package org.springframework.security.web.server.header;
 
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
-import java.util.Arrays;
-
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -28,10 +23,19 @@
 import org.springframework.mock.http.server.reactive.MockServerHttpRequest;
 import org.springframework.mock.web.server.MockServerWebExchange;
 import org.springframework.web.server.ServerWebExchange;
-
 import reactor.core.publisher.Mono;
 import reactor.test.StepVerifier;
 
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
 /**
  *
  * @author Rob Winch
@@ -55,7 +59,6 @@ public void setup() {
 	@Test
 	public void writeHttpHeadersWhenErrorNoErrorThenError() {
 		when(writer1.writeHttpHeaders(exchange)).thenReturn(Mono.error(new RuntimeException()));
-		when(writer2.writeHttpHeaders(exchange)).thenReturn(Mono.empty());
 
 		Mono<Void> result = writer.writeHttpHeaders(exchange);
 
@@ -64,13 +67,11 @@ public void writeHttpHeadersWhenErrorNoErrorThenError() {
 			.verify();
 
 		verify(writer1).writeHttpHeaders(exchange);
-		verify(writer2).writeHttpHeaders(exchange);
 	}
 
 	@Test
 	public void writeHttpHeadersWhenErrorErrorThenError() {
 		when(writer1.writeHttpHeaders(exchange)).thenReturn(Mono.error(new RuntimeException()));
-		when(writer2.writeHttpHeaders(exchange)).thenReturn(Mono.error(new RuntimeException()));
 
 		Mono<Void> result = writer.writeHttpHeaders(exchange);
 
@@ -79,7 +80,6 @@ public void writeHttpHeadersWhenErrorErrorThenError() {
 			.verify();
 
 		verify(writer1).writeHttpHeaders(exchange);
-		verify(writer2).writeHttpHeaders(exchange);
 	}
 
 	@Test
@@ -96,4 +96,26 @@ public void writeHttpHeadersWhenNoErrorThenNoError() {
 		verify(writer1).writeHttpHeaders(exchange);
 		verify(writer2).writeHttpHeaders(exchange);
 	}
+
+	@Test
+	public void writeHttpHeadersSequential() throws Exception {
+		AtomicBoolean slowDone = new AtomicBoolean();
+		CountDownLatch latch = new CountDownLatch(1);
+		ServerHttpHeadersWriter slow = exchange ->
+				Mono.delay(Duration.ofMillis(100))
+						.doOnSuccess(__ -> slowDone.set(true))
+						.then();
+		ServerHttpHeadersWriter second = exchange ->
+				Mono.fromRunnable(() -> {
+					latch.countDown();
+					assertThat(slowDone.get())
+							.describedAs("ServerLogoutHandler should be executed sequentially")
+							.isTrue();
+				});
+		CompositeServerHttpHeadersWriter writer = new CompositeServerHttpHeadersWriter(slow, second);
+
+		writer.writeHttpHeaders(this.exchange).block();
+
+		assertThat(latch.await(3, TimeUnit.SECONDS)).isTrue();
+	}
 }