This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push: new af983a45c8 Correct handling of cookie values with quotes af983a45c8 is described below commit af983a45c8e3f2252c1a14d52024dde63ffffff2 Author: Mark Thomas <ma...@apache.org> AuthorDate: Fri Dec 2 15:21:01 2022 +0000 Correct handling of cookie values with quotes While we should back-port this, I don't intend to at this point. The possibility of breakage is too great. --- .../tomcat/util/http/Rfc6265CookieProcessor.java | 7 ++-- .../org/apache/tomcat/util/http/parser/Cookie.java | 11 ++---- test/org/apache/tomcat/util/http/TestCookies.java | 41 +++++++++++++--------- webapps/docs/changelog.xml | 9 +++++ 4 files changed, 42 insertions(+), 26 deletions(-) diff --git a/java/org/apache/tomcat/util/http/Rfc6265CookieProcessor.java b/java/org/apache/tomcat/util/http/Rfc6265CookieProcessor.java index 8f2da681b2..2a35863a2b 100644 --- a/java/org/apache/tomcat/util/http/Rfc6265CookieProcessor.java +++ b/java/org/apache/tomcat/util/http/Rfc6265CookieProcessor.java @@ -210,14 +210,17 @@ public class Rfc6265CookieProcessor extends CookieProcessorBase { private void validateCookieValue(String value) { int start = 0; int end = value.length(); + boolean quoted = false; if (end > 1 && value.charAt(0) == '"' && value.charAt(end - 1) == '"') { - start = 1; - end--; + quoted = true; } char[] chars = value.toCharArray(); for (int i = start; i < end; i++) { + if (quoted && (i == start || i == end - 1)) { + continue; + } char c = chars[i]; if (c < 0x21 || c == 0x22 || c == 0x2c || c == 0x3b || c == 0x5c || c == 0x7f) { throw new IllegalArgumentException(sm.getString( diff --git a/java/org/apache/tomcat/util/http/parser/Cookie.java b/java/org/apache/tomcat/util/http/parser/Cookie.java index f10d53d232..8801f0f5e9 100644 --- a/java/org/apache/tomcat/util/http/parser/Cookie.java +++ b/java/org/apache/tomcat/util/http/parser/Cookie.java @@ -185,13 +185,6 @@ public class Cookie { */ private static ByteBuffer readCookieValueRfc6265(ByteBuffer bb) { boolean quoted = false; - if (bb.hasRemaining()) { - if (bb.get() == QUOTE_BYTE) { - quoted = true; - } else { - bb.rewind(); - } - } int start = bb.position(); int end = bb.limit(); while (bb.hasRemaining()) { @@ -202,8 +195,10 @@ public class Cookie { end = bb.position() - 1; bb.position(end); break; + } else if (b == QUOTE_BYTE && start == bb.position() -1) { + quoted = true; } else if (quoted && b == QUOTE_BYTE) { - end = bb.position() - 1; + end = bb.position(); break; } else { // Invalid cookie diff --git a/test/org/apache/tomcat/util/http/TestCookies.java b/test/org/apache/tomcat/util/http/TestCookies.java index 265801789f..3333bb08c0 100644 --- a/test/org/apache/tomcat/util/http/TestCookies.java +++ b/test/org/apache/tomcat/util/http/TestCookies.java @@ -29,9 +29,12 @@ public class TestCookies { private final Cookie FOO = new Cookie("foo", "bar"); private final Cookie FOO_EMPTY = new Cookie("foo", ""); private final Cookie FOO_CONTROL = new Cookie("foo", "b\u00e1r"); + private final Cookie FOO_CONTROL_QUOTED = new Cookie("foo", "\"b\u00e1r\""); + private final Cookie FOO_QUOTED = new Cookie("foo", "\"bar\""); private final Cookie BAR = new Cookie("bar", "rab"); private final Cookie BAR_EMPTY = new Cookie("bar", ""); private final Cookie A = new Cookie("a", "b"); + private final Cookie A_QUOTED = new Cookie("a", "\"b\""); private final Cookie HASH_EMPTY = new Cookie("#", ""); private final Cookie $PORT = new Cookie("$Port", "8080"); // RFC 2109 attributes are generally interpreted as additional cookies by @@ -74,8 +77,13 @@ public class TestCookies { @Test public void testQuotedValueRfc6265() { - test("foo=bar;a=\"b\"", FOO, A); - test("foo=bar;a=\"b\";", FOO, A); + test("foo=bar;a=\"b\"", FOO, A_QUOTED); + test("foo=bar;a=\"b\";", FOO, A_QUOTED); + } + + @Test + public void testInvalidQuotedValueRfc6265() { + test("a=\"b\"x;foo=bar", FOO); } @Test @@ -109,7 +117,7 @@ public class TestCookies { @Test public void v1QuotedValueRfc6265() { - test("$Version=1;foo=\"bar\";a=b; ; ", $VERSION_1, FOO, A); + test("$Version=1;foo=\"bar\";a=b; ; ", $VERSION_1, FOO_QUOTED, A); } @Test @@ -126,7 +134,7 @@ public class TestCookies { @Test public void v1QuoteInQuotedValueRfc6265() { - FOO.setValue("b'ar"); + FOO.setValue("\"b'ar\""); test("$Version=1;foo=\"b'ar\";a=b", $VERSION_1, FOO, A); } @@ -155,28 +163,28 @@ public class TestCookies { @Test public void v1DomainIsParsedRfc6265() { - FOO.setDomain("apache.org"); + FOO_QUOTED.setDomain("apache.org"); A.setDomain("yahoo.com"); test("$Version=1;foo=\"bar\";$Domain=apache.org;a=b;$Domain=yahoo.com", - $VERSION_1, FOO, $DOMAIN_ASF, A, $DOMAIN_YAHOO); + $VERSION_1, FOO_QUOTED, $DOMAIN_ASF, A, $DOMAIN_YAHOO); } @Test public void v1DomainOnlyAffectsPrecedingCookieRfc6265() { - FOO.setDomain("apache.org"); - test("$Version=1;foo=\"bar\";$Domain=apache.org;a=b", $VERSION_1, FOO, $DOMAIN_ASF, A); + FOO_QUOTED.setDomain("apache.org"); + test("$Version=1;foo=\"bar\";$Domain=apache.org;a=b", $VERSION_1, FOO_QUOTED, $DOMAIN_ASF, A); } @Test public void v1PortIsIgnoredRfc6265() { FOO.setDomain("apache.org"); - test("$Version=1;foo=\"bar\";$Domain=apache.org;$Port=8080;a=b", $VERSION_1, FOO, $DOMAIN_ASF, $PORT, A); + test("$Version=1;foo=\"bar\";$Domain=apache.org;$Port=8080;a=b", $VERSION_1, FOO_QUOTED, $DOMAIN_ASF, $PORT, A); } @Test public void v1PathAffectsPrecedingCookieRfc6265() { - FOO.setPath("/examples"); - test("$Version=1;foo=\"bar\";$Path=/examples;a=b; ; ", $VERSION_1, FOO, $PATH, A); + FOO_QUOTED.setPath("/examples"); + test("$Version=1;foo=\"bar\";$Path=/examples;a=b; ; ", $VERSION_1, FOO_QUOTED, $PATH, A); } @Test @@ -219,7 +227,7 @@ public class TestCookies { @Test public void allow8bitInV1QuotedValue() { // Bug 55917 - test("$Version=1; foo=\"b\u00e1r\"", $VERSION_1, FOO_CONTROL); + test("$Version=1; foo=\"b\u00e1r\"", $VERSION_1, FOO_CONTROL_QUOTED); } @Test @@ -268,12 +276,13 @@ public class TestCookies { @Test public void testBug60788Rfc6265() { - Cookie c1 = new Cookie("userId", "foo"); - Cookie c2 = new Cookie("$Path", "/"); - Cookie c3 = new Cookie("$Domain", "www.example.org"); + Cookie c1 = new Cookie("$Version", "\"1\""); + Cookie c2 = new Cookie("userId", "\"foo\""); + Cookie c3 = new Cookie("$Path", "\"/\""); + Cookie c4 = new Cookie("$Domain", "\"www.example.org\""); test("$Version=\"1\"; userId=\"foo\";$Path=\"/\";$Domain=\"www.example.org\"", - $VERSION_1, c1, c2, c3); + c1, c2, c3, c4); } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 508ee70c28..46ea8df376 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -105,6 +105,15 @@ issues do not "pop up" wrt. others). --> <section name="Tomcat 11.0.0-M2 (markt)" rtext="in development"> + <subsection name="Coyote"> + <changelog> + <fix> + Update Cookie parsing and handling to treat the quotes in a quoted + cookie value as part of the value as required by RFC 6265 and explicitly + clarified in RFC 6265bis. (markt) + </fix> + </changelog> + </subsection> </section> <section name="Tomcat 11.0.0-M1 (markt)" rtext="release in progress"> <subsection name="General"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org