From 6ad148c8f9445d55a58d7f183eb6e5eeab2ef4d4 Mon Sep 17 00:00:00 2001 From: Bruce Date: Wed, 14 Aug 2019 22:06:25 -0700 Subject: [PATCH] parse samesite from cookie comment flag utility functions Signed-off-by: Bruce MacDonald --- .../org/eclipse/jetty/http/HttpCookie.java | 46 ++++++------------- .../eclipse/jetty/http/HttpCookieTest.java | 45 +++++++++++------- .../org/eclipse/jetty/server/Response.java | 9 +++- 3 files changed, 49 insertions(+), 51 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java index bb19db8a280..e0ecacf81c8 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java @@ -101,13 +101,12 @@ public class HttpCookie _domain = domain; _path = path; _maxAge = maxAge; - // HttpOnly was supported as a comment in cookie flags before the java.net.HttpCookie implementation so need to check that - _httpOnly = httpOnly || isHttpOnlyInComment(comment); + _httpOnly = httpOnly; _secure = secure; - _comment = getCommentWithoutFlags(comment); + _comment = comment; _version = version; _expiration = maxAge < 0 ? -1 : System.nanoTime() + TimeUnit.SECONDS.toNanos(maxAge); - _sameSite = sameSite == null ? getSameSiteFromComment(comment) : sameSite; + _sameSite = sameSite; } public HttpCookie(String setCookie) @@ -123,12 +122,12 @@ public class HttpCookie _domain = cookie.getDomain(); _path = cookie.getPath(); _maxAge = cookie.getMaxAge(); - _httpOnly = cookie.isHttpOnly() || isHttpOnlyInComment(cookie.getComment()); + _httpOnly = cookie.isHttpOnly(); _secure = cookie.getSecure(); - _comment = getCommentWithoutFlags(cookie.getComment()); + _comment = cookie.getComment(); _version = cookie.getVersion(); _expiration = _maxAge < 0 ? -1 : System.nanoTime() + TimeUnit.SECONDS.toNanos(_maxAge); - // TODO support for SameSite values has not yet been added to java.net.HttpCookie so check comment only + // TODO support for SameSite values has not yet been added to java.net.HttpCookie _sameSite = getSameSiteFromComment(cookie.getComment()); } @@ -421,12 +420,12 @@ public class HttpCookie return buf.toString(); } - private static boolean isHttpOnlyInComment(String comment) + public static boolean isHttpOnlyInComment(String comment) { return comment != null && comment.contains(HTTP_ONLY_COMMENT); } - private static SameSite getSameSiteFromComment(String comment) + public static SameSite getSameSiteFromComment(String comment) { if (comment != null) { @@ -447,7 +446,7 @@ public class HttpCookie return null; } - private static String getCommentWithoutFlags(String comment) + public static String getCommentWithoutFlags(String comment) { if (comment == null) { @@ -456,29 +455,10 @@ public class HttpCookie String strippedComment = comment.trim(); - if (isHttpOnlyInComment(strippedComment)) - { - strippedComment = StringUtil.strip(strippedComment, HTTP_ONLY_COMMENT); - } - - SameSite commentSameSite = getSameSiteFromComment(strippedComment); - if (commentSameSite != null) - { - switch (commentSameSite) - { - case NONE: - strippedComment = StringUtil.strip(strippedComment, SAME_SITE_NONE_COMMENT); - break; - case LAX: - strippedComment = StringUtil.strip(strippedComment, SAME_SITE_LAX_COMMENT); - break; - case STRICT: - strippedComment = StringUtil.strip(strippedComment, SAME_SITE_STRICT_COMMENT); - break; - default: - break; - } - } + strippedComment = StringUtil.strip(strippedComment, HTTP_ONLY_COMMENT); + strippedComment = StringUtil.strip(strippedComment, SAME_SITE_NONE_COMMENT); + strippedComment = StringUtil.strip(strippedComment, SAME_SITE_LAX_COMMENT); + strippedComment = StringUtil.strip(strippedComment, SAME_SITE_STRICT_COMMENT); return strippedComment.length() == 0 ? null : strippedComment; } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java index f3892a83c4c..11c4d6d6417 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java @@ -25,6 +25,9 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; public class HttpCookieTest @@ -100,22 +103,6 @@ public class HttpCookieTest httpCookie = new HttpCookie("everything", "value", "domain", "path", 0, true, true, null, -1, HttpCookie.SameSite.STRICT); assertEquals("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly; SameSite=Strict", httpCookie.getRFC6265SetCookie()); - httpCookie = new HttpCookie("everything", "value", "domain", "path", 0, false, true, "comment__HTTP_ONLY__", -1, null); - assertEquals("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly", httpCookie.getRFC6265SetCookie()); - - httpCookie = new HttpCookie("everything", "value", "domain", "path", 0, true, true, "__SAME_SITE_NONE__", -1, null); - assertEquals("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly; SameSite=None", httpCookie.getRFC6265SetCookie()); - - httpCookie = new HttpCookie("everything", "value", "domain", "path", 0, true, true, "__SAME_SITE_LAX__", -1, null); - assertEquals("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly; SameSite=Lax", httpCookie.getRFC6265SetCookie()); - - httpCookie = new HttpCookie("everything", "value", "domain", "path", 0, true, true, "__SAME_SITE_STRICT__", -1, null); - assertEquals("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly; SameSite=Strict", httpCookie.getRFC6265SetCookie()); - - // specified SameSite takes precedence over comment value - httpCookie = new HttpCookie("everything", "value", "domain", "path", 0, true, true, "comment__SAME_SITE_STRICT__", -1, HttpCookie.SameSite.LAX); - assertEquals("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly; SameSite=Lax", httpCookie.getRFC6265SetCookie()); - String[] badNameExamples = { "\"name\"", "name\t", @@ -205,4 +192,30 @@ public class HttpCookieTest // should not throw an exception } } + + @Test + public void testGetHttpOnlyFromComment() + { + assertTrue(HttpCookie.isHttpOnlyInComment("__HTTP_ONLY__")); + assertTrue(HttpCookie.isHttpOnlyInComment("__HTTP_ONLY__comment")); + assertFalse(HttpCookie.isHttpOnlyInComment("comment")); + } + + @Test + public void testGetSameSiteFromComment() + { + assertEquals(HttpCookie.getSameSiteFromComment("__SAME_SITE_NONE__"), HttpCookie.SameSite.NONE); + assertEquals(HttpCookie.getSameSiteFromComment("__SAME_SITE_LAX__"), HttpCookie.SameSite.LAX); + assertEquals(HttpCookie.getSameSiteFromComment("__SAME_SITE_STRICT__"), HttpCookie.SameSite.STRICT); + assertEquals(HttpCookie.getSameSiteFromComment("__SAME_SITE_NONE____SAME_SITE_STRICT__"), HttpCookie.SameSite.NONE); + assertNull(HttpCookie.getSameSiteFromComment("comment")); + } + + @Test + public void getCommentWithoutFlags() + { + assertEquals(HttpCookie.getCommentWithoutFlags("comment__SAME_SITE_NONE__"), "comment"); + assertEquals(HttpCookie.getCommentWithoutFlags("comment__HTTP_ONLY____SAME_SITE_NONE__"), "comment"); + assertNull(HttpCookie.getCommentWithoutFlags("__SAME_SITE_LAX__")); + } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index cade181abe5..d190a22fba3 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -42,6 +42,7 @@ import org.eclipse.jetty.http.CookieCompliance; import org.eclipse.jetty.http.DateGenerator; import org.eclipse.jetty.http.HttpContent; import org.eclipse.jetty.http.HttpCookie; +import org.eclipse.jetty.http.HttpCookie.SameSite; import org.eclipse.jetty.http.HttpCookie.SetCookieHttpField; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; @@ -229,7 +230,10 @@ public class Response implements HttpServletResponse throw new IllegalArgumentException("Cookie.name cannot be blank/null"); String comment = cookie.getComment(); - boolean httpOnly = cookie.isHttpOnly(); + // HttpOnly was supported as a comment in cookie flags before the java.net.HttpCookie implementation so need to check that + boolean httpOnly = cookie.isHttpOnly() || HttpCookie.isHttpOnlyInComment(comment); + SameSite sameSite = HttpCookie.getSameSiteFromComment(comment); + comment = HttpCookie.getCommentWithoutFlags(comment); addCookie(new HttpCookie( cookie.getName(), @@ -240,7 +244,8 @@ public class Response implements HttpServletResponse httpOnly, cookie.getSecure(), comment, - cookie.getVersion())); + cookie.getVersion(), + sameSite)); } @Override