Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #12241 Restore SameSite config as session cookie comment. #12263

Merged
merged 3 commits into from
Sep 15, 2024
Merged

Issue #12241 Restore SameSite config as session cookie comment. #12263

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d3b2e0
Issue #12241 Restore SameSite config as session cookie comment.
janbartel Sep 12, 2024
518b6c9
Changes after review: expose and use HttpCookieFacade.
janbartel Sep 12, 2024
0f231e4
Remove statics; expose statics in Response.
janbartel Sep 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1494,7 +1494,7 @@ public Supplier<Map<String, String>> getSupplier()
}
}

private static class HttpCookieFacade implements HttpCookie
protected static class HttpCookieFacade implements HttpCookie
{
private final Cookie _cookie;
private final String _comment;
Expand Down Expand Up @@ -1622,12 +1622,12 @@ private static boolean isHttpOnlyInComment(String comment)
return comment != null && comment.contains(HTTP_ONLY_COMMENT);
}

private static boolean isPartitionedInComment(String comment)
protected static boolean isPartitionedInComment(String comment)
{
return comment != null && comment.contains(PARTITIONED_COMMENT);
}

private static SameSite getSameSiteFromComment(String comment)
protected static SameSite getSameSiteFromComment(String comment)
{
if (comment == null)
return null;
Expand All @@ -1640,7 +1640,7 @@ private static SameSite getSameSiteFromComment(String comment)
return null;
}

private static String getCommentWithoutAttributes(String comment)
protected static String getCommentWithoutAttributes(String comment)
{
if (comment == null)
return null;
Expand Down
24 changes: 22 additions & 2 deletions jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SessionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import org.eclipse.jetty.session.SessionIdManager;
import org.eclipse.jetty.session.SessionManager;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.StringUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -613,12 +614,19 @@ public void doHandle(String target, Request baseRequest, HttpServletRequest requ
* CookieConfig
*
* Implementation of the jakarta.servlet.SessionCookieConfig.
* SameSite configuration can be achieved by using setComment
* SameSite configuration can be achieved by using setComment.
* Partitioned configuration can be achieved by using setComment.
*
* @see HttpCookie
*/
public final class CookieConfig implements SessionCookieConfig
{
protected static final String SAME_SITE_COMMENT = "__SAME_SITE_";
joakime marked this conversation as resolved.
Show resolved Hide resolved
protected static final String SAME_SITE_NONE_COMMENT = SAME_SITE_COMMENT + "NONE__";
protected static final String SAME_SITE_LAX_COMMENT = SAME_SITE_COMMENT + "LAX__";
protected static final String SAME_SITE_STRICT_COMMENT = SAME_SITE_COMMENT + "STRICT__";
protected static final String PARTITIONED_COMMENT = "__PARTITIONED__";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These constants are no longer needed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


@Override
public String getComment()
{
Expand Down Expand Up @@ -671,7 +679,19 @@ private void checkAvailable()
public void setComment(String comment)
{
checkAvailable();
_sessionManager.setSessionComment(comment);

if (!StringUtil.isEmpty(comment))
{
HttpCookie.SameSite sameSite = Response.HttpCookieFacade.getSameSiteFromComment(comment);
if (sameSite != null)
_sessionManager.setSameSite(sameSite);

boolean partitioned = Response.HttpCookieFacade.isPartitionedInComment(comment);
if (partitioned)
_sessionManager.setPartitioned(partitioned);

_sessionManager.setSessionComment(Response.HttpCookieFacade.getCommentWithoutAttributes(comment));
}
}

@Override
Expand Down
47 changes: 43 additions & 4 deletions ...y-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/SessionHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ public void afterEach() throws Exception
}

@Test
public void testSessionCookie() throws Exception
public void testSessionCookieConfig() throws Exception
{
Server server = new Server();
MockSessionIdManager idMgr = new MockSessionIdManager(server);
Expand All @@ -190,12 +190,51 @@ public void testSessionCookie() throws Exception
sessionCookieConfig.setSecure(false);
sessionCookieConfig.setPath("/foo");
sessionCookieConfig.setMaxAge(99);

//test setting SameSite and Partitioned the old way in the comment
sessionCookieConfig.setComment(SessionHandler.CookieConfig.PARTITIONED_COMMENT + " " + SessionHandler.CookieConfig.SAME_SITE_STRICT_COMMENT);

//for < ee10, SameSite cannot be set on the SessionCookieConfig, only on the SessionManager, or
//a default value on the context attribute org.eclipse.jetty.cookie.sameSiteDefault
HttpCookie cookie = mgr.getSessionManager().getSessionCookie(session, false);
assertEquals("SPECIAL", cookie.getName());
assertEquals("universe", cookie.getDomain());
assertEquals("/foo", cookie.getPath());
assertFalse(cookie.isHttpOnly());
assertFalse(cookie.isSecure());
assertTrue(cookie.isPartitioned());
assertEquals(99, cookie.getMaxAge());
assertEquals(HttpCookie.SameSite.STRICT, cookie.getSameSite());

String cookieStr = HttpCookieUtils.getRFC6265SetCookie(cookie);
assertThat(cookieStr, containsString("; Partitioned; SameSite=Strict"));
}

@Test
public void testSessionCookieViaSetters() throws Exception
{
Server server = new Server();
MockSessionIdManager idMgr = new MockSessionIdManager(server);
idMgr.setWorkerName("node1");
SessionHandler mgr = new SessionHandler();
MockSessionCache cache = new MockSessionCache(mgr.getSessionManager());
cache.setSessionDataStore(new NullSessionDataStore());
mgr.setSessionCache(cache);
mgr.setSessionIdManager(idMgr);

long now = System.currentTimeMillis();

ManagedSession session = new ManagedSession(mgr.getSessionManager(), new SessionData("123", "_foo", "0.0.0.0", now, now, now, 30));
session.setExtendedId("123.node1");

//test setting up session cookie via setters on SessionHandler
mgr.setSessionCookie("SPECIAL");
mgr.setSessionDomain("universe");
mgr.setHttpOnly(false);
mgr.setSecureCookies(false);
mgr.setSessionPath("/foo");
mgr.setMaxCookieAge(99);
mgr.setSameSite(HttpCookie.SameSite.STRICT);
mgr.setPartitioned(true);

HttpCookie cookie = mgr.getSessionManager().getSessionCookie(session, false);
assertEquals("SPECIAL", cookie.getName());
assertEquals("universe", cookie.getDomain());
Expand Down
Loading