Jetty 12.0.x 9760 fix cookie parsing (#9894)

* Add test to show failure * Fix #9760 EE9 Cookies Fix #9760 Only set path and domain if they are not blank * Fix #9760 EE9 Cookies Fix #9760 Only set path and domain if they are not blank Switch on violation rather than type * Handle legacy cookie version and comment * Handle cookie version and comment --------- Co-authored-by: gregw <[email protected]>
jetty · Jun 8, 2023 · c13c869 · c13c869
1 parent 9330c1c
commit c13c869
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 6 deletions.
diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java
@@ -2196,18 +2196,30 @@ private static Cookie convertCookie(HttpCookie cookie, CookieCompliance complian
             try
             {
                 Cookie result = new Cookie(cookie.getName(), cookie.getValue());
-                //RFC2965 defines the cookie header as supporting path and domain but RFC6265 permits only name=value
-                if (CookieCompliance.RFC2965.equals(compliance))
+
+                if (compliance.allows(CookieCompliance.Violation.ATTRIBUTE_VALUES))
                 {
-                    result.setPath(cookie.getPath());
-                    result.setDomain(cookie.getDomain());
+                    if (cookie.getVersion() > 0)
+                        result.setVersion(cookie.getVersion());
+
+                    String path = cookie.getPath();
+                    if (StringUtil.isNotBlank(path))
+                        result.setPath(path);
+
+                    String domain = cookie.getDomain();
+                    if (StringUtil.isNotBlank(domain))
+                        result.setDomain(domain);
+
+                    String comment = cookie.getComment();
+                    if (StringUtil.isNotBlank(comment))
+                        result.setComment(comment);
                 }
                 return result;
             }
-            catch (Exception ignore)
+            catch (Exception x)
             {
                 if (LOG.isDebugEnabled())
-                    LOG.debug("Bad Cookie", ignore);
+                    LOG.debug("Bad Cookie", x);
             }
             return null;
         }

diff --git a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java
@@ -1475,6 +1475,74 @@ public void testConnectionClose() throws Exception
         assertThat(response, containsString("Hello World"));
     }
 
+    @Test
+    public void testSpecCookies() throws Exception
+    {
+        _server.stop();
+        _connector.getConnectionFactory(HttpConnectionFactory.class)
+            .getHttpConfiguration().setRequestCookieCompliance(CookieCompliance.RFC2965);
+        _server.start();
+
+        final ArrayList<Cookie> cookies = new ArrayList<>();
+
+        _handler._checker = (request, response) ->
+        {
+            Cookie[] ca = request.getCookies();
+            if (ca != null)
+                cookies.addAll(Arrays.asList(ca));
+            response.getOutputStream().println("Cookie monster!");
+            return true;
+        };
+
+        String response = _connector.getResponse(
+            """
+                GET / HTTP/1.1
+                Host: whatever
+                Cookie: name1=value1
+                Connection: close
+
+                """
+        );
+        assertTrue(response.startsWith("HTTP/1.1 200 OK"));
+        assertEquals(1, cookies.size());
+    }
+
+    @Test
+    public void testSpecCookiesVersion() throws Exception
+    {
+        _server.stop();
+        _connector.getConnectionFactory(HttpConnectionFactory.class)
+            .getHttpConfiguration().setRequestCookieCompliance(CookieCompliance.RFC2965);
+        _server.start();
+
+        final ArrayList<Cookie> cookies = new ArrayList<>();
+
+        _handler._checker = (request, response) ->
+        {
+            Cookie[] ca = request.getCookies();
+            if (ca != null)
+                cookies.addAll(Arrays.asList(ca));
+            response.getOutputStream().println("Cookie monster!");
+            return true;
+        };
+
+        String response = _connector.getResponse(
+            """
+                GET / HTTP/1.1
+                Host: whatever
+                Cookie: $Version="1"; name1="value1"; $Path="/servlet_jsh_cookie_web"; $Domain="localhost"
+                Connection: close
+                
+                """
+        );
+        assertTrue(response.startsWith("HTTP/1.1 200 OK"));
+        assertEquals(1, cookies.size());
+        Cookie cookie = cookies.get(0);
+        assertThat(cookie.getVersion(), is(1));
+        assertThat(cookie.getPath(), is("/servlet_jsh_cookie_web"));
+        assertThat(cookie.getDomain(), is("localhost"));
+    }
+
     @Test
     public void testCookies() throws Exception
     {