This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push: new 6b8aeaf Add support for generic cookie attributes to SessionCookieConfig 6b8aeaf is described below commit 6b8aeaf80860dd48d5ddd44e866fea5d6072ded3 Author: Mark Thomas <ma...@apache.org> AuthorDate: Tue Oct 12 09:38:53 2021 +0100 Add support for generic cookie attributes to SessionCookieConfig --- java/jakarta/servlet/SessionCookieConfig.java | 46 +++++++ java/jakarta/servlet/resources/web-common_6_0.xsd | 57 ++++++++ .../core/ApplicationSessionCookieConfig.java | 87 +++++++++--- .../org/apache/catalina/startup/ContextConfig.java | 17 +-- .../tomcat/util/descriptor/web/Constants.java | 8 ++ .../util/descriptor/web/LocalStrings.properties | 7 +- .../tomcat/util/descriptor/web/SessionConfig.java | 57 +++++--- .../tomcat/util/descriptor/web/WebRuleSet.java | 4 + .../apache/tomcat/util/descriptor/web/WebXml.java | 149 +++------------------ .../tomcat/util/http/LegacyCookieProcessor.java | 38 +++++- .../tomcat/util/http/LocalStrings.properties | 2 + .../tomcat/util/http/Rfc6265CookieProcessor.java | 59 +++++++- test/jakarta/servlet/TestSessionCookieConfig.java | 56 ++++++++ .../valves/TestLoadBalancerDrainingValve.java | 60 ++++++--- .../tomcat/unittest/TesterSessionCookieConfig.java | 17 +++ .../tomcat/util/descriptor/web/TestWebXml.java | 90 +++++++++++++ test/webapp/WEB-INF/web.xml | 8 ++ test/webapp/jsp/session.jsp | 18 +++ webapps/docs/changelog.xml | 5 + 19 files changed, 576 insertions(+), 209 deletions(-) diff --git a/java/jakarta/servlet/SessionCookieConfig.java b/java/jakarta/servlet/SessionCookieConfig.java index 936e49d..6246d02 100644 --- a/java/jakarta/servlet/SessionCookieConfig.java +++ b/java/jakarta/servlet/SessionCookieConfig.java @@ -16,6 +16,8 @@ */ package jakarta.servlet; +import java.util.Map; + /** * Configures the session cookies used by the web application associated with * the ServletContext from which this SessionCookieConfig was obtained. @@ -142,4 +144,48 @@ public interface SessionCookieConfig { * @return the maximum age in seconds */ public int getMaxAge(); + + /** + * Sets the value for the given session cookie attribute. When a value is + * set via this method, the value returned by the attribute specific getter + * (if any) must be consistent with the value set via this method. + * + * @param name Name of attribute to set + * @param value Value of attribute + * + * @throws IllegalStateException if the associated ServletContext has + * already been initialised + * + * @throws IllegalArgumentException If the attribute name is null or + * contains any characters not permitted for use in Cookie names. + * + * @throws NumberFormatException If the attribute is known to be numerical + * but the provided value cannot be parsed to a number. + * + * @since Servlet 6.0 + */ + public void setAttribute(String name, String value); + + /** + * Obtain the value for a sesison cookie given attribute. Values returned + * from this method must be consistent with the values set and returned by + * the attribute specific getters and setters in this class. + * + * @param name Name of attribute to return + * + * @return Value of specified attribute + * + * @since Servlet 6.0 + */ + public String getAttribute(String name); + + /** + * Obtain the Map of attributes and values (excluding version) for this + * session cookie. + * + * @return A read-only Map of attributes to values, excluding version. + * + * @since Servlet 6.0 + */ + public Map<String,String> getAttributes(); } diff --git a/java/jakarta/servlet/resources/web-common_6_0.xsd b/java/jakarta/servlet/resources/web-common_6_0.xsd index c0e3dd8..fae4e41 100644 --- a/java/jakarta/servlet/resources/web-common_6_0.xsd +++ b/java/jakarta/servlet/resources/web-common_6_0.xsd @@ -167,6 +167,50 @@ <!-- **************************************************** --> + <xsd:complexType name="attribute-valueType"> + <xsd:annotation> + <xsd:documentation> + + This type is a general type that can be used to declare + attribute/value lists. + + </xsd:documentation> + </xsd:annotation> + <xsd:sequence> + <xsd:element name="description" + type="jakartaee:descriptionType" + minOccurs="0" + maxOccurs="unbounded"/> + <xsd:element name="attribute-name" + type="jakartaee:string"> + <xsd:annotation> + <xsd:documentation> + + The attribute-name element contains the name of an + attribute. + + </xsd:documentation> + </xsd:annotation> + </xsd:element> + <xsd:element name="attribute-value" + type="jakartaee:xsdStringType"> + <xsd:annotation> + <xsd:documentation> + + The attribute-value element contains the value of a + attribute. + + </xsd:documentation> + </xsd:annotation> + </xsd:element> + </xsd:sequence> + <xsd:attribute name="id" + type="xsd:ID"/> + </xsd:complexType> + + +<!-- **************************************************** --> + <xsd:complexType name="auth-constraintType"> <xsd:annotation> <xsd:documentation> @@ -985,6 +1029,19 @@ </xsd:documentation> </xsd:annotation> </xsd:element> + <xsd:element name="attribute" + type="jakartaee:attribute-valueType" + minOccurs="0" + maxOccurs="unbounded"> + <xsd:annotation> + <xsd:documentation> + + The attribute-param element contains a name/value pair to + be added as an attribute to every session cookie. + + </xsd:documentation> + </xsd:annotation> + </xsd:element> </xsd:sequence> <xsd:attribute name="id" type="xsd:ID"/> diff --git a/java/org/apache/catalina/core/ApplicationSessionCookieConfig.java b/java/org/apache/catalina/core/ApplicationSessionCookieConfig.java index 5793d92..aaf7a13 100644 --- a/java/org/apache/catalina/core/ApplicationSessionCookieConfig.java +++ b/java/org/apache/catalina/core/ApplicationSessionCookieConfig.java @@ -16,12 +16,17 @@ */ package org.apache.catalina.core; +import java.util.Collections; +import java.util.Map; +import java.util.TreeMap; + import jakarta.servlet.SessionCookieConfig; import jakarta.servlet.http.Cookie; import org.apache.catalina.Context; import org.apache.catalina.LifecycleState; import org.apache.catalina.util.SessionConfig; +import org.apache.tomcat.util.descriptor.web.Constants; import org.apache.tomcat.util.res.StringManager; public class ApplicationSessionCookieConfig implements SessionCookieConfig { @@ -31,13 +36,13 @@ public class ApplicationSessionCookieConfig implements SessionCookieConfig { */ private static final StringManager sm = StringManager.getManager(ApplicationSessionCookieConfig.class); - private boolean httpOnly; - private boolean secure; - private int maxAge = -1; - private String comment; - private String domain; + private static final int DEFAULT_MAX_AGE = -1; + private static final boolean DEFAULT_HTTP_ONLY = false; + private static final boolean DEFAULT_SECURE = false; + + private final Map<String,String> attributes = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + private String name; - private String path; private StandardContext context; public ApplicationSessionCookieConfig(StandardContext context) { @@ -46,17 +51,21 @@ public class ApplicationSessionCookieConfig implements SessionCookieConfig { @Override public String getComment() { - return comment; + return getAttribute(Constants.COOKIE_COMMENT_ATTR); } @Override public String getDomain() { - return domain; + return getAttribute(Constants.COOKIE_DOMAIN_ATTR); } @Override public int getMaxAge() { - return maxAge; + String maxAge = getAttribute(Constants.COOKIE_MAX_AGE_ATTR); + if (maxAge == null) { + return DEFAULT_MAX_AGE; + } + return Integer.parseInt(maxAge); } @Override @@ -66,17 +75,25 @@ public class ApplicationSessionCookieConfig implements SessionCookieConfig { @Override public String getPath() { - return path; + return getAttribute(Constants.COOKIE_PATH_ATTR); } @Override public boolean isHttpOnly() { - return httpOnly; + String httpOnly = getAttribute(Constants.COOKIE_HTTP_ONLY_ATTR); + if (httpOnly == null) { + return DEFAULT_HTTP_ONLY; + } + return Boolean.parseBoolean(httpOnly); } @Override public boolean isSecure() { - return secure; + String secure = getAttribute(Constants.COOKIE_SECURE_ATTR); + if (secure == null) { + return DEFAULT_SECURE; + } + return Boolean.parseBoolean(secure); } @Override @@ -86,7 +103,7 @@ public class ApplicationSessionCookieConfig implements SessionCookieConfig { "applicationSessionCookieConfig.ise", "comment", context.getPath())); } - this.comment = comment; + setAttribute(Constants.COOKIE_COMMENT_ATTR, comment); } @Override @@ -96,7 +113,7 @@ public class ApplicationSessionCookieConfig implements SessionCookieConfig { "applicationSessionCookieConfig.ise", "domain name", context.getPath())); } - this.domain = domain; + setAttribute(Constants.COOKIE_DOMAIN_ATTR, domain); } @Override @@ -106,7 +123,7 @@ public class ApplicationSessionCookieConfig implements SessionCookieConfig { "applicationSessionCookieConfig.ise", "HttpOnly", context.getPath())); } - this.httpOnly = httpOnly; + setAttribute(Constants.COOKIE_HTTP_ONLY_ATTR, Boolean.toString(httpOnly)); } @Override @@ -116,7 +133,7 @@ public class ApplicationSessionCookieConfig implements SessionCookieConfig { "applicationSessionCookieConfig.ise", "max age", context.getPath())); } - this.maxAge = maxAge; + setAttribute(Constants.COOKIE_MAX_AGE_ATTR, Integer.toString(maxAge)); } @Override @@ -136,7 +153,7 @@ public class ApplicationSessionCookieConfig implements SessionCookieConfig { "applicationSessionCookieConfig.ise", "path", context.getPath())); } - this.path = path; + setAttribute(Constants.COOKIE_PATH_ATTR, path); } @Override @@ -146,7 +163,24 @@ public class ApplicationSessionCookieConfig implements SessionCookieConfig { "applicationSessionCookieConfig.ise", "secure", context.getPath())); } - this.secure = secure; + setAttribute(Constants.COOKIE_SECURE_ATTR, Boolean.toString(secure)); + } + + + @Override + public void setAttribute(String name, String value) { + attributes.put(name, value); + } + + @Override + public String getAttribute(String name) { + return attributes.get(name); + } + + + @Override + public Map<String, String> getAttributes() { + return Collections.unmodifiableMap(attributes); } /** @@ -197,6 +231,23 @@ public class ApplicationSessionCookieConfig implements SessionCookieConfig { cookie.setPath(SessionConfig.getSessionCookiePath(context)); + // Other attributes + for (Map.Entry<String,String> attribute : scc.getAttributes().entrySet()) { + switch (attribute.getKey()) { + case Constants.COOKIE_COMMENT_ATTR: + case Constants.COOKIE_DOMAIN_ATTR: + case Constants.COOKIE_MAX_AGE_ATTR: + case Constants.COOKIE_PATH_ATTR: + case Constants.COOKIE_SECURE_ATTR: + case Constants.COOKIE_HTTP_ONLY_ATTR: + // Handled above so NO-OP + break; + default: { + cookie.setAttribute(attribute.getKey(), attribute.getValue()); + } + } + } + return cookie; } } diff --git a/java/org/apache/catalina/startup/ContextConfig.java b/java/org/apache/catalina/startup/ContextConfig.java index 508457a..3e75e79 100644 --- a/java/org/apache/catalina/startup/ContextConfig.java +++ b/java/org/apache/catalina/startup/ContextConfig.java @@ -1567,20 +1567,11 @@ public class ContextConfig implements LifecycleListener { context.setSessionTimeout( sessionConfig.getSessionTimeout().intValue()); } - SessionCookieConfig scc = - context.getServletContext().getSessionCookieConfig(); + SessionCookieConfig scc = context.getServletContext().getSessionCookieConfig(); scc.setName(sessionConfig.getCookieName()); - scc.setDomain(sessionConfig.getCookieDomain()); - scc.setPath(sessionConfig.getCookiePath()); - scc.setComment(sessionConfig.getCookieComment()); - if (sessionConfig.getCookieHttpOnly() != null) { - scc.setHttpOnly(sessionConfig.getCookieHttpOnly().booleanValue()); - } - if (sessionConfig.getCookieSecure() != null) { - scc.setSecure(sessionConfig.getCookieSecure().booleanValue()); - } - if (sessionConfig.getCookieMaxAge() != null) { - scc.setMaxAge(sessionConfig.getCookieMaxAge().intValue()); + Map<String,String> attributes = sessionConfig.getCookieAttributes(); + for (Map.Entry<String,String> attribute : attributes.entrySet()) { + scc.setAttribute(attribute.getKey(), attribute.getValue()); } if (sessionConfig.getSessionTrackingModes().size() > 0) { context.getServletContext().setSessionTrackingModes( diff --git a/java/org/apache/tomcat/util/descriptor/web/Constants.java b/java/org/apache/tomcat/util/descriptor/web/Constants.java index dd3e5dd..7b6228c 100644 --- a/java/org/apache/tomcat/util/descriptor/web/Constants.java +++ b/java/org/apache/tomcat/util/descriptor/web/Constants.java @@ -23,4 +23,12 @@ public class Constants { public static final String WEB_XML_LOCATION = "/WEB-INF/web.xml"; + // -------------------------------------------------- Cookie attribute names + public static final String COOKIE_COMMENT_ATTR = "Comment"; + public static final String COOKIE_DOMAIN_ATTR = "Domain"; + public static final String COOKIE_MAX_AGE_ATTR = "Max-Age"; + public static final String COOKIE_PATH_ATTR = "Path"; + public static final String COOKIE_SECURE_ATTR = "Secure"; + public static final String COOKIE_HTTP_ONLY_ATTR = "HttpOnly"; + public static final String COOKIE_SAME_SITE_ATTR = "SameSite"; } diff --git a/java/org/apache/tomcat/util/descriptor/web/LocalStrings.properties b/java/org/apache/tomcat/util/descriptor/web/LocalStrings.properties index dd57b51..a48dac4 100644 --- a/java/org/apache/tomcat/util/descriptor/web/LocalStrings.properties +++ b/java/org/apache/tomcat/util/descriptor/web/LocalStrings.properties @@ -45,13 +45,8 @@ webXml.mergeConflictLoginConfig=A LoginConfig was defined inconsistently in mult webXml.mergeConflictOrder=Fragment relative ordering contains circular references. This can be resolved by using absolute ordering in web.xml. webXml.mergeConflictResource=The Resource [{0}] was defined inconsistently in multiple fragments including fragment with name [{1}] located at [{2}] webXml.mergeConflictServlet=The Servlet [{0}] was defined inconsistently in multiple fragments including fragment with name [{1}] located at [{2}] -webXml.mergeConflictSessionCookieComment=The session cookie comment was defined inconsistently in multiple fragments with different values including fragment with name [{0}] located at [{1}] -webXml.mergeConflictSessionCookieDomain=The session cookie domain was defined inconsistently in multiple fragments with different values including fragment with name [{0}] located at [{1}] -webXml.mergeConflictSessionCookieHttpOnly=The session cookie http-only flag was defined inconsistently in multiple fragments with different values including fragment with name [{0}] located at [{1}] -webXml.mergeConflictSessionCookieMaxAge=The session cookie max-age was defined inconsistently in multiple fragments with different values including fragment with name [{0}] located at [{1}] +webXml.mergeConflictSessionCookieAttributes=The session cookie attributes were defined inconsistently in multiple fragments with different values including fragment with name [{0}] located at [{1}] webXml.mergeConflictSessionCookieName=The session cookie name was defined inconsistently in multiple fragments with different values including fragment with name [{0}] located at [{1}] -webXml.mergeConflictSessionCookiePath=The session cookie path was defined inconsistently in multiple fragments with different values including fragment with name [{0}] located at [{1}] -webXml.mergeConflictSessionCookieSecure=The session cookie secure flag was defined inconsistently in multiple fragments with different values including fragment with name [{0}] located at [{1}] webXml.mergeConflictSessionTimeout=The session timeout was defined inconsistently in multiple fragments with different values including fragment with name [{0}] located at [{1}] webXml.mergeConflictSessionTrackingMode=The session tracking modes were defined inconsistently in multiple fragments including fragment with name [{0}] located at [{1}] webXml.mergeConflictString=The [{0}] with name [{1}] was defined inconsistently in multiple fragments including fragment with name [{2}] located at [{3}] diff --git a/java/org/apache/tomcat/util/descriptor/web/SessionConfig.java b/java/org/apache/tomcat/util/descriptor/web/SessionConfig.java index 7e169ac..2daddd4 100644 --- a/java/org/apache/tomcat/util/descriptor/web/SessionConfig.java +++ b/java/org/apache/tomcat/util/descriptor/web/SessionConfig.java @@ -17,6 +17,8 @@ package org.apache.tomcat.util.descriptor.web; import java.util.EnumSet; +import java.util.Map; +import java.util.TreeMap; import jakarta.servlet.SessionTrackingMode; @@ -26,14 +28,10 @@ import jakarta.servlet.SessionTrackingMode; * deployment descriptor. */ public class SessionConfig { + private Integer sessionTimeout; private String cookieName; - private String cookieDomain; - private String cookiePath; - private String cookieComment; - private Boolean cookieHttpOnly; - private Boolean cookieSecure; - private Integer cookieMaxAge; + private final Map<String,String> cookieAttributes = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); private final EnumSet<SessionTrackingMode> sessionTrackingModes = EnumSet.noneOf(SessionTrackingMode.class); @@ -52,45 +50,67 @@ public class SessionConfig { } public String getCookieDomain() { - return cookieDomain; + return getCookieAttribute(Constants.COOKIE_DOMAIN_ATTR); } public void setCookieDomain(String cookieDomain) { - this.cookieDomain = cookieDomain; + setCookieAttribute(Constants.COOKIE_DOMAIN_ATTR, cookieDomain); } public String getCookiePath() { - return cookiePath; + return getCookieAttribute(Constants.COOKIE_PATH_ATTR); } public void setCookiePath(String cookiePath) { - this.cookiePath = cookiePath; + setCookieAttribute(Constants.COOKIE_PATH_ATTR, cookiePath); } public String getCookieComment() { - return cookieComment; + return getCookieAttribute(Constants.COOKIE_COMMENT_ATTR); } public void setCookieComment(String cookieComment) { - this.cookieComment = cookieComment; + setCookieAttribute(Constants.COOKIE_COMMENT_ATTR, cookieComment); } public Boolean getCookieHttpOnly() { - return cookieHttpOnly; + String httpOnly = getCookieAttribute(Constants.COOKIE_HTTP_ONLY_ATTR); + if (httpOnly == null) { + return null; + } + return Boolean.valueOf(httpOnly); } public void setCookieHttpOnly(String cookieHttpOnly) { - this.cookieHttpOnly = Boolean.valueOf(cookieHttpOnly); + setCookieAttribute(Constants.COOKIE_HTTP_ONLY_ATTR, cookieHttpOnly); } public Boolean getCookieSecure() { - return cookieSecure; + String secure = getCookieAttribute(Constants.COOKIE_SECURE_ATTR); + if (secure == null) { + return null; + } + return Boolean.valueOf(secure); } public void setCookieSecure(String cookieSecure) { - this.cookieSecure = Boolean.valueOf(cookieSecure); + setCookieAttribute(Constants.COOKIE_SECURE_ATTR, cookieSecure); } public Integer getCookieMaxAge() { - return cookieMaxAge; + String maxAge = getCookieAttribute(Constants.COOKIE_MAX_AGE_ATTR); + if (maxAge == null) { + return null; + } + return Integer.valueOf(maxAge); } public void setCookieMaxAge(String cookieMaxAge) { - this.cookieMaxAge = Integer.valueOf(cookieMaxAge); + setCookieAttribute(Constants.COOKIE_MAX_AGE_ATTR, cookieMaxAge); + } + + public Map<String,String> getCookieAttributes() { + return cookieAttributes; + } + public void setCookieAttribute(String name, String value) { + cookieAttributes.put(name, value); + } + public String getCookieAttribute(String name) { + return cookieAttributes.get(name); } public EnumSet<SessionTrackingMode> getSessionTrackingModes() { @@ -100,5 +120,4 @@ public class SessionConfig { sessionTrackingModes.add( SessionTrackingMode.valueOf(sessionTrackingMode)); } - } diff --git a/java/org/apache/tomcat/util/descriptor/web/WebRuleSet.java b/java/org/apache/tomcat/util/descriptor/web/WebRuleSet.java index ba3bd53..61d3799 100644 --- a/java/org/apache/tomcat/util/descriptor/web/WebRuleSet.java +++ b/java/org/apache/tomcat/util/descriptor/web/WebRuleSet.java @@ -443,6 +443,10 @@ public class WebRuleSet implements RuleSet { "setCookieSecure", 0); digester.addCallMethod(fullPrefix + "/session-config/cookie-config/max-age", "setCookieMaxAge", 0); + digester.addCallMethod(fullPrefix + "/session-config/cookie-config/attribute", + "setCookieAttribute", 2); + digester.addCallParam(fullPrefix + "/session-config/cookie-config/attribute/attribute-name", 0); + digester.addCallParam(fullPrefix + "/session-config/cookie-config/attribute/attribute-value", 1); digester.addCallMethod(fullPrefix + "/session-config/tracking-mode", "addSessionTrackingMode", 0); diff --git a/java/org/apache/tomcat/util/descriptor/web/WebXml.java b/java/org/apache/tomcat/util/descriptor/web/WebXml.java index 8cba46f..3dc6a3e 100644 --- a/java/org/apache/tomcat/util/descriptor/web/WebXml.java +++ b/java/org/apache/tomcat/util/descriptor/web/WebXml.java @@ -31,6 +31,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import java.util.TreeMap; import jakarta.servlet.DispatcherType; import jakarta.servlet.ServletContext; @@ -1779,138 +1780,32 @@ public class WebXml extends XmlEncodingBase implements DocumentProperties.Charse sessionConfig.setCookieName( temp.getSessionConfig().getCookieName()); } - if (sessionConfig.getCookieDomain() == null) { - for (WebXml fragment : fragments) { - String value = fragment.getSessionConfig().getCookieDomain(); - if (value != null) { - if (temp.getSessionConfig().getCookieDomain() == null) { - temp.getSessionConfig().setCookieDomain(value); - } else if (value.equals( - temp.getSessionConfig().getCookieDomain())) { - // Fragments use same value - no conflict - } else { - log.error(sm.getString( - "webXml.mergeConflictSessionCookieDomain", - fragment.getName(), - fragment.getURL())); - return false; - } - } - } - sessionConfig.setCookieDomain( - temp.getSessionConfig().getCookieDomain()); - } - if (sessionConfig.getCookiePath() == null) { - for (WebXml fragment : fragments) { - String value = fragment.getSessionConfig().getCookiePath(); - if (value != null) { - if (temp.getSessionConfig().getCookiePath() == null) { - temp.getSessionConfig().setCookiePath(value); - } else if (value.equals( - temp.getSessionConfig().getCookiePath())) { - // Fragments use same value - no conflict - } else { - log.error(sm.getString( - "webXml.mergeConflictSessionCookiePath", - fragment.getName(), - fragment.getURL())); - return false; - } - } - } - sessionConfig.setCookiePath( - temp.getSessionConfig().getCookiePath()); - } - if (sessionConfig.getCookieComment() == null) { - for (WebXml fragment : fragments) { - String value = fragment.getSessionConfig().getCookieComment(); - if (value != null) { - if (temp.getSessionConfig().getCookieComment() == null) { - temp.getSessionConfig().setCookieComment(value); - } else if (value.equals( - temp.getSessionConfig().getCookieComment())) { - // Fragments use same value - no conflict - } else { - log.error(sm.getString( - "webXml.mergeConflictSessionCookieComment", - fragment.getName(), - fragment.getURL())); - return false; - } - } - } - sessionConfig.setCookieComment( - temp.getSessionConfig().getCookieComment()); - } - if (sessionConfig.getCookieHttpOnly() == null) { - for (WebXml fragment : fragments) { - Boolean value = fragment.getSessionConfig().getCookieHttpOnly(); - if (value != null) { - if (temp.getSessionConfig().getCookieHttpOnly() == null) { - temp.getSessionConfig().setCookieHttpOnly(value.toString()); - } else if (value.equals( - temp.getSessionConfig().getCookieHttpOnly())) { - // Fragments use same value - no conflict - } else { - log.error(sm.getString( - "webXml.mergeConflictSessionCookieHttpOnly", - fragment.getName(), - fragment.getURL())); - return false; - } - } - } - if (temp.getSessionConfig().getCookieHttpOnly() != null) { - sessionConfig.setCookieHttpOnly( - temp.getSessionConfig().getCookieHttpOnly().toString()); - } - } - if (sessionConfig.getCookieSecure() == null) { - for (WebXml fragment : fragments) { - Boolean value = fragment.getSessionConfig().getCookieSecure(); - if (value != null) { - if (temp.getSessionConfig().getCookieSecure() == null) { - temp.getSessionConfig().setCookieSecure(value.toString()); - } else if (value.equals( - temp.getSessionConfig().getCookieSecure())) { - // Fragments use same value - no conflict - } else { - log.error(sm.getString( - "webXml.mergeConflictSessionCookieSecure", - fragment.getName(), - fragment.getURL())); - return false; - } - } - } - if (temp.getSessionConfig().getCookieSecure() != null) { - sessionConfig.setCookieSecure( - temp.getSessionConfig().getCookieSecure().toString()); - } - } - if (sessionConfig.getCookieMaxAge() == null) { - for (WebXml fragment : fragments) { - Integer value = fragment.getSessionConfig().getCookieMaxAge(); - if (value != null) { - if (temp.getSessionConfig().getCookieMaxAge() == null) { - temp.getSessionConfig().setCookieMaxAge(value.toString()); - } else if (value.equals( - temp.getSessionConfig().getCookieMaxAge())) { - // Fragments use same value - no conflict + + Map<String,String> mainAttributes = getSessionConfig().getCookieAttributes(); + Map<String,String> mergedFragmentAttributes = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + for (WebXml fragment : fragments) { + for (Map.Entry<String,String> attribute : fragment.getSessionConfig().getCookieAttributes().entrySet()) { + // Skip any attribute in a fragment that is defined in the main web.xml + if (!mainAttributes.containsKey(attribute.getKey())) { + if (mergedFragmentAttributes.containsKey(attribute.getKey())) { + // Attribute has already been seen. + // If values are the same, NO-OP. If they are different + // trigger a merge error + if (!mergedFragmentAttributes.get(attribute.getKey()).equals(attribute.getValue())) { + log.error(sm.getString( + "webXml.mergeConflictSessionCookieAttributes", + fragment.getName(), + fragment.getURL())); + return false; + } } else { - log.error(sm.getString( - "webXml.mergeConflictSessionCookieMaxAge", - fragment.getName(), - fragment.getURL())); - return false; + // First time this attribute has been seen. Add it. + mergedFragmentAttributes.put(attribute.getKey(), attribute.getValue()); } } } - if (temp.getSessionConfig().getCookieMaxAge() != null) { - sessionConfig.setCookieMaxAge( - temp.getSessionConfig().getCookieMaxAge().toString()); - } } + mainAttributes.putAll(mergedFragmentAttributes); if (sessionConfig.getSessionTrackingModes().size() == 0) { for (WebXml fragment : fragments) { diff --git a/java/org/apache/tomcat/util/http/LegacyCookieProcessor.java b/java/org/apache/tomcat/util/http/LegacyCookieProcessor.java index 9a5078e..990ad3e 100644 --- a/java/org/apache/tomcat/util/http/LegacyCookieProcessor.java +++ b/java/org/apache/tomcat/util/http/LegacyCookieProcessor.java @@ -21,6 +21,7 @@ import java.nio.charset.StandardCharsets; import java.text.FieldPosition; import java.util.BitSet; import java.util.Date; +import java.util.Map; import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletRequest; @@ -29,6 +30,7 @@ import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.apache.tomcat.util.buf.ByteChunk; import org.apache.tomcat.util.buf.MessageBytes; +import org.apache.tomcat.util.descriptor.web.Constants; import org.apache.tomcat.util.log.UserDataHelper; import org.apache.tomcat.util.res.StringManager; @@ -326,11 +328,39 @@ public final class LegacyCookieProcessor extends CookieProcessorBase { buf.append("; HttpOnly"); } - SameSiteCookies sameSiteCookiesValue = getSameSiteCookies(); - - if (!sameSiteCookiesValue.equals(SameSiteCookies.UNSET)) { + String cookieSameSite = cookie.getAttribute(Constants.COOKIE_SAME_SITE_ATTR); + if (cookieSameSite == null) { + // Use processor config + SameSiteCookies sameSiteCookiesValue = getSameSiteCookies(); + if (sameSiteCookiesValue.equals(SameSiteCookies.UNSET)) { + buf.append("; SameSite="); + buf.append(sameSiteCookiesValue.getValue()); + } + } else { + // Use explict config buf.append("; SameSite="); - buf.append(sameSiteCookiesValue.getValue()); + buf.append(cookieSameSite); + } + + // Add the remaining attributes + for (Map.Entry<String,String> entry : cookie.getAttributes().entrySet()) { + switch (entry.getKey()) { + case Constants.COOKIE_COMMENT_ATTR: + case Constants.COOKIE_DOMAIN_ATTR: + case Constants.COOKIE_MAX_AGE_ATTR: + case Constants.COOKIE_PATH_ATTR: + case Constants.COOKIE_SECURE_ATTR: + case Constants.COOKIE_HTTP_ONLY_ATTR: + case Constants.COOKIE_SAME_SITE_ATTR: + // Handled above so NO-OP + break; + default: { + buf.append("; "); + buf.append(entry.getKey()); + buf.append('='); + maybeQuote(buf, entry.getValue(), version); + } + } } return buf.toString(); diff --git a/java/org/apache/tomcat/util/http/LocalStrings.properties b/java/org/apache/tomcat/util/http/LocalStrings.properties index f9b8e0d..43307a8 100644 --- a/java/org/apache/tomcat/util/http/LocalStrings.properties +++ b/java/org/apache/tomcat/util/http/LocalStrings.properties @@ -36,6 +36,8 @@ parameters.maxCountFail.fallToDebug=\n\ parameters.multipleDecodingFail=Character decoding failed. A total of [{0}] failures were detected but only the first was logged. Enable debug level logging for this logger to log all failures. parameters.noequal=Parameter starting at position [{0}] and ending at position [{1}] with a value of [{2}] was not followed by an ''='' character +rfc6265CookieProcessor.invalidAttributeName=An invalid attribute name [{0}] was specified for this cookie +rfc6265CookieProcessor.invalidAttributeValue=An invalid attribute value [{1}] was specified for this cookie attribute [{0}] rfc6265CookieProcessor.invalidCharInValue=An invalid character [{0}] was present in the Cookie value rfc6265CookieProcessor.invalidDomain=An invalid domain [{0}] was specified for this cookie rfc6265CookieProcessor.invalidPath=An invalid path [{0}] was specified for this cookie diff --git a/java/org/apache/tomcat/util/http/Rfc6265CookieProcessor.java b/java/org/apache/tomcat/util/http/Rfc6265CookieProcessor.java index 0864750..5a7bcde 100644 --- a/java/org/apache/tomcat/util/http/Rfc6265CookieProcessor.java +++ b/java/org/apache/tomcat/util/http/Rfc6265CookieProcessor.java @@ -21,6 +21,7 @@ import java.nio.charset.StandardCharsets; import java.text.FieldPosition; import java.util.BitSet; import java.util.Date; +import java.util.Map; import jakarta.servlet.http.HttpServletRequest; @@ -28,7 +29,9 @@ import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.apache.tomcat.util.buf.ByteChunk; import org.apache.tomcat.util.buf.MessageBytes; +import org.apache.tomcat.util.descriptor.web.Constants; import org.apache.tomcat.util.http.parser.Cookie; +import org.apache.tomcat.util.http.parser.HttpParser; import org.apache.tomcat.util.res.StringManager; public class Rfc6265CookieProcessor extends CookieProcessorBase { @@ -164,11 +167,40 @@ public class Rfc6265CookieProcessor extends CookieProcessorBase { header.append("; HttpOnly"); } - SameSiteCookies sameSiteCookiesValue = getSameSiteCookies(); - - if (!sameSiteCookiesValue.equals(SameSiteCookies.UNSET)) { + String cookieSameSite = cookie.getAttribute(Constants.COOKIE_SAME_SITE_ATTR); + if (cookieSameSite == null) { + // Use processor config + SameSiteCookies sameSiteCookiesValue = getSameSiteCookies(); + if (sameSiteCookiesValue.equals(SameSiteCookies.UNSET)) { + header.append("; SameSite="); + header.append(sameSiteCookiesValue.getValue()); + } + } else { + // Use explict config header.append("; SameSite="); - header.append(sameSiteCookiesValue.getValue()); + header.append(cookieSameSite); + } + + // Add the remaining attributes + for (Map.Entry<String,String> entry : cookie.getAttributes().entrySet()) { + switch (entry.getKey()) { + case Constants.COOKIE_COMMENT_ATTR: + case Constants.COOKIE_DOMAIN_ATTR: + case Constants.COOKIE_MAX_AGE_ATTR: + case Constants.COOKIE_PATH_ATTR: + case Constants.COOKIE_SECURE_ATTR: + case Constants.COOKIE_HTTP_ONLY_ATTR: + case Constants.COOKIE_SAME_SITE_ATTR: + // Handled above so NO-OP + break; + default: { + validateAttribute(entry.getKey(), entry.getValue()); + header.append("; "); + header.append(entry.getKey()); + header.append('='); + header.append(entry.getValue()); + } + } } return header.toString(); @@ -237,4 +269,23 @@ public class Rfc6265CookieProcessor extends CookieProcessorBase { } } } + + + private void validateAttribute(String name, String value) { + char[] chars = name.toCharArray(); + for (char ch : chars) { + if (!HttpParser.isToken(ch)) { + throw new IllegalArgumentException(sm.getString( + "rfc6265CookieProcessor.invalidAttributeName", name)); + } + } + + chars = value.toCharArray(); + for (char ch : chars) { + if (ch < 0x20 || ch > 0x7E || ch == ';') { + throw new IllegalArgumentException(sm.getString( + "rfc6265CookieProcessor.invalidAttributeValue", name, value)); + } + } + } } diff --git a/test/jakarta/servlet/TestSessionCookieConfig.java b/test/jakarta/servlet/TestSessionCookieConfig.java new file mode 100644 index 0000000..5faa668 --- /dev/null +++ b/test/jakarta/servlet/TestSessionCookieConfig.java @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package jakarta.servlet; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.tomcat.util.buf.ByteChunk; + +public class TestSessionCookieConfig extends TomcatBaseTest { + + /* + * Not strictly testing the SessionCookieConfig class + */ + @Test + public void testCustomAttribute() throws Exception { + getTomcatInstanceTestWebapp(false, true); + + ByteChunk responseBody = new ByteChunk(); + Map<String,List<String>> responseHeaders = new HashMap<>(); + + int statusCode = + getUrl("http://localhost:" + getPort() + "/test/bug49nnn/bug49196.jsp", responseBody, responseHeaders); + + Assert.assertEquals(HttpServletResponse.SC_OK, statusCode); + Assert.assertTrue(responseBody.toString().contains("OK")); + Assert.assertTrue(responseHeaders.containsKey("Set-Cookie")); + + List<String> setCookieHeaders = responseHeaders.get("Set-Cookie"); + Assert.assertEquals(1, setCookieHeaders.size()); + + String setCookieHeader = setCookieHeaders.get(0); + Assert.assertTrue(setCookieHeader.contains("; aaa=bbb")); + } +} diff --git a/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java b/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java index fc06ab7..1e13ed2 100644 --- a/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java +++ b/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java @@ -17,7 +17,10 @@ package org.apache.catalina.valves; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; +import java.util.Map; +import java.util.TreeMap; import jakarta.servlet.ServletContext; import jakarta.servlet.SessionCookieConfig; @@ -34,6 +37,7 @@ import org.apache.catalina.Valve; import org.apache.catalina.connector.Request; import org.apache.catalina.connector.Response; import org.apache.catalina.core.StandardPipeline; +import org.apache.tomcat.util.descriptor.web.Constants; import org.easymock.EasyMock; import org.easymock.IMocksControl; @@ -191,12 +195,8 @@ public class TestLoadBalancerDrainingValve { private static class CookieConfig implements SessionCookieConfig { private String name; - private String domain; - private String path; - private String comment; - private boolean httpOnly; - private boolean secure; - private int maxAge; + + private final Map<String,String> attributes = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); @Override public String getName() { @@ -208,51 +208,75 @@ public class TestLoadBalancerDrainingValve { } @Override public String getDomain() { - return domain; + return attributes.get(Constants.COOKIE_DOMAIN_ATTR); } @Override public void setDomain(String domain) { - this.domain = domain; + attributes.put(Constants.COOKIE_DOMAIN_ATTR, domain); } @Override public String getPath() { - return path; + return attributes.get(Constants.COOKIE_PATH_ATTR); } @Override public void setPath(String path) { - this.path = path; + attributes.put(Constants.COOKIE_PATH_ATTR, path); } @Override public String getComment() { - return comment; + return attributes.get(Constants.COOKIE_COMMENT_ATTR); } @Override public void setComment(String comment) { - this.comment = comment; + attributes.put(Constants.COOKIE_COMMENT_ATTR, comment); } @Override public boolean isHttpOnly() { - return httpOnly; + String httpOnly = getAttribute(Constants.COOKIE_HTTP_ONLY_ATTR); + if (httpOnly == null) { + return false; + } + return Boolean.parseBoolean(httpOnly); } @Override public void setHttpOnly(boolean httpOnly) { - this.httpOnly = httpOnly; + setAttribute(Constants.COOKIE_HTTP_ONLY_ATTR, Boolean.toString(httpOnly)); } @Override public boolean isSecure() { - return secure; + String secure = getAttribute(Constants.COOKIE_SECURE_ATTR); + if (secure == null) { + return false; + } + return Boolean.parseBoolean(secure); } @Override public void setSecure(boolean secure) { - this.secure = secure; + setAttribute(Constants.COOKIE_SECURE_ATTR, Boolean.toString(secure)); } @Override public int getMaxAge() { - return maxAge; + String maxAge = getAttribute(Constants.COOKIE_MAX_AGE_ATTR); + if (maxAge == null) { + return -1; + } + return Integer.parseInt(maxAge); } @Override public void setMaxAge(int maxAge) { - this.maxAge = maxAge; + setAttribute(Constants.COOKIE_MAX_AGE_ATTR, Integer.toString(maxAge)); + } + @Override + public void setAttribute(String name, String value) { + attributes.put(name, value); + } + @Override + public String getAttribute(String name) { + return attributes.get(name); + } + @Override + public Map<String, String> getAttributes() { + return Collections.unmodifiableMap(attributes); } } diff --git a/test/org/apache/tomcat/unittest/TesterSessionCookieConfig.java b/test/org/apache/tomcat/unittest/TesterSessionCookieConfig.java index 7994752..afae9b1 100644 --- a/test/org/apache/tomcat/unittest/TesterSessionCookieConfig.java +++ b/test/org/apache/tomcat/unittest/TesterSessionCookieConfig.java @@ -16,6 +16,8 @@ */ package org.apache.tomcat.unittest; +import java.util.Map; + import jakarta.servlet.SessionCookieConfig; public class TesterSessionCookieConfig implements SessionCookieConfig { @@ -90,4 +92,19 @@ public class TesterSessionCookieConfig implements SessionCookieConfig { public int getMaxAge() { throw new UnsupportedOperationException(); } + + @Override + public void setAttribute(String name, String value) { + throw new UnsupportedOperationException(); + } + + @Override + public String getAttribute(String name) { + throw new UnsupportedOperationException(); + } + + @Override + public Map<String, String> getAttributes() { + throw new UnsupportedOperationException(); + } } diff --git a/test/org/apache/tomcat/util/descriptor/web/TestWebXml.java b/test/org/apache/tomcat/util/descriptor/web/TestWebXml.java index a2c79e2..fa223aa 100644 --- a/test/org/apache/tomcat/util/descriptor/web/TestWebXml.java +++ b/test/org/apache/tomcat/util/descriptor/web/TestWebXml.java @@ -537,6 +537,96 @@ public class TestWebXml { Assert.assertEquals(StandardCharsets.ISO_8859_1, securityCollection.getCharset()); } } + } + + + @Test + public void testMergeSessionCookieConfig01() { + WebXml main = new WebXml(); + WebXml fragmentA = new WebXml(); + WebXml fragmentB = new WebXml(); + + fragmentA.getSessionConfig().setCookieHttpOnly("true"); + fragmentB.getSessionConfig().setCookieSecure("true"); + + Set<WebXml> fragments = new HashSet<>(); + fragments.add(fragmentA); + fragments.add(fragmentB); + + Assert.assertTrue(main.merge(fragments)); + Assert.assertEquals(Boolean.TRUE, main.getSessionConfig().getCookieHttpOnly()); + Assert.assertEquals(Boolean.TRUE, main.getSessionConfig().getCookieSecure()); + } + + + @Test + public void testMergeSessionCookieConfig02() { + WebXml main = new WebXml(); + WebXml fragmentA = new WebXml(); + WebXml fragmentB = new WebXml(); + + fragmentA.getSessionConfig().setCookieHttpOnly("true"); + fragmentB.getSessionConfig().setCookieHttpOnly("false"); + + Set<WebXml> fragments = new HashSet<>(); + fragments.add(fragmentA); + fragments.add(fragmentB); + + Assert.assertFalse(main.merge(fragments)); + } + + + @Test + public void testMergeSessionCookieConfig03() { + WebXml main = new WebXml(); + WebXml fragmentA = new WebXml(); + WebXml fragmentB = new WebXml(); + + main.getSessionConfig().setCookieHttpOnly("false"); + fragmentA.getSessionConfig().setCookieHttpOnly("true"); + fragmentB.getSessionConfig().setCookieSecure("true"); + + Set<WebXml> fragments = new HashSet<>(); + fragments.add(fragmentA); + fragments.add(fragmentB); + + Assert.assertTrue(main.merge(fragments)); + Assert.assertEquals(Boolean.FALSE, main.getSessionConfig().getCookieHttpOnly()); + Assert.assertEquals(Boolean.TRUE, main.getSessionConfig().getCookieSecure()); + } + + + @Test + public void testMergeSessionCookieConfig04() { + WebXml main = new WebXml(); + WebXml fragmentA = new WebXml(); + WebXml fragmentB = new WebXml(); + + fragmentA.getSessionConfig().setCookieAttribute("aaa", "bbb"); + fragmentB.getSessionConfig().setCookieAttribute("AAA", "bbb"); + + Set<WebXml> fragments = new HashSet<>(); + fragments.add(fragmentA); + fragments.add(fragmentB); + + Assert.assertTrue(main.merge(fragments)); + Assert.assertEquals("bbb", main.getSessionConfig().getCookieAttribute("aAa")); + } + + + @Test + public void testMergeSessionCookieConfig05() { + WebXml main = new WebXml(); + WebXml fragmentA = new WebXml(); + WebXml fragmentB = new WebXml(); + + fragmentA.getSessionConfig().setCookieAttribute("aaa", "bBb"); + fragmentB.getSessionConfig().setCookieAttribute("AAA", "bbb"); + + Set<WebXml> fragments = new HashSet<>(); + fragments.add(fragmentA); + fragments.add(fragmentB); + Assert.assertFalse(main.merge(fragments)); } } diff --git a/test/webapp/WEB-INF/web.xml b/test/webapp/WEB-INF/web.xml index b1f30f0..43eb0f1 100644 --- a/test/webapp/WEB-INF/web.xml +++ b/test/webapp/WEB-INF/web.xml @@ -327,4 +327,12 @@ <mapped-name>Bug53465MappedName</mapped-name> </env-entry> + <session-config> + <cookie-config> + <attribute> + <attribute-name>aaa</attribute-name> + <attribute-value>bbb</attribute-value> + </attribute> + </cookie-config> + </session-config> </web-app> \ No newline at end of file diff --git a/test/webapp/jsp/session.jsp b/test/webapp/jsp/session.jsp new file mode 100644 index 0000000..87ec0c3 --- /dev/null +++ b/test/webapp/jsp/session.jsp @@ -0,0 +1,18 @@ +<%-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--%> +<%@ page session="true" %> +<p>OK</p> \ No newline at end of file diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 0648e73..febc140 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -117,6 +117,11 @@ recent changes updates for Servlet 6.0 in the Jakarta Servlet specification project. (markt) </scode> + <add> + Add support for setting generic attributes for session cookies. This + aligns Apache Tomcat with recent changes in the Jakarta Servlet + specification project. (markt) + </add> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org