diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java index d8c08f1b368..851ca0abc73 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java @@ -13,8 +13,12 @@ package org.eclipse.jetty.http; +import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Locale; +import java.util.Map; +import java.util.TreeMap; import java.util.concurrent.TimeUnit; import org.eclipse.jetty.util.Attributes; @@ -76,7 +80,7 @@ public class HttpCookie private final int _version; private final boolean _httpOnly; private final long _expiration; - private final SameSite _sameSite; + private final Map _attributes; public HttpCookie(String name, String value) { @@ -100,10 +104,16 @@ public class HttpCookie public HttpCookie(String name, String value, String domain, String path, long maxAge, boolean httpOnly, boolean secure, String comment, int version) { - this(name, value, domain, path, maxAge, httpOnly, secure, comment, version, null); + this(name, value, domain, path, maxAge, httpOnly, secure, comment, version, (SameSite)null); } public HttpCookie(String name, String value, String domain, String path, long maxAge, boolean httpOnly, boolean secure, String comment, int version, SameSite sameSite) + { + + this(name, value, domain, path, maxAge, httpOnly, secure, comment, version, Collections.singletonMap("SameSite", sameSite == null ? null : sameSite.getAttributeValue())); + } + + public HttpCookie(String name, String value, String domain, String path, long maxAge, boolean httpOnly, boolean secure, String comment, int version, Map attributes) { _name = name; _value = value; @@ -115,29 +125,26 @@ public class HttpCookie _comment = comment; _version = version; _expiration = maxAge < 0 ? -1 : System.nanoTime() + TimeUnit.SECONDS.toNanos(maxAge); - _sameSite = sameSite; + _attributes = (attributes == null ? Collections.emptyMap() : attributes); } - - public HttpCookie(String setCookie) + + public HttpCookie(String name, String value, int version, Map attributes) { - List cookies = java.net.HttpCookie.parse(setCookie); - if (cookies.size() != 1) - throw new IllegalStateException(); + _name = name; + _value = value; + _version = version; + _attributes = (attributes == null ? Collections.emptyMap() : new TreeMap<>(attributes)); + + //remove all of the well-known attributes, leaving only those pass-through ones + _domain = _attributes.remove("Domain"); + _path = _attributes.remove("Path"); - java.net.HttpCookie cookie = cookies.get(0); - - _name = cookie.getName(); - _value = cookie.getValue(); - _domain = cookie.getDomain(); - _path = cookie.getPath(); - _maxAge = cookie.getMaxAge(); - _httpOnly = cookie.isHttpOnly(); - _secure = cookie.getSecure(); - _comment = cookie.getComment(); - _version = cookie.getVersion(); + String tmp = _attributes.remove("Max-Age"); + _maxAge = StringUtil.isBlank(tmp) ? -1L : Long.valueOf(tmp); _expiration = _maxAge < 0 ? -1 : System.nanoTime() + TimeUnit.SECONDS.toNanos(_maxAge); - // support for SameSite values has not yet been added to java.net.HttpCookie - _sameSite = getSameSiteFromComment(cookie.getComment()); + _httpOnly = Boolean.parseBoolean(_attributes.remove("HttpOnly")); + _secure = Boolean.parseBoolean(_attributes.remove("Secure")); + _comment = _attributes.remove("Comment"); } /** @@ -209,7 +216,10 @@ public class HttpCookie */ public SameSite getSameSite() { - return _sameSite; + String val = _attributes.get("SameSite"); + if (val == null) + return null; + return SameSite.valueOf(val.toUpperCase(Locale.ENGLISH)); } /** @@ -421,12 +431,21 @@ public class HttpCookie buf.append("; Secure"); if (_httpOnly) buf.append("; HttpOnly"); - if (_sameSite != null) + + String sameSite = _attributes.get("SameSite"); + if (sameSite != null) { buf.append("; SameSite="); - buf.append(_sameSite.getAttributeValue()); + buf.append(sameSite); } + //Add all other attributes + _attributes.entrySet().stream().filter(e -> !"SameSite".equals(e.getKey())).forEach(e -> + { + buf.append("; " + e.getKey() + "="); + buf.append(e.getValue()); + }); + return buf.toString(); } @@ -491,6 +510,109 @@ public class HttpCookie throw new IllegalStateException(e); } } + + /** + * Extract the bare minimum of info from a Set-Cookie header string. + * + * Ideally this method should not be necessary, however as java.net.HttpCookie + * does not yet support generic attributes, we have to use it in a minimal + * fashion. When it supports attributes, we could look at reverting to a + * constructor on o.e.j.h.HttpCookie to take the set-cookie header string. + * + * @param setCookieHeader the header as a string + * @return a map containing the name, value, domain, path. max-age of the set cookie header + */ + public static Map extractBasics(String setCookieHeader) + { + //Parse the bare minimum + List cookies = java.net.HttpCookie.parse(setCookieHeader); + if (cookies.size() != 1) + return Collections.emptyMap(); + java.net.HttpCookie cookie = cookies.get(0); + Map fields = new HashMap<>(); + fields.put("name", cookie.getName()); + fields.put("value", cookie.getValue()); + fields.put("domain", cookie.getDomain()); + fields.put("path", cookie.getPath()); + fields.put("max-age", Long.toString(cookie.getMaxAge())); + return fields; + } + + /** + * Check if the Set-Cookie header represented as a string is for the name, domain and path given. + * + * @param setCookieHeader a Set-Cookie header + * @param name the cookie name to check + * @param domain the cookie domain to check + * @param path the cookie path to check + * @return true if all of the name, domain and path match the Set-Cookie header, false otherwise + */ + public static boolean match(String setCookieHeader, String name, String domain, String path) + { + //Parse the bare minimum + List cookies = java.net.HttpCookie.parse(setCookieHeader); + if (cookies.size() != 1) + return false; + + java.net.HttpCookie cookie = cookies.get(0); + return match(cookie.getName(), cookie.getDomain(), cookie.getPath(), name, domain, path); + } + + /** + * Check if the HttpCookie is for the given name, domain and path. + * + * @param cookie the jetty HttpCookie to check + * @param name the cookie name to check + * @param domain the cookie domain to check + * @param path the cookie path to check + * @return true if all of the name, domain and path all match the HttpCookie, false otherwise + */ + public static boolean match(HttpCookie cookie, String name, String domain, String path) + { + if (cookie == null) + return false; + return match(cookie.getName(), cookie.getDomain(), cookie.getPath(), name, domain, path); + } + + /** + * Check if all old parameters match the new parameters. + * + * @param oldName + * @param oldDomain + * @param oldPath + * @param newName + * @param newDomain + * @param newPath + * @return true if old and new names match exactly and the old and new domains match case-insensitively and the paths match exactly + */ + private static boolean match(String oldName, String oldDomain, String oldPath, String newName, String newDomain, String newPath) + { + if (oldName == null) + { + if (newName != null) + return false; + } + else if (!oldName.equals(newName)) + return false; + + if (oldDomain == null) + { + if (newDomain != null) + return false; + } + else if (!oldDomain.equalsIgnoreCase(newDomain)) + return false; + + if (oldPath == null) + { + if (newPath != null) + return false; + } + else if (!oldPath.equals(newPath)) + return false; + + return true; + } /** * @deprecated We should not need to do this now @@ -563,7 +685,7 @@ public class HttpCookie return _cookie; } } - + /** * Check that samesite is set on the cookie. If not, use a * context default value, if one has been set. diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java index 30cfe5cb449..248121b67ba 100644 --- a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java +++ b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.http; +import java.util.Collections; import java.util.stream.Stream; import org.eclipse.jetty.http.HttpCookie.SameSite; @@ -64,9 +65,28 @@ public class HttpCookieTest } @Test - public void testConstructFromSetCookie() + public void testMatchCookie() { - HttpCookie cookie = new HttpCookie("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly"); + //match with header string + assertTrue(HttpCookie.match("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly; SameSite=Lax; Foo=Bar", + "everything", "domain", "path")); + assertFalse(HttpCookie.match("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly; SameSite=Lax; Foo=Bar", + "something", "domain", "path")); + assertFalse(HttpCookie.match("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly; SameSite=Lax; Foo=Bar", + "everything", "realm", "path")); + assertFalse(HttpCookie.match("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly; SameSite=Lax; Foo=Bar", + "everything", "domain", "street")); + + //match including set-cookie:, this is really testing the java.net.HttpCookie parser, but worth throwing in there + assertTrue(HttpCookie.match("Set-Cookie: everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly; SameSite=Lax; Foo=Bar", + "everything", "domain", "path")); + + //match via cookie + HttpCookie httpCookie = new HttpCookie("everything", "value", "domain", "path", 0, true, true, "comment", 0); + assertTrue(HttpCookie.match(httpCookie, "everything", "domain", "path")); + assertFalse(HttpCookie.match(httpCookie, "something", "domain", "path")); + assertFalse(HttpCookie.match(httpCookie, "everything", "realm", "path")); + assertFalse(HttpCookie.match(httpCookie, "everything", "domain", "street")); } @Test @@ -131,6 +151,9 @@ 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, true, true, null, -1, Collections.singletonMap("SameSite", "None")); + 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()); } public static Stream rfc6265BadNameSource() diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index 2d5f578c8a2..5a9ba67180e 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -147,28 +147,15 @@ public interface Response extends Content.Sink CookieCompliance compliance = httpConfiguration.getResponseCookieCompliance(); HttpCookie oldCookie; if (field instanceof HttpCookie.SetCookieHttpField) - oldCookie = ((HttpCookie.SetCookieHttpField)field).getHttpCookie(); + { + if (!HttpCookie.match(((HttpCookie.SetCookieHttpField)field).getHttpCookie(), cookie.getName(), cookie.getDomain(), cookie.getPath())) + continue; + } else - oldCookie = new HttpCookie(field.getValue()); - - if (!cookie.getName().equals(oldCookie.getName())) - continue; - - if (cookie.getDomain() == null) { - if (oldCookie.getDomain() != null) + if (!HttpCookie.match(field.getValue(), cookie.getName(), cookie.getDomain(), cookie.getPath())) continue; } - else if (!cookie.getDomain().equalsIgnoreCase(oldCookie.getDomain())) - continue; - - if (cookie.getPath() == null) - { - if (oldCookie.getPath() != null) - continue; - } - else if (!cookie.getPath().equals(oldCookie.getPath())) - continue; i.set(new HttpCookie.SetCookieHttpField(HttpCookie.checkSameSite(cookie, request.getContext()), compliance)); return; diff --git a/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionManager.java b/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionManager.java index 4e0f59f46a3..0690258363e 100644 --- a/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionManager.java +++ b/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionManager.java @@ -15,10 +15,13 @@ package org.eclipse.jetty.session; import java.nio.ByteBuffer; import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.TreeMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import java.util.function.Consumer; @@ -77,6 +80,7 @@ public abstract class AbstractSessionManager extends ContainerLifeCycle implemen private String _sessionDomain; private String _sessionPath; private String _sessionComment; + private final Map _sessionAttributes = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); private boolean _secureCookies = false; private boolean _secureRequestOnly = true; private int _maxCookieAge = -1; @@ -356,28 +360,6 @@ public abstract class AbstractSessionManager extends ContainerLifeCycle implemen { _refreshCookieAge = ageInSeconds; } - - @Override - public HttpCookie.SameSite getSameSite() - { - // TODO do this properly - return HttpCookie.getSameSiteFromComment(_sessionComment); - } - - /** - * Set Session cookie sameSite mode. - * - * @param sameSite The sameSite setting for Session cookies (or null for no sameSite setting) - */ - @Override - public void setSameSite(HttpCookie.SameSite sameSite) - { - // TODO this can be done properly now? - // Encode in comment whilst not supported by SessionConfig, so that it can be set/saved in - // web.xml and quickstart. - // Always pass false for httpOnly as it has it's own setter. - _sessionComment = HttpCookie.getCommentWithAttributes(_sessionComment, false, sameSite); - } public abstract Server getServer(); @@ -479,65 +461,6 @@ public abstract class AbstractSessionManager extends ContainerLifeCycle implemen return _sessionContext; } - /** - * A session cookie is marked as secure IFF any of the following conditions are true: - *
    - *
  1. SessionCookieConfig.setSecure == true
    2. - *
  2. SessionCookieConfig.setSecure == false && _secureRequestOnly==true && request is HTTPS
    3. - *
- * According to SessionCookieConfig javadoc, case 1 can be used when: - * "... even though the request that initiated the session came over HTTP, - * is to support a topology where the web container is front-ended by an - * SSL offloading load balancer. In this case, the traffic between the client - * and the load balancer will be over HTTPS, whereas the traffic between the - * load balancer and the web container will be over HTTP." - *

- * For case 2, you can use _secureRequestOnly to determine if you want the - * Servlet Spec 3.0 default behavior when SessionCookieConfig.setSecure==false, - * which is: - * - * "they shall be marked as secure only if the request that initiated the - * corresponding session was also secure" - * - *

- * The default for _secureRequestOnly is true, which gives the above behavior. If - * you set it to false, then a session cookie is NEVER marked as secure, even if - * the initiating request was secure. - * - * @param session the session to which the cookie should refer. - * @param contextPath the context to which the cookie should be linked. - * The client will only send the cookie value when requesting resources under this path. - * @param requestIsSecure whether the client is accessing the server over a secure protocol (i.e. HTTPS). - * @return if this SessionManager uses cookies, then this method will return a new - * {@link HttpCookie cookie object} that should be set on the client in order to link future HTTP requests - * with the session. If cookies are not in use, this method returns null. - */ - @Override - public HttpCookie getSessionCookie(Session session, String contextPath, boolean requestIsSecure) - { - if (isUsingCookies()) - { - String sessionPath = (_sessionPath == null) ? contextPath : _sessionPath; - sessionPath = (StringUtil.isEmpty(sessionPath)) ? "/" : sessionPath; - String id = session.getExtendedId(); - HttpCookie cookie = new HttpCookie( - (_sessionCookie == null ? __DefaultSessionCookie : _sessionCookie), - id, - _sessionDomain, - sessionPath, - _maxCookieAge, - _httpOnly, - _secureCookies || (isSecureRequestOnly() && requestIsSecure), - HttpCookie.getCommentWithoutAttributes(_sessionComment), - 0, - HttpCookie.getSameSiteFromComment(_sessionComment)); - - session.onSetCookieGenerated(); - return cookie; - } - return null; - } - @Override public String getSessionCookie() { @@ -565,6 +488,25 @@ public abstract class AbstractSessionManager extends ContainerLifeCycle implemen _sessionDomain = domain; } + public void setSessionAttribute(String name, String value) + { + _sessionAttributes.put(name, value); + } + + public String getSessionAttribute(String name) + { + return _sessionAttributes.get(name); + } + + /** + * @return all of the cookie config attributes EXCEPT for + * those that have explicit setter/getters + */ + public Map getSessionAttributes() + { + return Collections.unmodifiableMap(_sessionAttributes); + } + @Override public SessionIdManager getSessionIdManager() { diff --git a/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/Session.java b/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/Session.java index 931a3804e82..50c66818a19 100644 --- a/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/Session.java +++ b/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/Session.java @@ -14,9 +14,11 @@ package org.eclipse.jetty.session; import java.util.Collections; +import java.util.Map; import java.util.Set; import java.util.concurrent.locks.Condition; +import org.eclipse.jetty.http.HttpCookie; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; @@ -148,6 +150,14 @@ public class Session _extendedId = extendedId; } + public HttpCookie generateSetCookie(String name, String domain, String path, int maxAge, + boolean httpOnly, boolean secure, String comment, int version, Map attributes) + { + HttpCookie sessionCookie = new HttpCookie(name, getExtendedId(), domain, path, maxAge, httpOnly, secure, comment, version, attributes); + onSetCookieGenerated(); + return sessionCookie; + } + /** * Set the time that the cookie was set and clear the idChanged flag. */ diff --git a/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/SimpleSessionHandler.java b/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/SimpleSessionHandler.java index 78b24a25807..d3feafe09f8 100644 --- a/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/SimpleSessionHandler.java +++ b/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/SimpleSessionHandler.java @@ -13,15 +13,19 @@ package org.eclipse.jetty.session; +import java.util.Collections; +import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import org.eclipse.jetty.http.HttpCookie; +import org.eclipse.jetty.http.HttpCookie.SameSite; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.StringUtil; /** * SimpleSessionHandler example @@ -190,4 +194,41 @@ public class SimpleSessionHandler extends AbstractSessionManager implements Hand Response.replaceCookie(response, sessionManager.getSessionCookie(getCoreSession(), request.getContext().getContextPath(), request.isSecure())); } } + + @Override + public HttpCookie getSessionCookie(Session session, String contextPath, boolean requestIsSecure) + { + if (isUsingCookies()) + { + String sessionPath = getSessionPath(); + sessionPath = (sessionPath == null) ? contextPath : sessionPath; + sessionPath = (StringUtil.isEmpty(sessionPath)) ? "/" : sessionPath; + SameSite sameSite = HttpCookie.getSameSiteFromComment(getSessionComment()); + Map attributes = Collections.emptyMap(); + if (sameSite != null) + attributes = Collections.singletonMap("SameSite", sameSite.getAttributeValue()); + return session.generateSetCookie((getSessionCookie() == null ? __DefaultSessionCookie : getSessionCookie()), + getSessionDomain(), + sessionPath, + getMaxCookieAge(), + isHttpOnly(), + isSecureCookies() || (isSecureRequestOnly() && requestIsSecure), + HttpCookie.getCommentWithoutAttributes(getSessionComment()), + 0, + attributes); + } + return null; + } + + @Override + public SameSite getSameSite() + { + return HttpCookie.getSameSiteFromComment(getSessionComment()); + } + + @Override + public void setSameSite(SameSite sameSite) + { + setSessionComment(HttpCookie.getCommentWithAttributes(getSessionComment(), false, sameSite)); + } } diff --git a/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/TestableSessionManager.java b/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/TestableSessionManager.java index f0ad318843b..249836db421 100644 --- a/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/TestableSessionManager.java +++ b/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/TestableSessionManager.java @@ -14,12 +14,16 @@ package org.eclipse.jetty.session; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.Map; +import org.eclipse.jetty.http.HttpCookie; +import org.eclipse.jetty.http.HttpCookie.SameSite; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.session.Session.APISession; +import org.eclipse.jetty.util.StringUtil; /** * TestSessionHandler @@ -173,4 +177,41 @@ public class TestableSessionManager extends AbstractSessionManager { return _cookieConfig; } + + @Override + public HttpCookie getSessionCookie(Session session, String contextPath, boolean requestIsSecure) + { + if (isUsingCookies()) + { + String sessionPath = getSessionPath(); + sessionPath = (sessionPath == null) ? contextPath : sessionPath; + sessionPath = (StringUtil.isEmpty(sessionPath)) ? "/" : sessionPath; + SameSite sameSite = HttpCookie.getSameSiteFromComment(getSessionComment()); + Map attributes = Collections.emptyMap(); + if (sameSite != null) + attributes = Collections.singletonMap("SameSite", sameSite.getAttributeValue()); + return session.generateSetCookie((getSessionCookie() == null ? __DefaultSessionCookie : getSessionCookie()), + getSessionDomain(), + sessionPath, + getMaxCookieAge(), + isHttpOnly(), + isSecureCookies() || (isSecureRequestOnly() && requestIsSecure), + HttpCookie.getCommentWithoutAttributes(getSessionComment()), + 0, + attributes); + } + return null; + } + + @Override + public SameSite getSameSite() + { + return HttpCookie.getSameSiteFromComment(getSessionComment()); + } + + @Override + public void setSameSite(SameSite sameSite) + { + setSessionComment(HttpCookie.getCommentWithAttributes(getSessionComment(), false, sameSite)); + } } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java index a9f162a6048..05f6a9dbcc6 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java @@ -498,21 +498,33 @@ public class ServletContextResponse extends ContextResponse String comment = cookie.getComment(); // 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); - HttpCookie.SameSite sameSite = HttpCookie.getSameSiteFromComment(comment); + boolean httpOnlyFromComment = cookie.isHttpOnly() || HttpCookie.isHttpOnlyInComment(comment); + HttpCookie.SameSite sameSiteFromComment = HttpCookie.getSameSiteFromComment(comment); comment = HttpCookie.getCommentWithoutAttributes(comment); - - addCookie(new HttpCookie( - cookie.getName(), - cookie.getValue(), - cookie.getDomain(), - cookie.getPath(), - cookie.getMaxAge(), - httpOnly, - cookie.getSecure(), - comment, - cookie.getVersion(), - sameSite)); + //old style cookie + if (sameSiteFromComment != null || httpOnlyFromComment) + { + addCookie(new HttpCookie( + cookie.getName(), + cookie.getValue(), + cookie.getDomain(), + cookie.getPath(), + cookie.getMaxAge(), + httpOnlyFromComment, + cookie.getSecure(), + comment, + cookie.getVersion(), + sameSiteFromComment)); + } + else + { + //new style cookie, everything is an attribute + addCookie(new HttpCookie( + cookie.getName(), + cookie.getValue(), + cookie.getVersion(), + cookie.getAttributes())); + } } public void addCookie(HttpCookie cookie) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/SessionHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/SessionHandler.java index b435084716a..5c1a0e32e55 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/SessionHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/SessionHandler.java @@ -20,8 +20,10 @@ import java.util.EventListener; import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.TreeMap; import java.util.concurrent.CopyOnWriteArrayList; import jakarta.servlet.ServletContext; @@ -37,6 +39,7 @@ import jakarta.servlet.http.HttpSessionIdListener; import jakarta.servlet.http.HttpSessionListener; import org.eclipse.jetty.ee10.servlet.ServletContextRequest.ServletApiRequest; import org.eclipse.jetty.http.HttpCookie; +import org.eclipse.jetty.http.HttpCookie.SameSite; import org.eclipse.jetty.http.Syntax; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; @@ -45,6 +48,7 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.session.AbstractSessionManager; import org.eclipse.jetty.session.Session; +import org.eclipse.jetty.util.StringUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -102,8 +106,6 @@ public class SessionHandler extends AbstractSessionManager implements Handler.Ne */ public final class CookieConfig implements SessionCookieConfig { - private final Map _attributes = new HashMap<>(); - @Override public String getComment() { @@ -125,21 +127,57 @@ public class SessionHandler extends AbstractSessionManager implements Handler.Ne @Override public void setAttribute(String name, String value) { - // TODO check that context is not available - _attributes.put(name, value); + checkState(); + String lcase = name.toLowerCase(Locale.ENGLISH); + + switch (lcase) + { + case "name" -> setName(value); + case "max-age" -> setMaxAge(value == null ? -1 : Integer.parseInt(value)); + case "comment" -> setComment(value); + case "domain" -> setDomain(value); + case "httponly" -> setHttpOnly(Boolean.valueOf(value)); + case "secure" -> setSecure(Boolean.valueOf(value)); + case "path" -> setPath(value); + default -> setSessionAttribute(name, value); + } } @Override public String getAttribute(String name) { - // TODO use these attributes - return _attributes.get(name); + String lcase = name.toLowerCase(Locale.ENGLISH); + return switch (lcase) + { + case "name" -> getName(); + case "max-age" -> Integer.toString(getMaxAge()); + case "comment" -> getComment(); + case "domain" -> getDomain(); + case "httponly" -> String.valueOf(isHttpOnly()); + case "secure" -> String.valueOf(isSecure()); + case "path" -> getPath(); + default -> getSessionAttribute(name); + }; } + /** + * According to the SessionCookieConfig javadoc, the attributes must also include + * all values set by explicit setters. + * @see SessionCookieConfig + */ @Override public Map getAttributes() { - return Collections.unmodifiableMap(_attributes); + Map specials = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + specials.put("name", getAttribute("name")); + specials.put("max-age", getAttribute("max-age")); + specials.put("comment", getAttribute("comment")); + specials.put("domain", getAttribute("domain")); + specials.put("httponly", getAttribute("httponly")); + specials.put("secure", getAttribute("secure")); + specials.put("path", getAttribute("path")); + specials.putAll(getSessionAttributes()); + return Collections.unmodifiableMap(specials); } @Override @@ -353,6 +391,60 @@ public class SessionHandler extends AbstractSessionManager implements Handler.Ne return apiRequest == null ? null : apiRequest.getCoreSession(); } + /** + * A session cookie is marked as secure IFF any of the following conditions are true: + *

    + *
  1. SessionCookieConfig.setSecure == true
    2. + *
  2. SessionCookieConfig.setSecure == false && _secureRequestOnly==true && request is HTTPS
    3. + *
+ * According to SessionCookieConfig javadoc, case 1 can be used when: + * "... even though the request that initiated the session came over HTTP, + * is to support a topology where the web container is front-ended by an + * SSL offloading load balancer. In this case, the traffic between the client + * and the load balancer will be over HTTPS, whereas the traffic between the + * load balancer and the web container will be over HTTP." + *

+ * For case 2, you can use _secureRequestOnly to determine if you want the + * Servlet Spec 3.0 default behavior when SessionCookieConfig.setSecure==false, + * which is: + * + * "they shall be marked as secure only if the request that initiated the + * corresponding session was also secure" + * + *

+ * The default for _secureRequestOnly is true, which gives the above behavior. If + * you set it to false, then a session cookie is NEVER marked as secure, even if + * the initiating request was secure. + * + * @param session the session to which the cookie should refer. + * @param contextPath the context to which the cookie should be linked. + * The client will only send the cookie value when requesting resources under this path. + * @param requestIsSecure whether the client is accessing the server over a secure protocol (i.e. HTTPS). + * @return if this SessionManager uses cookies, then this method will return a new + * {@link HttpCookie cookie object} that should be set on the client in order to link future HTTP requests + * with the session. If cookies are not in use, this method returns null. + */ + @Override + public HttpCookie getSessionCookie(Session session, String contextPath, boolean requestIsSecure) + { + if (isUsingCookies()) + { + String sessionPath = getSessionPath(); + sessionPath = (sessionPath == null) ? contextPath : sessionPath; + sessionPath = (StringUtil.isEmpty(sessionPath)) ? "/" : sessionPath; + return session.generateSetCookie((getSessionCookie() == null ? __DefaultSessionCookie : getSessionCookie()), + getSessionDomain(), + sessionPath, + getMaxCookieAge(), + isHttpOnly(), + isSecureCookies() || (isSecureRequestOnly() && requestIsSecure), + null, + 0, + getSessionAttributes()); + } + return null; + } + /** * Adds an event listener for session-related events. * @@ -588,7 +680,28 @@ public class SessionHandler extends AbstractSessionManager implements Handler.Ne return Collections.emptySet(); } - + + @Override + public HttpCookie.SameSite getSameSite() + { + String sameSite = getSessionAttribute("SameSite"); + if (sameSite == null) + return null; + return SameSite.valueOf(sameSite.toUpperCase(Locale.ENGLISH)); + } + + /** + * Set Session cookie sameSite mode. + * In ee10 this is set as a generic session cookie attribute. + * + * @param sameSite The sameSite setting for Session cookies (or null for no sameSite setting) + */ + @Override + public void setSameSite(HttpCookie.SameSite sameSite) + { + setSessionAttribute("SameSite", sameSite.getAttributeValue()); + } + public void setSessionTrackingModes(Set sessionTrackingModes) { if (sessionTrackingModes != null && diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SessionHandlerTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SessionHandlerTest.java index a31ff07b3c1..6eac739d3ae 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SessionHandlerTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SessionHandlerTest.java @@ -61,6 +61,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.greaterThan; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -459,6 +460,45 @@ public class SessionHandlerTest return ""; } } + + @Test + public void testSessionCookie() throws Exception + { + Server server = new Server(); + MockSessionIdManager idMgr = new MockSessionIdManager(server); + idMgr.setWorkerName("node1"); + SessionHandler mgr = new SessionHandler(); + MockSessionCache cache = new MockSessionCache(mgr); + cache.setSessionDataStore(new NullSessionDataStore()); + mgr.setSessionCache(cache); + mgr.setSessionIdManager(idMgr); + + long now = TimeUnit.NANOSECONDS.toMillis(System.nanoTime()); + + Session session = new Session(mgr, new SessionData("123", "_foo", "0.0.0.0", now, now, now, 30)); + session.setExtendedId("123.node1"); + SessionCookieConfig sessionCookieConfig = mgr.getSessionCookieConfig(); + sessionCookieConfig.setName("SPECIAL"); + sessionCookieConfig.setDomain("universe"); + sessionCookieConfig.setHttpOnly(false); + sessionCookieConfig.setSecure(false); + sessionCookieConfig.setPath("/foo"); + sessionCookieConfig.setMaxAge(99); + sessionCookieConfig.setAttribute("SameSite", "Strict"); + sessionCookieConfig.setAttribute("ham", "cheese"); + + HttpCookie cookie = mgr.getSessionCookie(session, "/bar", false); + assertEquals("SPECIAL", cookie.getName()); + assertEquals("universe", cookie.getDomain()); + assertEquals("/foo", cookie.getPath()); + assertFalse(cookie.isHttpOnly()); + assertFalse(cookie.isSecure()); + assertEquals(99, cookie.getMaxAge()); + assertEquals(HttpCookie.SameSite.STRICT, cookie.getSameSite()); + + String cookieStr = cookie.getRFC6265SetCookie(); + assertThat(cookieStr, containsString("; SameSite=Strict; ham=cheese")); + } @Test public void testSecureSessionCookie() throws Exception diff --git a/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/StandardDescriptorProcessor.java b/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/StandardDescriptorProcessor.java index efd77d9b83f..e1ca8a0b9e2 100644 --- a/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/StandardDescriptorProcessor.java +++ b/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/StandardDescriptorProcessor.java @@ -45,6 +45,7 @@ import org.eclipse.jetty.ee10.servlet.security.authentication.FormAuthenticator; import org.eclipse.jetty.http.pathmap.ServletPathSpec; import org.eclipse.jetty.util.ArrayUtil; import org.eclipse.jetty.util.Loader; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.security.Constraint; import org.eclipse.jetty.xml.XmlParser; import org.eclipse.jetty.xml.XmlParser.Node; @@ -715,274 +716,103 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor String name = cookieConfig.getString("name", false, true); if (name != null) { - Origin origin = context.getMetaData().getOrigin("cookie-config.name"); - switch (origin) - { - case NotSet: - { - //no set yet, accept it - context.getSessionHandler().getSessionCookieConfig().setName(name); - context.getMetaData().setOrigin("cookie-config.name", descriptor); - break; - } - case WebXml: - case WebDefaults: - case WebOverride: - { - // set in a web xml, only allow web-default/web-override to change - if (!(descriptor instanceof FragmentDescriptor)) - { - context.getSessionHandler().getSessionCookieConfig().setName(name); - context.getMetaData().setOrigin("cookie-config.name", descriptor); - } - break; - } - case WebFragment: - { - //a web-fragment set the value, all web-fragments must have the same value - //TODO: evaluate that is can never be null?! - if (!name.equals(context.getSessionHandler().getSessionCookieConfig().getName())) - throw new IllegalStateException("Conflicting cookie-config name " + name + " in " + descriptor.getResource()); - break; - } - default: - unknownOrigin(origin); - } + addSessionConfigAttribute(context, descriptor, "name", name); } // String domain = cookieConfig.getString("domain", false, true); if (domain != null) { - Origin origin = context.getMetaData().getOrigin("cookie-config.domain"); - switch (origin) - { - case NotSet: - { - //no set yet, accept it - context.getSessionHandler().getSessionCookieConfig().setDomain(domain); - context.getMetaData().setOrigin("cookie-config.domain", descriptor); - break; - } - case WebXml: - case WebDefaults: - case WebOverride: - { - // set in a web xml, only allow web-default/web-override to change - if (!(descriptor instanceof FragmentDescriptor)) - { - context.getSessionHandler().getSessionCookieConfig().setDomain(domain); - context.getMetaData().setOrigin("cookie-config.domain", descriptor); - } - break; - } - case WebFragment: - { - //a web-fragment set the value, all web-fragments must have the same value - if (!context.getSessionHandler().getSessionCookieConfig().getDomain().equals(domain)) - throw new IllegalStateException("Conflicting cookie-config domain " + domain + " in " + descriptor.getResource()); - break; - } - default: - unknownOrigin(origin); - } + addSessionConfigAttribute(context, descriptor, "domain", domain); } // String path = cookieConfig.getString("path", false, true); if (path != null) { - Origin origin = context.getMetaData().getOrigin("cookie-config.path"); - switch (origin) - { - case NotSet: - { - //no set yet, accept it - context.getSessionHandler().getSessionCookieConfig().setPath(path); - context.getMetaData().setOrigin("cookie-config.path", descriptor); - break; - } - case WebXml: - case WebDefaults: - case WebOverride: - { - // set in a web xml, only allow web-default/web-override to change - if (!(descriptor instanceof FragmentDescriptor)) - { - context.getSessionHandler().getSessionCookieConfig().setPath(path); - context.getMetaData().setOrigin("cookie-config.path", descriptor); - } - break; - } - case WebFragment: - { - //a web-fragment set the value, all web-fragments must have the same value - if (!path.equals(context.getSessionHandler().getSessionCookieConfig().getPath())) - throw new IllegalStateException("Conflicting cookie-config path " + path + " in " + descriptor.getResource()); - break; - } - default: - unknownOrigin(origin); - } + addSessionConfigAttribute(context, descriptor, "path", path); } // String comment = cookieConfig.getString("comment", false, true); if (comment != null) { - Origin origin = context.getMetaData().getOrigin("cookie-config.comment"); - switch (origin) - { - case NotSet: - { - //no set yet, accept it - context.getSessionHandler().getSessionCookieConfig().setComment(comment); - context.getMetaData().setOrigin("cookie-config.comment", descriptor); - break; - } - case WebXml: - case WebDefaults: - case WebOverride: - { - // set in a web xml, only allow web-default/web-override to change - if (!(descriptor instanceof FragmentDescriptor)) - { - context.getSessionHandler().getSessionCookieConfig().setComment(comment); - context.getMetaData().setOrigin("cookie-config.comment", descriptor); - } - break; - } - case WebFragment: - { - //a web-fragment set the value, all web-fragments must have the same value - if (!context.getSessionHandler().getSessionCookieConfig().getComment().equals(comment)) - throw new IllegalStateException("Conflicting cookie-config comment " + comment + " in " + descriptor.getResource()); - break; - } - default: - unknownOrigin(origin); - } + addSessionConfigAttribute(context, descriptor, "comment", comment); } // true/false tNode = cookieConfig.get("http-only"); if (tNode != null) { - boolean httpOnly = Boolean.parseBoolean(tNode.toString(false, true)); - Origin origin = context.getMetaData().getOrigin("cookie-config.http-only"); - switch (origin) - { - case NotSet: - { - //no set yet, accept it - context.getSessionHandler().getSessionCookieConfig().setHttpOnly(httpOnly); - context.getMetaData().setOrigin("cookie-config.http-only", descriptor); - break; - } - case WebXml: - case WebDefaults: - case WebOverride: - { - // set in a web xml, only allow web-default/web-override to change - if (!(descriptor instanceof FragmentDescriptor)) - { - context.getSessionHandler().getSessionCookieConfig().setHttpOnly(httpOnly); - context.getMetaData().setOrigin("cookie-config.http-only", descriptor); - } - break; - } - case WebFragment: - { - //a web-fragment set the value, all web-fragments must have the same value - if (context.getSessionHandler().getSessionCookieConfig().isHttpOnly() != httpOnly) - throw new IllegalStateException("Conflicting cookie-config http-only " + httpOnly + " in " + descriptor.getResource()); - break; - } - default: - unknownOrigin(origin); - } + //TODO: note that this is not http-only + addSessionConfigAttribute(context, descriptor, "HttpOnly", tNode.toString(false, true)); } // true/false tNode = cookieConfig.get("secure"); if (tNode != null) { - boolean secure = Boolean.parseBoolean(tNode.toString(false, true)); - Origin origin = context.getMetaData().getOrigin("cookie-config.secure"); - switch (origin) - { - case NotSet: - { - //no set yet, accept it - context.getSessionHandler().getSessionCookieConfig().setSecure(secure); - context.getMetaData().setOrigin("cookie-config.secure", descriptor); - break; - } - case WebXml: - case WebDefaults: - case WebOverride: - { - // set in a web xml, only allow web-default/web-override to change - if (!(descriptor instanceof FragmentDescriptor)) - { - context.getSessionHandler().getSessionCookieConfig().setSecure(secure); - context.getMetaData().setOrigin("cookie-config.secure", descriptor); - } - break; - } - case WebFragment: - { - //a web-fragment set the value, all web-fragments must have the same value - if (context.getSessionHandler().getSessionCookieConfig().isSecure() != secure) - throw new IllegalStateException("Conflicting cookie-config secure " + secure + " in " + descriptor.getResource()); - break; - } - default: - unknownOrigin(origin); - } + addSessionConfigAttribute(context, descriptor, "secure", tNode.toString(false, true)); } // tNode = cookieConfig.get("max-age"); if (tNode != null) { - int maxAge = Integer.parseInt(tNode.toString(false, true)); - Origin origin = context.getMetaData().getOrigin("cookie-config.max-age"); - switch (origin) - { - case NotSet: - { - //no set yet, accept it - context.getSessionHandler().getSessionCookieConfig().setMaxAge(maxAge); - context.getMetaData().setOrigin("cookie-config.max-age", descriptor); - break; - } - case WebXml: - case WebDefaults: - case WebOverride: - { - // set in a web xml, only allow web-default/web-override to change - if (!(descriptor instanceof FragmentDescriptor)) - { - context.getSessionHandler().getSessionCookieConfig().setMaxAge(maxAge); - context.getMetaData().setOrigin("cookie-config.max-age", descriptor); - } - break; - } - case WebFragment: - { - //a web-fragment set the value, all web-fragments must have the same value - if (context.getSessionHandler().getSessionCookieConfig().getMaxAge() != maxAge) - throw new IllegalStateException("Conflicting cookie-config max-age " + maxAge + " in " + descriptor.getResource()); - break; - } - default: - unknownOrigin(origin); - } + addSessionConfigAttribute(context, descriptor, "max-age", tNode.toString(false, true)); + } + + Iterator attributes = cookieConfig.iterator("attribute"); + while (attributes.hasNext()) + { + XmlParser.Node attribute = attributes.next(); + String aname = attribute.getString("attribute-name", false, true); + String avalue = attribute.getString("attribute-value", false, true); + addSessionConfigAttribute(context, descriptor, aname, avalue); } } } + public void addSessionConfigAttribute(WebAppContext context, Descriptor descriptor, String name, String value) + { + if (StringUtil.isBlank(name)) + return; + + Origin origin = context.getMetaData().getOrigin("cookie-config.attribute." + name); + switch (origin) + { + case NotSet: + { + //no with attribute of that name set yet, accept it. + //if it is the max-age attribute, it must be set as an integer + context.getSessionHandler().getSessionCookieConfig().setAttribute(name, value); + context.getMetaData().setOrigin("cookie-config.attribute." + name, descriptor); + break; + } + case WebXml: + case WebDefaults: + case WebOverride: + { + // with attribute of that name set in a web xml, only allow web-default/web-override to change + if (!(descriptor instanceof FragmentDescriptor)) + { + context.getSessionHandler().getSessionCookieConfig().setAttribute(name, value); + context.getMetaData().setOrigin("cookie-config.attribute." + name, descriptor); + } + break; + } + case WebFragment: + { + //a web-fragment set an attribute of the same name, all web-fragments must have the same value + if (!StringUtil.nonNull(value).equals(StringUtil.nonNull(context.getSessionHandler().getSessionCookieConfig().getAttribute(name)))) + throw new IllegalStateException("Conflicting attribute " + name + "=" + value + " in " + descriptor.getResource()); + break; + } + default: + unknownOrigin(origin); + } + } + public void visitMimeMapping(WebAppContext context, Descriptor descriptor, XmlParser.Node node) { String extension = node.getString("extension", false, true); diff --git a/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/StandardDescriptorProcessorTest.java b/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/StandardDescriptorProcessorTest.java index 4a7cb7564fc..801189d3e4b 100644 --- a/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/StandardDescriptorProcessorTest.java +++ b/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/StandardDescriptorProcessorTest.java @@ -14,6 +14,8 @@ package org.eclipse.jetty.ee10.webapp; import java.io.File; +import java.util.Arrays; +import java.util.Map; import java.util.concurrent.TimeUnit; import org.eclipse.jetty.server.Server; @@ -24,7 +26,10 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.equalToIgnoringCase; +import static org.hamcrest.Matchers.not; import static org.junit.jupiter.api.Assertions.assertEquals; public class StandardDescriptorProcessorTest @@ -57,5 +62,64 @@ public class StandardDescriptorProcessorTest wac.setDescriptor(webXml.toURI().toURL().toString()); wac.start(); assertEquals(54, TimeUnit.SECONDS.toMinutes(wac.getSessionHandler().getMaxInactiveInterval())); + + //test the CookieConfig attributes and getters, and the getters on SessionHandler + //name + assertEquals("SPECIALSESSIONID", wac.getSessionHandler().getSessionCookieConfig().getName()); + assertEquals("SPECIALSESSIONID", wac.getSessionHandler().getSessionCookieConfig().getAttribute("Name")); + assertEquals("SPECIALSESSIONID", wac.getSessionHandler().getSessionCookie()); + + //comment + assertEquals("nocomment", wac.getSessionHandler().getSessionCookieConfig().getComment()); + assertEquals("nocomment", wac.getSessionHandler().getSessionCookieConfig().getAttribute("Comment")); + assertEquals("nocomment", wac.getSessionHandler().getSessionComment()); + + //domain + assertEquals("universe", wac.getSessionHandler().getSessionCookieConfig().getDomain()); + assertEquals("universe", wac.getSessionHandler().getSessionCookieConfig().getAttribute("Domain")); + assertEquals("universe", wac.getSessionHandler().getSessionDomain()); + + //path + assertEquals("foo", wac.getSessionHandler().getSessionCookieConfig().getPath()); + assertEquals("foo", wac.getSessionHandler().getSessionCookieConfig().getAttribute("Path")); + assertEquals("foo", wac.getSessionHandler().getSessionPath()); + + //max-age + assertEquals(10, wac.getSessionHandler().getSessionCookieConfig().getMaxAge()); + assertEquals("10", wac.getSessionHandler().getSessionCookieConfig().getAttribute("Max-Age")); + assertEquals(10, wac.getSessionHandler().getMaxCookieAge()); + + //secure + assertEquals(false, wac.getSessionHandler().getSessionCookieConfig().isSecure()); + assertEquals("false", wac.getSessionHandler().getSessionCookieConfig().getAttribute("Secure")); + assertEquals(false, wac.getSessionHandler().isSecureCookies()); + + //httponly + assertEquals(false, wac.getSessionHandler().getSessionCookieConfig().isHttpOnly()); + assertEquals("false", wac.getSessionHandler().getSessionCookieConfig().getAttribute("HttpOnly")); + assertEquals(false, wac.getSessionHandler().isHttpOnly()); + + Map attributes = wac.getSessionHandler().getSessionCookieConfig().getAttributes(); + + //SessionCookieConfig javadoc states that all setters must be also represented as attributes + assertThat(wac.getSessionHandler().getSessionCookieConfig().getAttributes().keySet(), + containsInAnyOrder(Arrays.asList( + equalToIgnoringCase("name"), + equalToIgnoringCase("comment"), + equalToIgnoringCase("domain"), + equalToIgnoringCase("path"), + equalToIgnoringCase("max-age"), + equalToIgnoringCase("secure"), + equalToIgnoringCase("httponly"), + equalToIgnoringCase("length"), + equalToIgnoringCase("width"), + equalToIgnoringCase("SameSite")))); + + //test the attributes on SessionHandler do NOT contain the well-known ones of Name, Comment, Domain etc etc + assertThat(wac.getSessionHandler().getSessionAttributes().keySet(), + containsInAnyOrder(Arrays.asList( + equalToIgnoringCase("length"), + equalToIgnoringCase("width"), + equalToIgnoringCase("SameSite")))); } } diff --git a/jetty-ee10/jetty-ee10-webapp/src/test/resources/web-session-config.xml b/jetty-ee10/jetty-ee10-webapp/src/test/resources/web-session-config.xml index 594178a34da..e1c8100208d 100644 --- a/jetty-ee10/jetty-ee10-webapp/src/test/resources/web-session-config.xml +++ b/jetty-ee10/jetty-ee10-webapp/src/test/resources/web-session-config.xml @@ -6,6 +6,27 @@ Test 4 WebApp + + universe + SPECIALSESSIONID + foo + 10 + nocomment + false + false + + length + short + + + width + long + + + SameSite + Strict + + 54 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 index 8f964aa679d..636bf977137 100644 --- 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 @@ -306,14 +306,28 @@ public class Request implements HttpServletRequest HttpHeader header = field.getHeader(); if (header == HttpHeader.SET_COOKIE) { - HttpCookie cookie = (field instanceof SetCookieHttpField) - ? ((SetCookieHttpField)field).getHttpCookie() - : new HttpCookie(field.getValue()); - - if (cookie.getMaxAge() > 0) - cookies.put(cookie.getName(), cookie.getValue()); + String cookieName; + String cookieValue; + long cookieMaxAge; + if (field instanceof SetCookieHttpField) + { + HttpCookie cookie = ((SetCookieHttpField)field).getHttpCookie(); + cookieName = cookie.getName(); + cookieValue = cookie.getValue(); + cookieMaxAge = cookie.getMaxAge(); + } else - cookies.remove(cookie.getName()); + { + Map cookieFields = HttpCookie.extractBasics(field.getValue()); + cookieName = cookieFields.get("name"); + cookieValue = cookieFields.get("value"); + cookieMaxAge = cookieFields.get("max-age") != null ? Long.valueOf(cookieFields.get("max-age")) : -1; + } + + if (cookieMaxAge > 0) + cookies.put(cookieName, cookieValue); + else + cookies.remove(cookieName); } } diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java index 6155043c0c5..02e87c3be3f 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java @@ -299,31 +299,17 @@ public class Response implements HttpServletResponse if (field.getHeader() == HttpHeader.SET_COOKIE) { CookieCompliance compliance = getHttpChannel().getHttpConfiguration().getResponseCookieCompliance(); - - HttpCookie oldCookie; - if (field instanceof SetCookieHttpField) - oldCookie = ((SetCookieHttpField)field).getHttpCookie(); + + if (field instanceof HttpCookie.SetCookieHttpField) + { + if (!HttpCookie.match(((HttpCookie.SetCookieHttpField)field).getHttpCookie(), cookie.getName(), cookie.getDomain(), cookie.getPath())) + continue; + } else - oldCookie = new HttpCookie(field.getValue()); - - if (!cookie.getName().equals(oldCookie.getName())) - continue; - - if (cookie.getDomain() == null) { - if (oldCookie.getDomain() != null) + if (!HttpCookie.match(field.getValue(), cookie.getName(), cookie.getDomain(), cookie.getPath())) continue; } - else if (!cookie.getDomain().equalsIgnoreCase(oldCookie.getDomain())) - continue; - - if (cookie.getPath() == null) - { - if (oldCookie.getPath() != null) - continue; - } - else if (!cookie.getPath().equals(oldCookie.getPath())) - continue; i.set(new SetCookieHttpField(checkSameSite(cookie), compliance)); return; diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SessionHandler.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SessionHandler.java index 997b3d5dfaf..4fbf9a73d86 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SessionHandler.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SessionHandler.java @@ -19,6 +19,7 @@ import java.util.EnumSet; import java.util.Enumeration; import java.util.EventListener; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; @@ -39,6 +40,7 @@ import jakarta.servlet.http.HttpSessionEvent; import jakarta.servlet.http.HttpSessionIdListener; import jakarta.servlet.http.HttpSessionListener; import org.eclipse.jetty.http.HttpCookie; +import org.eclipse.jetty.http.HttpCookie.SameSite; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.session.AbstractSessionManager; import org.eclipse.jetty.session.Session; @@ -46,6 +48,7 @@ import org.eclipse.jetty.session.SessionCache; import org.eclipse.jetty.session.SessionConfig; import org.eclipse.jetty.session.SessionIdManager; import org.eclipse.jetty.session.SessionManager; +import org.eclipse.jetty.util.StringUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -567,6 +570,81 @@ public class SessionHandler extends ScopedHandler implements SessionConfig.Mutab return super.resolveRequestedSessionId(request); } + @Override + public HttpCookie.SameSite getSameSite() + { + return HttpCookie.getSameSiteFromComment(getSessionComment()); + } + + /** + * Set Session cookie sameSite mode. + * + * @param sameSite The sameSite setting for Session cookies (or null for no sameSite setting) + */ + @Override + public void setSameSite(HttpCookie.SameSite sameSite) + { + setSessionComment(HttpCookie.getCommentWithAttributes(getSessionComment(), false, sameSite)); + } + + /** + * A session cookie is marked as secure IFF any of the following conditions are true: + *

    + *
  1. SessionCookieConfig.setSecure == true
    2. + *
  2. SessionCookieConfig.setSecure == false && _secureRequestOnly==true && request is HTTPS
    3. + *
+ * According to SessionCookieConfig javadoc, case 1 can be used when: + * "... even though the request that initiated the session came over HTTP, + * is to support a topology where the web container is front-ended by an + * SSL offloading load balancer. In this case, the traffic between the client + * and the load balancer will be over HTTPS, whereas the traffic between the + * load balancer and the web container will be over HTTP." + *

+ * For case 2, you can use _secureRequestOnly to determine if you want the + * Servlet Spec 3.0 default behavior when SessionCookieConfig.setSecure==false, + * which is: + * + * "they shall be marked as secure only if the request that initiated the + * corresponding session was also secure" + * + *

+ * The default for _secureRequestOnly is true, which gives the above behavior. If + * you set it to false, then a session cookie is NEVER marked as secure, even if + * the initiating request was secure. + * + * @param session the session to which the cookie should refer. + * @param contextPath the context to which the cookie should be linked. + * The client will only send the cookie value when requesting resources under this path. + * @param requestIsSecure whether the client is accessing the server over a secure protocol (i.e. HTTPS). + * @return if this SessionManager uses cookies, then this method will return a new + * {@link HttpCookie cookie object} that should be set on the client in order to link future HTTP requests + * with the session. If cookies are not in use, this method returns null. + */ + @Override + public HttpCookie getSessionCookie(Session session, String contextPath, boolean requestIsSecure) + { + if (isUsingCookies()) + { + String sessionPath = getSessionPath(); + sessionPath = (sessionPath == null) ? contextPath : sessionPath; + sessionPath = (StringUtil.isEmpty(sessionPath)) ? "/" : sessionPath; + SameSite sameSite = HttpCookie.getSameSiteFromComment(getSessionComment()); + Map attributes = Collections.emptyMap(); + if (sameSite != null) + attributes = Collections.singletonMap("SameSite", sameSite.getAttributeValue()); + return session.generateSetCookie((getSessionCookie() == null ? __DefaultSessionCookie : getSessionCookie()), + getSessionDomain(), + sessionPath, + getMaxCookieAge(), + isHttpOnly(), + isSecureCookies() || (isSecureRequestOnly() && requestIsSecure), + HttpCookie.getCommentWithoutAttributes(getSessionComment()), + 0, + attributes); + } + return null; + } + @Override public void callSessionAttributeListeners(Session session, String name, Object old, Object value) { diff --git a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/SessionHandlerTest.java b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/SessionHandlerTest.java index 52228da9864..bd4fe6c9d1b 100644 --- a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/SessionHandlerTest.java +++ b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/SessionHandlerTest.java @@ -19,19 +19,27 @@ import java.util.Arrays; import java.util.Collections; import java.util.Enumeration; import java.util.HashSet; +import java.util.concurrent.TimeUnit; +import java.util.function.Function; +import jakarta.servlet.SessionCookieConfig; import jakarta.servlet.SessionTrackingMode; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.http.HttpSession; import jakarta.servlet.http.HttpSessionEvent; import jakarta.servlet.http.HttpSessionListener; +import org.eclipse.jetty.http.HttpCookie; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.session.AbstractSessionCache; +import org.eclipse.jetty.session.DefaultSessionIdManager; +import org.eclipse.jetty.session.NullSessionDataStore; import org.eclipse.jetty.session.Session; import org.eclipse.jetty.session.SessionData; +import org.eclipse.jetty.session.SessionManager; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -43,6 +51,7 @@ import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; public class SessionHandlerTest @@ -147,6 +156,47 @@ public class SessionHandlerTest { _server.stop(); } + + @Test + public void testSessionCookie() 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 = TimeUnit.NANOSECONDS.toMillis(System.nanoTime()); + + Session session = new Session(mgr.getSessionManager(), new SessionData("123", "_foo", "0.0.0.0", now, now, now, 30)); + session.setExtendedId("123.node1"); + SessionCookieConfig sessionCookieConfig = mgr.getSessionCookieConfig(); + sessionCookieConfig.setName("SPECIAL"); + sessionCookieConfig.setDomain("universe"); + sessionCookieConfig.setHttpOnly(false); + sessionCookieConfig.setSecure(false); + sessionCookieConfig.setPath("/foo"); + sessionCookieConfig.setMaxAge(99); + + //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 + mgr.setSameSite(HttpCookie.SameSite.STRICT); + + HttpCookie cookie = mgr.getSessionManager().getSessionCookie(session, "/bar", false); + assertEquals("SPECIAL", cookie.getName()); + assertEquals("universe", cookie.getDomain()); + assertEquals("/foo", cookie.getPath()); + assertFalse(cookie.isHttpOnly()); + assertFalse(cookie.isSecure()); + assertEquals(99, cookie.getMaxAge()); + assertEquals(HttpCookie.SameSite.STRICT, cookie.getSameSite()); + + String cookieStr = cookie.getRFC6265SetCookie(); + assertThat(cookieStr, containsString("; SameSite=Strict")); + } @Test public void testSessionTrackingMode() @@ -370,4 +420,81 @@ public class SessionHandlerTest assertThat(content, containsString("Session=" + id.substring(0, id.indexOf(".node0")))); assertThat(content, containsString("attribute = value")); } + + public class MockSessionCache extends AbstractSessionCache + { + + public MockSessionCache(SessionManager manager) + { + super(manager); + } + + @Override + public void shutdown() + { + } + + @Override + public Session doGet(String key) + { + return null; + } + + @Override + public Session doPutIfAbsent(String key, Session session) + { + return null; + } + + @Override + public Session doDelete(String key) + { + return null; + } + + @Override + public boolean doReplace(String id, Session oldValue, Session newValue) + { + return false; + } + + @Override + public Session newSession(SessionData data) + { + return null; + } + + @Override + protected Session doComputeIfAbsent(String id, Function mappingFunction) + { + return mappingFunction.apply(id); + } + } + + public class MockSessionIdManager extends DefaultSessionIdManager + { + public MockSessionIdManager(Server server) + { + super(server); + } + + @Override + public boolean isIdInUse(String id) + { + return false; + } + + @Override + public void expireAll(String id) + { + + } + + @Override + public String renewSessionId(String oldClusterId, String oldNodeId, org.eclipse.jetty.server.Request request) + { + return ""; + } + } + } diff --git a/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/StandardDescriptorProcessorTest.java b/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/StandardDescriptorProcessorTest.java index 1a6a85478ad..79c71ce19d0 100644 --- a/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/StandardDescriptorProcessorTest.java +++ b/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/StandardDescriptorProcessorTest.java @@ -57,5 +57,34 @@ public class StandardDescriptorProcessorTest wac.setDescriptor(webXml.toURI().toURL().toString()); wac.start(); assertEquals(54, TimeUnit.SECONDS.toMinutes(wac.getSessionHandler().getMaxInactiveInterval())); + + //test the CookieConfig attributes and getters, and the getters on SessionHandler + //name + assertEquals("SPECIALSESSIONID", wac.getSessionHandler().getSessionCookieConfig().getName()); + assertEquals("SPECIALSESSIONID", wac.getSessionHandler().getSessionCookie()); + + //comment + assertEquals("nocomment", wac.getSessionHandler().getSessionCookieConfig().getComment()); + assertEquals("nocomment", wac.getSessionHandler().getSessionComment()); + + //domain + assertEquals("universe", wac.getSessionHandler().getSessionCookieConfig().getDomain()); + assertEquals("universe", wac.getSessionHandler().getSessionDomain()); + + //path + assertEquals("foo", wac.getSessionHandler().getSessionCookieConfig().getPath()); + assertEquals("foo", wac.getSessionHandler().getSessionPath()); + + //max-age + assertEquals(10, wac.getSessionHandler().getSessionCookieConfig().getMaxAge()); + assertEquals(10, wac.getSessionHandler().getMaxCookieAge()); + + //secure + assertEquals(false, wac.getSessionHandler().getSessionCookieConfig().isSecure()); + assertEquals(false, wac.getSessionHandler().isSecureCookies()); + + //httponly + assertEquals(false, wac.getSessionHandler().getSessionCookieConfig().isHttpOnly()); + assertEquals(false, wac.getSessionHandler().isHttpOnly()); } } diff --git a/jetty-ee9/jetty-ee9-webapp/src/test/resources/web-session-config.xml b/jetty-ee9/jetty-ee9-webapp/src/test/resources/web-session-config.xml index 5b54e225ae1..d6cc330ce7c 100644 --- a/jetty-ee9/jetty-ee9-webapp/src/test/resources/web-session-config.xml +++ b/jetty-ee9/jetty-ee9-webapp/src/test/resources/web-session-config.xml @@ -6,7 +6,15 @@ Test 4 WebApp + + universe + SPECIALSESSIONID + foo + 10 + nocomment + false + false + 54 -