From ba9cf4b652946c7958449187e82990a919ce8fbc Mon Sep 17 00:00:00 2001 From: Arjan Tijms Date: Mon, 11 Apr 2022 00:29:28 +0200 Subject: [PATCH] Fix validation for session cookie config Signed-off-by: Arjan Tijms --- .../core/SessionCookieConfigImpl.java | 198 +++++++----------- 1 file changed, 81 insertions(+), 117 deletions(-) diff --git a/appserver/web/web-core/src/main/java/org/apache/catalina/core/SessionCookieConfigImpl.java b/appserver/web/web-core/src/main/java/org/apache/catalina/core/SessionCookieConfigImpl.java index 5a6f8a9c49f..6aa6b396bb1 100644 --- a/appserver/web/web-core/src/main/java/org/apache/catalina/core/SessionCookieConfigImpl.java +++ b/appserver/web/web-core/src/main/java/org/apache/catalina/core/SessionCookieConfigImpl.java @@ -17,22 +17,31 @@ package org.apache.catalina.core; -import org.apache.catalina.LogFacade; +import static java.lang.String.CASE_INSENSITIVE_ORDER; +import static java.util.Collections.unmodifiableMap; +import static org.apache.catalina.LogFacade.SESSION_COOKIE_CONFIG_ALREADY_INIT; +import static org.apache.catalina.core.Constants.COOKIE_DOMAIN_ATTR; +import static org.apache.catalina.core.Constants.COOKIE_HTTP_ONLY_ATTR; +import static org.apache.catalina.core.Constants.COOKIE_MAX_AGE_ATTR; +import static org.apache.catalina.core.Constants.COOKIE_PATH_ATTR; +import static org.apache.catalina.core.Constants.COOKIE_SECURE_ATTR; import java.text.MessageFormat; -import java.util.Collections; import java.util.Map; import java.util.ResourceBundle; import java.util.TreeMap; +import org.apache.catalina.LogFacade; + import jakarta.servlet.SessionCookieConfig; /** - * Class that may be used to configure various properties of cookies - * used for session tracking purposes. + * Class that may be used to configure various properties of cookies used for session tracking purposes. */ public class SessionCookieConfigImpl implements SessionCookieConfig { + private static final ResourceBundle rb = LogFacade.getLogger().getResourceBundle(); + private static final int DEFAULT_MAX_AGE = -1; private static final boolean DEFAULT_HTTP_ONLY = false; private static final boolean DEFAULT_SECURE = false; @@ -40,12 +49,9 @@ public class SessionCookieConfigImpl implements SessionCookieConfig { private String name = DEFAULT_NAME; - private final Map attributes = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + private final Map attributes = new TreeMap<>(CASE_INSENSITIVE_ORDER); private final StandardContext ctx; - private static final ResourceBundle rb = LogFacade.getLogger().getResourceBundle(); - - /** * Constructor */ @@ -53,215 +59,156 @@ public SessionCookieConfigImpl(StandardContext ctx) { this.ctx = ctx; } - /** * @param name the cookie name to use * - * @throws IllegalStateException if the ServletContext - * from which this SessionCookieConfig was acquired has - * already been initialized + * @throws IllegalStateException if the ServletContext from which this SessionCookieConfig was + * acquired has already been initialized */ @Override public void setName(String name) { - if (ctx.isContextInitializedCalled()) { - String msg = MessageFormat.format(rb.getString(LogFacade.SESSION_COOKIE_CONFIG_ALREADY_INIT), - new Object[] {"name", ctx.getName()}); - throw new IllegalStateException(msg); - } + checkContextInitialized("name"); this.name = name; ctx.setSessionCookieName(name); } - /** - * @return the cookie name set via {@link #setName}, or - * JSESSIONID if {@link #setName} was never called + * @return the cookie name set via {@link #setName}, or JSESSIONID if {@link #setName} was never called */ @Override public String getName() { return name; } - /** * @param domain the cookie domain to use * - * @throws IllegalStateException if the ServletContext - * from which this SessionCookieConfig was acquired has - * already been initialized + * @throws IllegalStateException if the ServletContext from which this SessionCookieConfig was + * acquired has already been initialized */ @Override public void setDomain(String domain) { - if (ctx.isContextInitializedCalled()) { - String msg = MessageFormat.format(rb.getString(LogFacade.SESSION_COOKIE_CONFIG_ALREADY_INIT), - new Object[] {"dnmain", ctx.getName()}); - throw new IllegalStateException(msg); - } - setAttribute(Constants.COOKIE_DOMAIN_ATTR, domain); + checkContextInitialized("domain"); + setAttribute(COOKIE_DOMAIN_ATTR, domain); } - /** - * @return the cookie domain set via {@link #setDomain}, or - * null if {@link #setDomain} was never called + * @return the cookie domain set via {@link #setDomain}, or null if {@link #setDomain} was never called */ @Override public String getDomain() { - return getAttribute(Constants.COOKIE_DOMAIN_ATTR); + return getAttribute(COOKIE_DOMAIN_ATTR); } - /** * @param path the cookie path to use * - * @throws IllegalStateException if the ServletContext - * from which this SessionCookieConfig was acquired has - * already been initialized + * @throws IllegalStateException if the ServletContext from which this SessionCookieConfig was + * acquired has already been initialized */ @Override public void setPath(String path) { - if (ctx.isContextInitializedCalled()) { - String msg = MessageFormat.format(rb.getString(LogFacade.SESSION_COOKIE_CONFIG_ALREADY_INIT), - new Object[] {"path", ctx.getName()}); - throw new IllegalStateException(msg); - } - setAttribute(Constants.COOKIE_PATH_ATTR, path); + checkContextInitialized("path"); + setAttribute(COOKIE_PATH_ATTR, path); } - /** - * @return the cookie path set via {@link #setPath}, or the context - * path of the ServletContext from which this - * SessionCookieConfig was acquired if {@link #setPath} - * was never called + * @return the cookie path set via {@link #setPath}, or the context path of the ServletContext from which this + * SessionCookieConfig was acquired if {@link #setPath} was never called */ @Override public String getPath() { - return getAttribute(Constants.COOKIE_PATH_ATTR); + return getAttribute(COOKIE_PATH_ATTR); } - /** * @param comment the cookie comment to use * - * @throws IllegalStateException if the ServletContext - * from which this SessionCookieConfig was acquired has - * already been initialized + * @throws IllegalStateException if the ServletContext from which this SessionCookieConfig was + * acquired has already been initialized */ @Override public void setComment(String comment) { - if (ctx.isContextInitializedCalled()) { - String msg = MessageFormat.format(rb.getString(LogFacade.SESSION_COOKIE_CONFIG_ALREADY_INIT), - new Object[] {"comment", ctx.getName()}); - throw new IllegalStateException(msg); - } + checkContextInitialized("comment"); setAttribute(Constants.COOKIE_COMMENT_ATTR, comment); } - /** - * @return the cookie comment set via {@link #setComment}, or - * null if {@link #setComment} was never called + * @return the cookie comment set via {@link #setComment}, or null if {@link #setComment} was never called */ @Override public String getComment() { return getAttribute(Constants.COOKIE_COMMENT_ATTR); } - /** - * @param httpOnly true if the session tracking cookies created - * on behalf of the ServletContext from which this - * SessionCookieConfig was acquired shall be marked as - * HttpOnly, false otherwise + * @param httpOnly true if the session tracking cookies created on behalf of the ServletContext from which this + * SessionCookieConfig was acquired shall be marked as HttpOnly, false otherwise * - * @throws IllegalStateException if the ServletContext - * from which this SessionCookieConfig was acquired has - * already been initialized + * @throws IllegalStateException if the ServletContext from which this SessionCookieConfig was + * acquired has already been initialized */ @Override public void setHttpOnly(boolean httpOnly) { - if (ctx.isContextInitializedCalled()) { - String msg = MessageFormat.format(rb.getString(LogFacade.SESSION_COOKIE_CONFIG_ALREADY_INIT), - new Object[] {"httpOnly", ctx.getName()}); - throw new IllegalStateException(msg); - } - setAttribute(Constants.COOKIE_HTTP_ONLY_ATTR, String.valueOf(httpOnly)); + checkContextInitialized("httpOnly"); + setAttribute(COOKIE_HTTP_ONLY_ATTR, String.valueOf(httpOnly)); } - /** - * @return true if the session tracking cookies created on behalf of the - * ServletContext from which this SessionCookieConfig - * was acquired will be marked as HttpOnly, false otherwise + * @return true if the session tracking cookies created on behalf of the ServletContext from which this + * SessionCookieConfig was acquired will be marked as HttpOnly, false otherwise */ @Override public boolean isHttpOnly() { - String value = getAttribute(Constants.COOKIE_HTTP_ONLY_ATTR); + String value = getAttribute(COOKIE_HTTP_ONLY_ATTR); return value == null ? DEFAULT_HTTP_ONLY : Boolean.parseBoolean(value); } - /** - * @param secure true if the session tracking cookies created on - * behalf of the ServletContext from which this - * SessionCookieConfig was acquired shall be marked as - * secure even if the request that initiated the corresponding - * session is using plain HTTP instead of HTTPS, and false if they - * shall be marked as secure only if the request that initiated - * the corresponding session was also secure + * @param secure true if the session tracking cookies created on behalf of the ServletContext from which this + * SessionCookieConfig was acquired shall be marked as secure even if the request that initiated the + * corresponding session is using plain HTTP instead of HTTPS, and false if they shall be marked as secure only + * if the request that initiated the corresponding session was also secure * - * @throws IllegalStateException if the ServletContext - * from which this SessionCookieConfig was acquired has - * already been initialized + * @throws IllegalStateException if the ServletContext from which this SessionCookieConfig was + * acquired has already been initialized */ @Override public void setSecure(boolean secure) { - if (ctx.isContextInitializedCalled()) { - String msg = MessageFormat.format(rb.getString(LogFacade.SESSION_COOKIE_CONFIG_ALREADY_INIT), - new Object[] {"secure", ctx.getName()}); - throw new IllegalStateException(msg); - } - setAttribute(Constants.COOKIE_SECURE_ATTR, String.valueOf(secure)); + checkContextInitialized("secure"); + setAttribute(COOKIE_SECURE_ATTR, String.valueOf(secure)); } - /** - * @return true if the session tracking cookies created on behalf of the - * ServletContext from which this SessionCookieConfig - * was acquired will be marked as secure even if the request - * that initiated the corresponding session is using plain HTTP - * instead of HTTPS, and false if they will be marked as secure - * only if the request that initiated the corresponding session was - * also secure + * @return true if the session tracking cookies created on behalf of the ServletContext from which this + * SessionCookieConfig was acquired will be marked as secure even if the request that initiated the + * corresponding session is using plain HTTP instead of HTTPS, and false if they will be marked as secure only if + * the request that initiated the corresponding session was also secure */ @Override public boolean isSecure() { - String value = getAttribute(Constants.COOKIE_SECURE_ATTR); + String value = getAttribute(COOKIE_SECURE_ATTR); return value == null ? DEFAULT_SECURE : Boolean.parseBoolean(value); } - @Override public void setMaxAge(int maxAge) { - if (ctx.isContextInitializedCalled()) { - String msg = MessageFormat.format(rb.getString(LogFacade.SESSION_COOKIE_CONFIG_ALREADY_INIT), - new Object[] {"maxAge", ctx.getName()}); - throw new IllegalStateException(msg); - } - setAttribute(Constants.COOKIE_MAX_AGE_ATTR, String.valueOf(maxAge)); + checkContextInitialized("maxAge"); + setAttribute(COOKIE_MAX_AGE_ATTR, String.valueOf(maxAge)); } - @Override public int getMaxAge() { - String value = getAttribute(Constants.COOKIE_MAX_AGE_ATTR); + String value = getAttribute(COOKIE_MAX_AGE_ATTR); return value == null ? DEFAULT_MAX_AGE : Integer.parseInt(value); } @Override public void setAttribute(String name, String value) { + checkContextInitialized("attribute"); + checkValid(name, value); + this.attributes.put(name, value); } @@ -272,6 +219,23 @@ public String getAttribute(String name) { @Override public Map getAttributes() { - return Collections.unmodifiableMap(this.attributes); + return unmodifiableMap(this.attributes); + } + + private void checkContextInitialized(String parameter) { + if (ctx.isContextInitializedCalled()) { + throw new IllegalStateException(MessageFormat.format(rb.getString(SESSION_COOKIE_CONFIG_ALREADY_INIT), + new Object[] { parameter, ctx.getName() })); + } + } + + private void checkValid(String name, String value) { + if (name == null) { + throw new IllegalArgumentException(name + " cannot be null"); + } + + if (name.equals("DEFAULT_MAX_AGE")) { + Integer.parseInt(value); + } } }