From ec502dea45083354b9ff906cda8d8690659c6321 Mon Sep 17 00:00:00 2001 From: markt Date: Sat, 31 Oct 2009 12:59:51 +0000 Subject: [PATCH] Add an explicit configuration option for cookie version switching and update test cases and docs to include it. git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@831536 13f79535-47bb-0310-9956-ffa450edef68 --- java/org/apache/tomcat/util/http/ServerCookie.java | 19 +++++- .../apache/tomcat/util/http/CookiesBaseTest.java | 22 ++++--- .../util/http/TestCookiesDefaultSysProps.java | 14 +++++ .../util/http/TestCookiesStrictSysProps.java | 14 +++++ .../util/http/TestCookiesSwitchSysProps.java | 71 ++++++++++++++++++++++ webapps/docs/config/systemprops.xml | 10 +++ 6 files changed, 141 insertions(+), 9 deletions(-) create mode 100644 test/org/apache/tomcat/util/http/TestCookiesSwitchSysProps.java diff --git a/java/org/apache/tomcat/util/http/ServerCookie.java b/java/org/apache/tomcat/util/http/ServerCookie.java index e9cc85103..39a274481 100644 --- a/java/org/apache/tomcat/util/http/ServerCookie.java +++ b/java/org/apache/tomcat/util/http/ServerCookie.java @@ -75,6 +75,14 @@ public class ServerCookie implements Serializable { public static final boolean STRICT_SERVLET_COMPLIANCE; /** + * If set to false, we don't auto switch invalid v0 cookies to v1 and add + * quotes to make them valid. + * Default is usually true. If STRICT_SERVLET_COMPLIANCE==true then default + * is false. Explicitly setting always takes priority. + */ + public static final boolean ALLOW_VERSION_SWITCH; + + /** * If set to false, we don't use the IE6/7 Max-Age/Expires work around. * Default is usually true. If STRICT_SERVLET_COMPLIANCE==true then default * is false. Explicitly setting always takes priority. @@ -97,6 +105,15 @@ public class ServerCookie implements Serializable { "false")).booleanValue(); + String allowVersionSwitch = System.getProperty( + "org.apache.tomcat.util.http.ServerCookie.ALLOW_VERSION_SWITCH"); + if (allowVersionSwitch == null) { + ALLOW_VERSION_SWITCH = !STRICT_SERVLET_COMPLIANCE; + } else { + ALLOW_VERSION_SWITCH = + Boolean.valueOf(allowVersionSwitch).booleanValue(); + } + String alwaysAddExpires = System.getProperty( "org.apache.tomcat.util.http.ServerCookie.ALWAYS_ADD_EXPIRES"); if (alwaysAddExpires == null) { @@ -400,7 +417,7 @@ public class ServerCookie implements Serializable { buf.append('"'); buf.append(escapeDoubleQuotes(value,1,value.length()-1)); buf.append('"'); - } else if (allowVersionSwitch && (!STRICT_SERVLET_COMPLIANCE) && version==0 && !isToken2(value, literals)) { + } else if (allowVersionSwitch && ALLOW_VERSION_SWITCH && version==0 && !isToken2(value, literals)) { buf.append('"'); buf.append(escapeDoubleQuotes(value,0,value.length())); buf.append('"'); diff --git a/test/org/apache/tomcat/util/http/CookiesBaseTest.java b/test/org/apache/tomcat/util/http/CookiesBaseTest.java index e18fb91e5..e6ad38369 100644 --- a/test/org/apache/tomcat/util/http/CookiesBaseTest.java +++ b/test/org/apache/tomcat/util/http/CookiesBaseTest.java @@ -39,20 +39,22 @@ public abstract class CookiesBaseTest extends TomcatBaseTest { /** * Servlet for cookie naming test. */ - public static class CookieName extends HttpServlet { + public static class CookieServlet extends HttpServlet { private static final long serialVersionUID = 1L; private final String cookieName; - - public CookieName(String cookieName) { + private final String cookieValue; + + public CookieServlet(String cookieName, String cookieValue) { this.cookieName = cookieName; + this.cookieValue = cookieValue; } public void doGet(HttpServletRequest req, HttpServletResponse res) throws IOException { try { - Cookie cookie = new Cookie(cookieName,"Value"); + Cookie cookie = new Cookie(cookieName, cookieValue); res.addCookie(cookie); res.getWriter().write("Cookie name ok"); } catch (IllegalArgumentException iae) { @@ -68,14 +70,18 @@ public abstract class CookiesBaseTest extends TomcatBaseTest { StandardContext ctx = tomcat.addContext("/", System.getProperty("java.io.tmpdir")); - Tomcat.addServlet(ctx, "invalid", new CookieName("na;me")); + Tomcat.addServlet(ctx, "invalid", new CookieServlet("na;me", "value")); ctx.addServletMapping("/invalid", "invalid"); - Tomcat.addServlet(ctx, "invalidFwd", new CookieName("na/me")); + Tomcat.addServlet(ctx, "invalidFwd", + new CookieServlet("na/me", "value")); ctx.addServletMapping("/invalidFwd", "invalidFwd"); - Tomcat.addServlet(ctx, "invalidStrict", new CookieName("na?me")); + Tomcat.addServlet(ctx, "invalidStrict", + new CookieServlet("na?me", "value")); ctx.addServletMapping("/invalidStrict", "invalidStrict"); - Tomcat.addServlet(ctx, "valid", new CookieName("name")); + Tomcat.addServlet(ctx, "valid", new CookieServlet("name", "value")); ctx.addServletMapping("/valid", "valid"); + Tomcat.addServlet(ctx, "switch", new CookieServlet("name", "val?ue")); + ctx.addServletMapping("/switch", "switch"); } diff --git a/test/org/apache/tomcat/util/http/TestCookiesDefaultSysProps.java b/test/org/apache/tomcat/util/http/TestCookiesDefaultSysProps.java index 035bcc096..c77ddbb3f 100644 --- a/test/org/apache/tomcat/util/http/TestCookiesDefaultSysProps.java +++ b/test/org/apache/tomcat/util/http/TestCookiesDefaultSysProps.java @@ -17,6 +17,10 @@ package org.apache.tomcat.util.http; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + import org.apache.catalina.startup.Tomcat; import org.apache.tomcat.util.buf.ByteChunk; @@ -46,6 +50,16 @@ public class TestCookiesDefaultSysProps extends CookiesBaseTest { res = getUrl("http://localhost:" + getPort() + "/valid"); assertEquals("Cookie name ok", res.toString()); + // Need to read response headers to test version switching + Map> headers = new HashMap>(); + getUrl("http://localhost:" + getPort() + "/switch", res, headers); + List cookieHeaders = headers.get("Set-Cookie"); + for (String cookieHeader : cookieHeaders) { + if (cookieHeader.contains("name=")) { + assertTrue(cookieHeader.contains("name=\"val?ue\";")); + } + } + } } diff --git a/test/org/apache/tomcat/util/http/TestCookiesStrictSysProps.java b/test/org/apache/tomcat/util/http/TestCookiesStrictSysProps.java index 0aaf46c64..a844195f8 100644 --- a/test/org/apache/tomcat/util/http/TestCookiesStrictSysProps.java +++ b/test/org/apache/tomcat/util/http/TestCookiesStrictSysProps.java @@ -17,6 +17,10 @@ package org.apache.tomcat.util.http; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + import org.apache.catalina.startup.Tomcat; import org.apache.tomcat.util.buf.ByteChunk; @@ -49,6 +53,16 @@ public class TestCookiesStrictSysProps extends CookiesBaseTest { res = getUrl("http://localhost:" + getPort() + "/valid"); assertEquals("Cookie name ok", res.toString()); + // Need to read response headers to test version switching + Map> headers = new HashMap>(); + getUrl("http://localhost:" + getPort() + "/switch", res, headers); + List cookieHeaders = headers.get("Set-Cookie"); + for (String cookieHeader : cookieHeaders) { + if (cookieHeader.contains("name=")) { + assertTrue(cookieHeader.contains("name=val?ue")); + } + } + } } diff --git a/test/org/apache/tomcat/util/http/TestCookiesSwitchSysProps.java b/test/org/apache/tomcat/util/http/TestCookiesSwitchSysProps.java new file mode 100644 index 000000000..d61a71b4e --- /dev/null +++ b/test/org/apache/tomcat/util/http/TestCookiesSwitchSysProps.java @@ -0,0 +1,71 @@ +/* + * 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 org.apache.tomcat.util.http; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.apache.catalina.startup.Tomcat; +import org.apache.tomcat.util.buf.ByteChunk; + +/** + * Test case for {@link Cookies}. Note because of the use of final + * static constants in {@link Cookies}, each of these tests must be + * executed in a new JVM instance. The tests have been place in separate classes + * to facilitate this when running the unit tests via Ant. + */ +public class TestCookiesSwitchSysProps extends CookiesBaseTest { + + @Override + public void testCookiesInstance() throws Exception { + + System.setProperty("org.apache.catalina.STRICT_SERVLET_COMPLIANCE", + "true"); + System.setProperty( + "org.apache.tomcat.util.http.ServerCookie.ALLOW_VERSION_SWITCH", + "true"); + + Tomcat tomcat = getTomcatInstance(); + + addServlets(tomcat); + + tomcat.start(); + + ByteChunk res = getUrl("http://localhost:" + getPort() + "/invalid"); + assertEquals("Cookie name fail", res.toString()); + res = getUrl("http://localhost:" + getPort() + "/invalidFwd"); + assertEquals("Cookie name fail", res.toString()); + res = getUrl("http://localhost:" + getPort() + "/invalidStrict"); + assertEquals("Cookie name fail", res.toString()); + res = getUrl("http://localhost:" + getPort() + "/valid"); + assertEquals("Cookie name ok", res.toString()); + + // Need to read response headers to test version switching + Map> headers = new HashMap>(); + getUrl("http://localhost:" + getPort() + "/switch", res, headers); + List cookieHeaders = headers.get("Set-Cookie"); + for (String cookieHeader : cookieHeaders) { + if (cookieHeader.contains("name=")) { + assertTrue(cookieHeader.contains("name=\"val?ue\"")); + } + } + + } + +} diff --git a/webapps/docs/config/systemprops.xml b/webapps/docs/config/systemprops.xml index 6d610fb81..d50bfad47 100644 --- a/webapps/docs/config/systemprops.xml +++ b/webapps/docs/config/systemprops.xml @@ -272,6 +272,16 @@ +

If this is true Tomcat will convert a v0 cookie that + contains invalid characters (i.e. separators) to a v1 cookie and add + quotes as required. If not specified, the default value will be used. If + org.apache.catalina.STRICT_SERVLET_COMPLIANCE is set to + true, the default of this setting will be false, + else the default value will be true.

+ + +

If this is true Tomcat will always add an expires parameter to a SetCookie header even for cookies with version greater than -- 2.11.0