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
reactor · Dec 6, 2023 · 44a7e89 · 44a7e89
1 parent 9fa10f8
commit 44a7e89
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 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
@@ -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
@@ -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));
+	}
 }