You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2022/12/02 15:21:12 UTC
[tomcat] branch main updated: Correct handling of cookie values with quotes
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org