From 44a7e89c794380671a79445547e34007ca115941 Mon Sep 17 00:00:00 2001
From: Violeta Georgieva <violeta.georgieva@broadcom.com>
Date: Wed, 6 Dec 2023 19:51:09 +0200
Subject: [PATCH] Ensure RFC 6265 compliance when sending multiple cookies
 (#2994)

According to RFC 6265

"When the user agent generates an HTTP request, the user agent MUST
NOT attach more than one Cookie header field."

https://datatracker.ietf.org/doc/html/rfc6265#section-5.4

Fixes #2983
---
 .../http/client/HttpClientOperations.java     | 12 +++++--
 .../netty/http/HttpCookieHandlingTests.java   | 36 +++++++++++++++++++
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientOperations.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientOperations.java
index 5c8b95726d..a0c8b56e74 100644
--- a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientOperations.java
+++ b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientOperations.java
@@ -20,7 +20,9 @@
 import java.net.URISyntaxException;
 import java.nio.channels.ClosedChannelException;
 import java.time.Duration;
+import java.util.ArrayList;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
@@ -106,6 +108,7 @@ class HttpClientOperations extends HttpOperations<NettyInbound, NettyOutbound>
 	final HttpHeaders            requestHeaders;
 	final ClientCookieEncoder    cookieEncoder;
 	final ClientCookieDecoder    cookieDecoder;
+	final List<Cookie>           cookieList;
 	final Sinks.One<HttpHeaders> trailerHeaders;
 
 	Supplier<String>[]          redirectedFrom = EMPTY_REDIRECTIONS;
@@ -144,6 +147,7 @@ class HttpClientOperations extends HttpOperations<NettyInbound, NettyOutbound>
 		this.requestHeaders = replaced.requestHeaders;
 		this.cookieEncoder = replaced.cookieEncoder;
 		this.cookieDecoder = replaced.cookieDecoder;
+		this.cookieList = replaced.cookieList;
 		this.resourceUrl = replaced.resourceUrl;
 		this.path = replaced.path;
 		this.responseTimeout = replaced.responseTimeout;
@@ -165,14 +169,14 @@ class HttpClientOperations extends HttpOperations<NettyInbound, NettyOutbound>
 		this.requestHeaders = nettyRequest.headers();
 		this.cookieDecoder = decoder;
 		this.cookieEncoder = encoder;
+		this.cookieList = new ArrayList<>();
 		this.trailerHeaders = Sinks.unsafe().one();
 	}
 
 	@Override
 	public HttpClientRequest addCookie(Cookie cookie) {
 		if (!hasSentHeaders()) {
-			this.requestHeaders.add(HttpHeaderNames.COOKIE,
-					cookieEncoder.encode(cookie));
+			this.cookieList.add(cookie);
 		}
 		else {
 			throw new IllegalStateException("Status and headers already sent");
@@ -587,6 +591,10 @@ protected void afterMarkSentHeaders() {
 
 	@Override
 	protected void beforeMarkSentHeaders() {
+		if (!cookieList.isEmpty()) {
+			requestHeaders.add(HttpHeaderNames.COOKIE, cookieEncoder.encode(cookieList));
+		}
+
 		if (redirectedFrom.length > 0) {
 			if (redirectRequestConsumer != null) {
 				redirectRequestConsumer.accept(this);
diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/HttpCookieHandlingTests.java b/reactor-netty-http/src/test/java/reactor/netty/http/HttpCookieHandlingTests.java
index dcb850657c..678c2b4f68 100644
--- a/reactor-netty-http/src/test/java/reactor/netty/http/HttpCookieHandlingTests.java
+++ b/reactor-netty-http/src/test/java/reactor/netty/http/HttpCookieHandlingTests.java
@@ -17,11 +17,13 @@
 
 import java.time.Duration;
 import java.util.Collection;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
 
 import io.netty.handler.codec.http.HttpHeaderNames;
+import io.netty.handler.codec.http.HttpMethod;
 import io.netty.handler.codec.http.cookie.ClientCookieDecoder;
 import io.netty.handler.codec.http.cookie.ClientCookieEncoder;
 import io.netty.handler.codec.http.cookie.Cookie;
@@ -164,4 +166,38 @@ private void doTestServerCookiesDecodingMultipleCookiesSameName(
 		        .expectComplete()
 		        .verify(Duration.ofSeconds(5));
 	}
+
+	@Test
+	void testIssue2983() {
+		disposableServer =
+				createServer()
+				        .handle((req, res) -> {
+				            List<String> cookies = req.requestHeaders().getAll(HttpHeaderNames.COOKIE);
+				            return cookies.size() == 1 ? res.sendString(Mono.just(cookies.get(0))) :
+				                    res.sendString(Mono.just("ERROR"));
+				        })
+				        .bindNow();
+
+		createClient(disposableServer.port())
+		        .request(HttpMethod.GET)
+		        .uri("/")
+		        .send((req, out) -> {
+		            Cookie cookie1 = new DefaultCookie("testIssue2983_1", "1");
+		            cookie1.setPath("/");
+		            Cookie cookie2 = new DefaultCookie("testIssue2983_2", "2");
+		            cookie2.setPath("/2");
+		            Cookie cookie3 = new DefaultCookie("testIssue2983_3", "3");
+		            req.addCookie(cookie1)
+		               .addCookie(cookie2)
+		               .addCookie(cookie3);
+		            return out;
+		        })
+		        .responseContent()
+		        .aggregate()
+		        .asString()
+		        .as(StepVerifier::create)
+		        .expectNext("testIssue2983_3=3; testIssue2983_2=2; testIssue2983_1=1")
+		        .expectComplete()
+		        .verify(Duration.ofSeconds(5));
+	}
 }
\ No newline at end of file