From f5f35d13fa7c8c556043c542616993382acf2a84 Mon Sep 17 00:00:00 2001 From: Javid Khan Date: Fri, 23 Oct 2026 12:54:57 +0530 Subject: [PATCH 1/2] reject cookies with control characters as invalid --- .../http/impl/cookie/RFC6265CookieSpec.java | 30 +++++++++++++- .../impl/cookie/TestRFC6265CookieSpec.java | 40 +++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/RFC6265CookieSpec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/RFC6265CookieSpec.java index 590cc5a319..935192dbe7 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/RFC6265CookieSpec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/RFC6265CookieSpec.java @@ -199,6 +199,12 @@ public final void validate(final Cookie cookie, final CookieOrigin origin) throws MalformedCookieException { Args.notNull(cookie, "Cookie"); Args.notNull(origin, "Cookie origin"); + if (containsControlChar(cookie.getName())) { + throw new MalformedCookieException("Cookie name contains control characters"); + } + if (containsControlChar(cookie.getValue())) { + throw new MalformedCookieException("Cookie value contains control characters"); + } for (final CookieAttributeHandler handler: this.attribHandlers) { handler.validate(cookie, origin); } @@ -236,8 +242,11 @@ public List
formatCookies(final List cookies) { buffer.append(PARAM_DELIMITER); buffer.append(' '); } - buffer.append(cookie.getName()); + final String name = cookie.getName(); + Args.check(!containsControlChar(name), "Cookie name contains control characters"); + buffer.append(name); final String s = cookie.getValue(); + Args.check(!containsControlChar(s), "Cookie value contains control characters"); if (s != null) { buffer.append(EQUAL_CHAR); if (containsSpecialChar(s)) { @@ -278,4 +287,23 @@ boolean containsChars(final CharSequence s, final Tokenizer.Delimiter chars) { return false; } + /** + * Tests whether the given cookie name or value contains a control character + * ({@code %x00-1F} or {@code %x7F}). RFC 6265 permits neither in a cookie-name (a token) + * nor in a cookie-value (cookie-octet), and such characters must never be copied into + * a {@code Cookie} request header. + */ + static boolean containsControlChar(final CharSequence s) { + if (s == null) { + return false; + } + for (int i = 0; i < s.length(); i++) { + final char ch = s.charAt(i); + if (ch < 0x20 || ch == 0x7f) { + return true; + } + } + return false; + } + } diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/cookie/TestRFC6265CookieSpec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/cookie/TestRFC6265CookieSpec.java index 56897c9b81..c74eabb649 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/cookie/TestRFC6265CookieSpec.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/cookie/TestRFC6265CookieSpec.java @@ -222,6 +222,30 @@ void testValidateCookieBasics() throws Exception { Mockito.verify(h2).validate(cookie, origin); } + @Test + void testValidateCookieRejectsControlCharsInName() { + final RFC6265CookieSpec cookiespec = new RFC6265CookieSpec(); + final CookieOrigin origin = new CookieOrigin("host", 80, "/path/", true); + final BasicClientCookie cookie = new BasicClientCookie("sid\r\nInjected: evil", "value"); + Assertions.assertThrows(MalformedCookieException.class, () -> + cookiespec.validate(cookie, origin)); + } + + @Test + void testValidateCookieRejectsControlCharsInValue() throws Exception { + final RFC6265CookieSpec cookiespec = new RFC6265CookieSpec(); + final CookieOrigin origin = new CookieOrigin("host", 80, "/path/", true); + // NUL is neither whitespace nor a delimiter, so it survives parsing and + // must be caught by validation before the cookie reaches the store. + final Header header = new BasicHeader("Set-Cookie", "sid=abc\u0000def"); + final List cookies = cookiespec.parse(header, origin); + Assertions.assertEquals(1, cookies.size()); + final Cookie cookie = cookies.get(0); + Assertions.assertEquals("abc\u0000def", cookie.getValue()); + Assertions.assertThrows(MalformedCookieException.class, () -> + cookiespec.validate(cookie, origin)); + } + @Test void testMatchCookie() { final CommonCookieAttributeHandler h1 = Mockito.mock(CommonCookieAttributeHandler.class); @@ -291,6 +315,22 @@ void testFormatCookiesIllegalCharsInValue() { Assertions.assertEquals("name1=value; name2=\"some value\"; name3=\"\\\"\\\\\\\"\"", header.getValue()); } + @Test + void testFormatCookiesRejectsControlCharsInName() { + final Cookie cookie = new BasicClientCookie("sid\r\nInjected: evil", "value"); + final RFC6265CookieSpec cookiespec = new RFC6265CookieSpec(); + Assertions.assertThrows(IllegalArgumentException.class, () -> + cookiespec.formatCookies(Collections.singletonList(cookie))); + } + + @Test + void testFormatCookiesRejectsControlCharsInValue() { + final Cookie cookie = new BasicClientCookie("sid", "abc\r\nInjected: evil"); + final RFC6265CookieSpec cookiespec = new RFC6265CookieSpec(); + Assertions.assertThrows(IllegalArgumentException.class, () -> + cookiespec.formatCookies(Collections.singletonList(cookie))); + } + @Test void testParseCookieMultipleAttributes() throws Exception { final CommonCookieAttributeHandler h1 = Mockito.mock(CommonCookieAttributeHandler.class); From c809f3064ed23bed7ddd1678158a0e9c5897b0c3 Mon Sep 17 00:00:00 2001 From: Javid Khan Date: Sat, 24 Oct 2026 10:07:23 +0530 Subject: [PATCH 2/2] replace control characters instead of failing when formatting cookies --- .../http/impl/cookie/RFC6265CookieSpec.java | 34 ++++++++++++++++--- .../impl/cookie/TestRFC6265CookieSpec.java | 19 +++++++---- 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/RFC6265CookieSpec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/RFC6265CookieSpec.java index 935192dbe7..967900c761 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/RFC6265CookieSpec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/RFC6265CookieSpec.java @@ -242,11 +242,9 @@ public List
formatCookies(final List cookies) { buffer.append(PARAM_DELIMITER); buffer.append(' '); } - final String name = cookie.getName(); - Args.check(!containsControlChar(name), "Cookie name contains control characters"); + final String name = replaceControlChars(cookie.getName()); buffer.append(name); - final String s = cookie.getValue(); - Args.check(!containsControlChar(s), "Cookie value contains control characters"); + final String s = replaceControlChars(cookie.getValue()); if (s != null) { buffer.append(EQUAL_CHAR); if (containsSpecialChar(s)) { @@ -306,4 +304,32 @@ static boolean containsControlChar(final CharSequence s) { return false; } + /** + * Replaces every control character ({@code %x00-1F} or {@code %x7F}) in the given cookie + * name or value with {@code '?'}, mirroring the substitution the session output buffer + * already performs, so such characters can never be copied verbatim into a {@code Cookie} + * request header. Cookies carrying control characters are rejected by {@link #validate} + * before they reach the cookie store; this is a defensive fallback for a value that bypassed + * validation. + */ + static String replaceControlChars(final CharSequence s) { + if (s == null) { + return null; + } + StringBuilder buf = null; + for (int i = 0; i < s.length(); i++) { + final char ch = s.charAt(i); + if (ch < 0x20 || ch == 0x7f) { + if (buf == null) { + buf = new StringBuilder(s.length()); + buf.append(s, 0, i); + } + buf.append('?'); + } else if (buf != null) { + buf.append(ch); + } + } + return buf != null ? buf.toString() : s.toString(); + } + } diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/cookie/TestRFC6265CookieSpec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/cookie/TestRFC6265CookieSpec.java index c74eabb649..823cc32c31 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/cookie/TestRFC6265CookieSpec.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/cookie/TestRFC6265CookieSpec.java @@ -35,6 +35,7 @@ import org.apache.hc.client5.http.cookie.Cookie; import org.apache.hc.client5.http.cookie.CookieOrigin; import org.apache.hc.client5.http.cookie.MalformedCookieException; +import org.apache.hc.core5.http.FormattedHeader; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.message.BasicHeader; import org.junit.jupiter.api.Assertions; @@ -316,19 +317,25 @@ void testFormatCookiesIllegalCharsInValue() { } @Test - void testFormatCookiesRejectsControlCharsInName() { + void testFormatCookiesReplacesControlCharsInName() { final Cookie cookie = new BasicClientCookie("sid\r\nInjected: evil", "value"); final RFC6265CookieSpec cookiespec = new RFC6265CookieSpec(); - Assertions.assertThrows(IllegalArgumentException.class, () -> - cookiespec.formatCookies(Collections.singletonList(cookie))); + final List
headers = cookiespec.formatCookies(Collections.singletonList(cookie)); + Assertions.assertEquals(1, headers.size()); + final String wire = ((FormattedHeader) headers.get(0)).getBuffer().toString(); + Assertions.assertFalse(wire.indexOf('\r') >= 0 || wire.indexOf('\n') >= 0); + Assertions.assertEquals("Cookie: sid??Injected: evil=value", wire); } @Test - void testFormatCookiesRejectsControlCharsInValue() { + void testFormatCookiesReplacesControlCharsInValue() { final Cookie cookie = new BasicClientCookie("sid", "abc\r\nInjected: evil"); final RFC6265CookieSpec cookiespec = new RFC6265CookieSpec(); - Assertions.assertThrows(IllegalArgumentException.class, () -> - cookiespec.formatCookies(Collections.singletonList(cookie))); + final List
headers = cookiespec.formatCookies(Collections.singletonList(cookie)); + Assertions.assertEquals(1, headers.size()); + final String wire = ((FormattedHeader) headers.get(0)).getBuffer().toString(); + Assertions.assertFalse(wire.indexOf('\r') >= 0 || wire.indexOf('\n') >= 0); + Assertions.assertEquals("Cookie: sid=\"abc??Injected: evil\"", wire); } @Test