From c1c241349e76f38588c372723abe85dd13522094 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 21 Aug 2019 12:54:26 -0500 Subject: [PATCH] Issue #3985 - Applying PR Review to CookieCutter Signed-off-by: Joakim Erdfelt --- .../eclipse/jetty/server/CookieCutter.java | 31 ++++++++++++------- .../server/CookieCutter_LenientTest.java | 2 +- .../org/eclipse/jetty/server/RequestTest.java | 4 ++- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/CookieCutter.java b/jetty-server/src/main/java/org/eclipse/jetty/server/CookieCutter.java index 148e55ebe9c..09fc0614e2a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/CookieCutter.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/CookieCutter.java @@ -314,11 +314,7 @@ public class CookieCutter if (_compliance == CookieCompliance.RFC6265) { - // Rejected cookie-octet characters - // US-ASCII characters excluding CTLs, - // whitespace DQUOTE, comma, semicolon, - // and backslash - if (Character.isISOControl(c) || Character.isWhitespace(c) || c == '\\') + if (isRFC6265RejectedCharacter(c)) { reject = true; } @@ -372,12 +368,7 @@ public class CookieCutter if (_compliance == CookieCompliance.RFC6265) { - // Rejected cookie-octet characters - // US-ASCII characters excluding CTLs, - // whitespace DQUOTE, comma, semicolon, - // and backslash - if (Character.isISOControl(c) || Character.isWhitespace(c) || - c == ',' || c == '\\') + if (isRFC6265RejectedCharacter(c)) { reject = true; } @@ -396,4 +387,22 @@ public class CookieCutter _cookies = cookies.toArray(new Cookie[0]); _lastCookies = _cookies; } + + protected boolean isRFC6265RejectedCharacter(char c) + { + // We only reject if a Control Character is encountered + if (Character.isISOControl(c)) + { + return true; + } + + /* TODO: Should we also reject for the complete list of invalid characters in RFC6265? + * + * US-ASCII characters excluding CTLs, + * whitespace DQUOTE, comma, semicolon, + * and backslash + */ + + return false; + } } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutter_LenientTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutter_LenientTest.java index 3f4ba333d96..24272efd981 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutter_LenientTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutter_LenientTest.java @@ -130,7 +130,7 @@ public class CookieCutter_LenientTest Arguments.of("foo=\"bar;baz\"", "foo", "bar;baz"), Arguments.of("z=a;b,c:d;e/f[g]", "z", "a"), Arguments.of("z=\"a;b,c:d;e/f[g]\"", "z", "a;b,c:d;e/f[g]"), - Arguments.of("name=quoted=\"\\\"badly\\\"\"", null, null), // someone attempting to escape a DQUOTE from within a DQUOTED pair) + Arguments.of("name=quoted=\"\\\"badly\\\"\"", "name", "quoted=\"\\\"badly\\\"\""), // someone attempting to escape a DQUOTE from within a DQUOTED pair) // Quoted with other Cookie keywords Arguments.of("x=\"$Version=0\"", "x", "$Version=0"), diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java index 5089ce9593b..69a29dc6b71 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java @@ -1515,7 +1515,9 @@ public class RequestTest "\n" ); assertTrue(response.startsWith("HTTP/1.1 200 OK")); - assertEquals(0, cookies.size()); // this is an invalid cookie + assertEquals(1, cookies.size()); + assertEquals("name", cookies.get(0).getName()); + assertEquals("quoted=\"\\\"badly\\\"\"", cookies.get(0).getValue()); cookies.clear(); response = _connector.getResponse(