This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push: new aaf0e97362 Add noEqualsCookie option with changed default: was "name", now "ignore" aaf0e97362 is described below commit aaf0e97362148b11ebf9ea69c0c4a714f188ade6 Author: Mark Thomas <ma...@apache.org> AuthorDate: Tue Aug 20 11:53:37 2024 +0100 Add noEqualsCookie option with changed default: was "name", now "ignore" --- .../tomcat/util/http/CookieProcessorBase.java | 17 ++++ .../tomcat/util/http/LocalStrings.properties | 2 + .../tomcat/util/http/LocalStrings_fr.properties | 2 + .../tomcat/util/http/LocalStrings_ko.properties | 2 + .../tomcat/util/http/LocalStrings_zh_CN.properties | 2 + .../apache/tomcat/util/http/NoEqualsCookie.java | 60 ++++++++++++++ .../tomcat/util/http/Rfc6265CookieProcessor.java | 3 +- .../org/apache/tomcat/util/http/parser/Cookie.java | 44 +++++++++- .../apache/tomcat/util/http/TestCookieParsing.java | 39 +++++++-- test/org/apache/tomcat/util/http/TestCookies.java | 93 +++++++++++++++++++++- webapps/docs/changelog.xml | 5 ++ webapps/docs/config/cookie-processor.xml | 9 +++ 12 files changed, 263 insertions(+), 15 deletions(-) diff --git a/java/org/apache/tomcat/util/http/CookieProcessorBase.java b/java/org/apache/tomcat/util/http/CookieProcessorBase.java index 5815ca4cd4..54cb7de5f4 100644 --- a/java/org/apache/tomcat/util/http/CookieProcessorBase.java +++ b/java/org/apache/tomcat/util/http/CookieProcessorBase.java @@ -42,6 +42,23 @@ public abstract class CookieProcessorBase implements CookieProcessor { private boolean partitioned = false; + private NoEqualsCookie noEqualsCookie = NoEqualsCookie.NAME; + + + public String getNoEqualsCookie() { + return noEqualsCookie.getValue(); + } + + + protected NoEqualsCookie getNoEqualsCookieInternal() { + return noEqualsCookie; + } + + + public void setNoEqualsCookie(String noEqualsCookie) { + this.noEqualsCookie = NoEqualsCookie.fromString(noEqualsCookie); + } + public SameSiteCookies getSameSiteCookies() { return sameSiteCookies; diff --git a/java/org/apache/tomcat/util/http/LocalStrings.properties b/java/org/apache/tomcat/util/http/LocalStrings.properties index a8f60eb911..f670b79cb9 100644 --- a/java/org/apache/tomcat/util/http/LocalStrings.properties +++ b/java/org/apache/tomcat/util/http/LocalStrings.properties @@ -22,6 +22,8 @@ cookies.maxCountFail=More than the maximum allowed number of cookies, [{0}], wer headers.maxCountFail=More than the maximum allowed number of headers, [{0}], were detected. +noEqualsCookie.invalid=The value [{0}] is not recognised + parameters.bytes=Start processing with input [{0}] parameters.copyFail=Failed to create copy of original parameter values for debug logging purposes parameters.decodeFail.debug=Character decoding failed. Parameter [{0}] with value [{1}] has been ignored. diff --git a/java/org/apache/tomcat/util/http/LocalStrings_fr.properties b/java/org/apache/tomcat/util/http/LocalStrings_fr.properties index 725fc96e07..220337bc53 100644 --- a/java/org/apache/tomcat/util/http/LocalStrings_fr.properties +++ b/java/org/apache/tomcat/util/http/LocalStrings_fr.properties @@ -22,6 +22,8 @@ cookies.maxCountFail=Le nombre maximum de cookies [{0}] est dépassé headers.maxCountFail=Le nombre d''en-têtes [{0}] dépasse le maximum autorisé +noEqualsCookie.invalid=La valeur [{0}] n''est pas reconnue + parameters.bytes=Début du traitement avec les données [{0}] parameters.copyFail=Echec de la copie des valeurs de paramètres originaux pour raisons de journalisation du déboguage parameters.decodeFail.debug=Echec de décodage de caractère, le paramètre [{0}] de valeur [{1}] a été ignoré diff --git a/java/org/apache/tomcat/util/http/LocalStrings_ko.properties b/java/org/apache/tomcat/util/http/LocalStrings_ko.properties index 274ec8711e..8fd24e7ae9 100644 --- a/java/org/apache/tomcat/util/http/LocalStrings_ko.properties +++ b/java/org/apache/tomcat/util/http/LocalStrings_ko.properties @@ -22,6 +22,8 @@ cookies.maxCountFail=허용된 최대 쿠키 개수 [{0}]을(를) 초과한 쿠 headers.maxCountFail=최대 허용 헤더 개수 [{0}]보다 더 많은 헤더들이 탐지되었습니다. +noEqualsCookie.invalid=해당 값 [{0}]은(는) 인식되지 않습니다. + parameters.bytes=입력 [{0}]을(를) 사용하여 처리를 시작합니다. parameters.copyFail=디버그 로그를 위한 원래의 파라미터 값들을 복사하지 못했습니다. parameters.decodeFail.debug=문자 디코딩 실패. 값 [{1}](으)로 설정된 파라미터 [{0}]은(는) 무시됩니다. diff --git a/java/org/apache/tomcat/util/http/LocalStrings_zh_CN.properties b/java/org/apache/tomcat/util/http/LocalStrings_zh_CN.properties index cee2400e84..105b70fcad 100644 --- a/java/org/apache/tomcat/util/http/LocalStrings_zh_CN.properties +++ b/java/org/apache/tomcat/util/http/LocalStrings_zh_CN.properties @@ -22,6 +22,8 @@ cookies.maxCountFail=检测到超过Cookie最大允许的数量[{0}] headers.maxCountFail=检测到超过了允许设置的最大header 数[{0}] +noEqualsCookie.invalid=值[{0}]未识别 + parameters.bytes=开始处理输入[{0}] parameters.copyFail=无法创建以调试日志记录为目的的原始参数值的副本 parameters.decodeFail.debug=字符解码失败.参数 [{0}]和值 [{1}]被忽略 diff --git a/java/org/apache/tomcat/util/http/NoEqualsCookie.java b/java/org/apache/tomcat/util/http/NoEqualsCookie.java new file mode 100644 index 0000000000..8d47102b95 --- /dev/null +++ b/java/org/apache/tomcat/util/http/NoEqualsCookie.java @@ -0,0 +1,60 @@ +/* + * 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.Locale; + +import org.apache.tomcat.util.res.StringManager; + +public enum NoEqualsCookie { + IGNORE("ignore"), + NAME("name"); + /* + * There is no VALUE option since the Servlet specification does not permit the creation of a Cookie with a name + * that is either null or the zero length string. + * + * The historical intention (from the user agent perspective) of using a name-value-pair without an equals sign has + * been to indicate a cookie with a name but no value. Tomcat has done the opposite. The current RFC6265bis text + * treats a name-value-pair without an equals sign as a cookie with a value but no name. Supporting this will + * require changes to the Servlet specifcation. + */ + + + private static final StringManager sm = StringManager.getManager(NoEqualsCookie.class); + + private final String value; + + NoEqualsCookie(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + + public static NoEqualsCookie fromString(String from) { + String trimmedLower = from.trim().toLowerCase(Locale.ENGLISH); + + for (NoEqualsCookie value : values()) { + if (value.getValue().equals(trimmedLower)) { + return value; + } + } + + throw new IllegalStateException(sm.getString("noEqualsCookie.invalid", from)); + } +} diff --git a/java/org/apache/tomcat/util/http/Rfc6265CookieProcessor.java b/java/org/apache/tomcat/util/http/Rfc6265CookieProcessor.java index 7ace9b1ee2..06060aa5ba 100644 --- a/java/org/apache/tomcat/util/http/Rfc6265CookieProcessor.java +++ b/java/org/apache/tomcat/util/http/Rfc6265CookieProcessor.java @@ -91,7 +91,8 @@ public class Rfc6265CookieProcessor extends CookieProcessorBase { } ByteChunk bc = cookieValue.getByteChunk(); - Cookie.parseCookie(bc.getBytes(), bc.getStart(), bc.getLength(), serverCookies); + Cookie.parseCookie(bc.getBytes(), bc.getStart(), bc.getLength(), serverCookies, + getNoEqualsCookieInternal()); } // search from the next position diff --git a/java/org/apache/tomcat/util/http/parser/Cookie.java b/java/org/apache/tomcat/util/http/parser/Cookie.java index 63b30d76c7..0d7c2c567e 100644 --- a/java/org/apache/tomcat/util/http/parser/Cookie.java +++ b/java/org/apache/tomcat/util/http/parser/Cookie.java @@ -20,6 +20,7 @@ import java.nio.charset.StandardCharsets; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; +import org.apache.tomcat.util.http.NoEqualsCookie; import org.apache.tomcat.util.http.ServerCookie; import org.apache.tomcat.util.http.ServerCookies; import org.apache.tomcat.util.log.UserDataHelper; @@ -89,7 +90,33 @@ public class Cookie { } + /** + * Parse byte array as cookie header. + * + * @param bytes Source + * @param offset Start point in array + * @param len Number of bytes to read + * @param serverCookies Structure to store results + * + * @deprecated Unused. This method will be removed in Tomcat 11 onwards. + */ + @Deprecated public static void parseCookie(byte[] bytes, int offset, int len, ServerCookies serverCookies) { + parseCookie(bytes, offset, len, serverCookies, NoEqualsCookie.IGNORE); + } + + + /** + * Parse byte array as cookie header. + * + * @param bytes Source + * @param offset Start point in array + * @param len Number of bytes to read + * @param serverCookies Structure to store results + * @param noEqualsCookie How to handle a cookie name-value-pair that does not contain an equals character + */ + public static void parseCookie(byte[] bytes, int offset, int len, ServerCookies serverCookies, + NoEqualsCookie noEqualsCookie) { // ByteBuffer is used throughout this parser as it allows the byte[] // and position information to be easily passed between parsing methods @@ -133,11 +160,22 @@ public class Cookie { } if (name.hasRemaining()) { - ServerCookie sc = serverCookies.addCookie(); - sc.getName().setBytes(name.array(), name.position(), name.remaining()); if (value == null) { - sc.getValue().setBytes(EMPTY_BYTES, 0, EMPTY_BYTES.length); + switch (noEqualsCookie) { + case IGNORE: { + // This name-value-pair is a NO-OP + break; + } + case NAME: { + ServerCookie sc = serverCookies.addCookie(); + sc.getName().setBytes(name.array(), name.position(), name.remaining()); + sc.getValue().setBytes(EMPTY_BYTES, 0, EMPTY_BYTES.length); + break; + } + } } else { + ServerCookie sc = serverCookies.addCookie(); + sc.getName().setBytes(name.array(), name.position(), name.remaining()); sc.getValue().setBytes(value.array(), value.position(), value.remaining()); } } diff --git a/test/org/apache/tomcat/util/http/TestCookieParsing.java b/test/org/apache/tomcat/util/http/TestCookieParsing.java index eb97917f39..7bb636f9f4 100644 --- a/test/org/apache/tomcat/util/http/TestCookieParsing.java +++ b/test/org/apache/tomcat/util/http/TestCookieParsing.java @@ -39,9 +39,13 @@ public class TestCookieParsing extends TomcatBaseTest { private static final String[] COOKIES_WITH_EQUALS = new String[] { "name=equals=middle", "name==equalsstart", "name=equalsend=" }; - private static final String[] COOKIES_WITH_NAME_ONLY = new String[] { - "bob", "bob=" }; - private static final String COOKIES_WITH_NAME_ONLY_CONCAT = "bob=bob="; + private static final String[] COOKIES_WITH_NAME_OR_VALUE_ONLY = new String[] { "bob=", "bob", "=bob" }; + + // First two are treated as name and no value, third is invalid (therefore ignored) + private static final String COOKIES_WITH_NAME_OR_VALUE_ONLY_NAME_CONCAT = "bob=bob="; + + // First is treated as name and no value, second is ignored and third is invalid (therefore ignored) + private static final String COOKIES_WITH_NAME_OR_VALUE_ONLY_IGNORE_CONCAT = "bob="; private static final String[] COOKIES_WITH_SEPS = new String[] { "name=val/ue" }; @@ -70,11 +74,30 @@ public class TestCookieParsing extends TomcatBaseTest { @Test - public void testRfc6265NameOnly() throws Exception { - // Always allows equals - TestCookieParsingClient client = new TestCookieParsingClient( - new Rfc6265CookieProcessor(), COOKIES_WITH_NAME_ONLY, - COOKIES_WITH_NAME_ONLY_CONCAT); + public void testRfc6265NameOrValueOnlyName() throws Exception { + doTestRfc6265NoEquals("name", COOKIES_WITH_NAME_OR_VALUE_ONLY_NAME_CONCAT); + } + + + @Test + public void testRfc6265NameOrValueOnlyIgnore() throws Exception { + doTestRfc6265NoEquals("ignore", COOKIES_WITH_NAME_OR_VALUE_ONLY_IGNORE_CONCAT); + } + + + @Test + public void testRfc6265NameOrValueOnlyDefault() throws Exception { + doTestRfc6265NoEquals(null, COOKIES_WITH_NAME_OR_VALUE_ONLY_NAME_CONCAT); + } + + + private void doTestRfc6265NoEquals(String cookieNoEquals, String expected) throws Exception { + Rfc6265CookieProcessor cookieProcessor = new Rfc6265CookieProcessor(); + if (cookieNoEquals != null) { + cookieProcessor.setNoEqualsCookie(cookieNoEquals); + } + TestCookieParsingClient client = new TestCookieParsingClient(cookieProcessor, COOKIES_WITH_NAME_OR_VALUE_ONLY, + expected); client.doRequest(); } diff --git a/test/org/apache/tomcat/util/http/TestCookies.java b/test/org/apache/tomcat/util/http/TestCookies.java index 265801789f..c18c579e6f 100644 --- a/test/org/apache/tomcat/util/http/TestCookies.java +++ b/test/org/apache/tomcat/util/http/TestCookies.java @@ -26,6 +26,9 @@ import org.junit.Test; import org.apache.tomcat.util.buf.MessageBytes; public class TestCookies { + private static final String NAME = "NAME"; + private static final String IGNORE = "IGNORE"; + private final Cookie FOO = new Cookie("foo", "bar"); private final Cookie FOO_EMPTY = new Cookie("foo", ""); private final Cookie FOO_CONTROL = new Cookie("foo", "b\u00e1r"); @@ -51,8 +54,53 @@ public class TestCookies { test("foo=bar;a=b; ;", FOO, A); } + + @Test + public void testNameOnlyAreDroppedRfc6265NoEqualsName() { + // Name only cookies are not dropped in RFC6265 + test("foo=;a=b; ;", NAME, FOO_EMPTY, A); + test("foo;a=b; ;", NAME, FOO_EMPTY, A); + test("foo;a=b;bar", NAME, FOO_EMPTY, A, BAR_EMPTY); + test("foo;a=b;bar;", NAME, FOO_EMPTY, A, BAR_EMPTY); + test("foo;a=b;bar ", NAME, FOO_EMPTY, A, BAR_EMPTY); + test("foo;a=b;bar ;", NAME, FOO_EMPTY, A, BAR_EMPTY); + + // Bug 49000 + Cookie fred = new Cookie("fred", "1"); + Cookie jim = new Cookie("jim", "2"); + Cookie bobEmpty = new Cookie("bob", ""); + Cookie george = new Cookie("george", "3"); + test("fred=1; jim=2; bob", NAME, fred, jim, bobEmpty); + test("fred=1; jim=2; bob; george=3", NAME, fred, jim, bobEmpty, george); + test("fred=1; jim=2; bob=; george=3", NAME, fred, jim, bobEmpty, george); + test("fred=1; jim=2; bob=", NAME, fred, jim, bobEmpty); + } + + + @Test + public void testNameOnlyAreDroppedRfc6265NoEqualsIgnore() { + // Name only cookies are not dropped in RFC6265 + test("foo=;a=b; ;", IGNORE, FOO_EMPTY, A); + test("foo;a=b; ;", IGNORE, A); + test("foo;a=b;bar", IGNORE, A); + test("foo;a=b;bar;", IGNORE, A); + test("foo;a=b;bar ", IGNORE, A); + test("foo;a=b;bar ;", IGNORE, A); + + // Bug 49000 + Cookie fred = new Cookie("fred", "1"); + Cookie jim = new Cookie("jim", "2"); + Cookie bobEmpty = new Cookie("bob", ""); + Cookie george = new Cookie("george", "3"); + test("fred=1; jim=2; bob", IGNORE, fred, jim); + test("fred=1; jim=2; bob; george=3", IGNORE, fred, jim, george); + test("fred=1; jim=2; bob=; george=3", IGNORE, fred, jim, bobEmpty, george); + test("fred=1; jim=2; bob=", IGNORE, fred, jim, bobEmpty); + } + + @Test - public void testNameOnlyAreDroppedRfc6265() { + public void testNameOnlyAreDroppedRfc6265NoEqualsDefault() { // Name only cookies are not dropped in RFC6265 test("foo=;a=b; ;", FOO_EMPTY, A); test("foo;a=b; ;", FOO_EMPTY, A); @@ -72,6 +120,7 @@ public class TestCookies { test("fred=1; jim=2; bob=", fred, jim, bobEmpty); } + @Test public void testQuotedValueRfc6265() { test("foo=bar;a=\"b\"", FOO, A); @@ -79,7 +128,29 @@ public class TestCookies { } @Test - public void testEmptyPairsRfc6265() { + public void testEmptyPairsRfc6265CookieNoEqualsName() { + test("foo;a=b; ;bar", NAME, FOO_EMPTY, A, BAR_EMPTY); + test("foo;a=b;;bar", NAME, FOO_EMPTY, A, BAR_EMPTY); + test("foo;a=b; ;;bar=rab", NAME, FOO_EMPTY, A, BAR); + test("foo;a=b;; ;bar=rab", NAME, FOO_EMPTY, A, BAR); + test("foo;a=b;;#;bar=rab", NAME, FOO_EMPTY, A, HASH_EMPTY, BAR); + test("foo;a=b;;\\;bar=rab", NAME, FOO_EMPTY, A, BAR); + } + + + @Test + public void testEmptyPairsRfc6265CookieNoEqualsIgnore() { + test("foo;a=b; ;bar", IGNORE, A); + test("foo;a=b;;bar", IGNORE, A); + test("foo;a=b; ;;bar=rab", IGNORE, A, BAR); + test("foo;a=b;; ;bar=rab", IGNORE, A, BAR); + test("foo;a=b;;#;bar=rab", IGNORE, A, BAR); + test("foo;a=b;;\\;bar=rab", IGNORE,A, BAR); + } + + + @Test + public void testEmptyPairsRfc6265CookieNoEqualsDefault() { test("foo;a=b; ;bar", FOO_EMPTY, A, BAR_EMPTY); test("foo;a=b;;bar", FOO_EMPTY, A, BAR_EMPTY); test("foo;a=b; ;;bar=rab", FOO_EMPTY, A, BAR); @@ -88,6 +159,7 @@ public class TestCookies { test("foo;a=b;;\\;bar=rab", FOO_EMPTY, A, BAR); } + @Test public void testSeparatorsInValueRfc6265() { test("a=()<>@:\\\"/[]?={}\t; foo=bar", FOO); @@ -104,6 +176,12 @@ public class TestCookies { public void v1NameOnlyRfc6265() { test("$Version=1;foo=;a=b; ; ", $VERSION_1, FOO_EMPTY, A); test("$Version=1;foo= ;a=b; ; ", $VERSION_1, FOO_EMPTY, A); + + // Name + test("$Version=1;foo;a=b; ; ", NAME, $VERSION_1, FOO_EMPTY, A); + // Ignore + test("$Version=1;foo;a=b; ; ", IGNORE, $VERSION_1, A); + // Default test("$Version=1;foo;a=b; ; ", $VERSION_1, FOO_EMPTY, A); } @@ -278,9 +356,18 @@ public class TestCookies { private void test(String header, Cookie... expected) { + test(header, null, expected); + } + + + private void test(String header, String cookieNoEquals, Cookie... expected) { + MimeHeaders mimeHeaders = new MimeHeaders(); ServerCookies serverCookies = new ServerCookies(4); - CookieProcessor cookieProcessor = new Rfc6265CookieProcessor(); + Rfc6265CookieProcessor cookieProcessor = new Rfc6265CookieProcessor(); + if (cookieNoEquals != null) { + cookieProcessor.setNoEqualsCookie(cookieNoEquals); + } MessageBytes cookieHeaderValue = mimeHeaders.addValue("Cookie"); byte[] bytes = header.getBytes(StandardCharsets.UTF_8); cookieHeaderValue.setBytes(bytes, 0, bytes.length); diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index f8d8d478e9..9af06cc60e 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -114,6 +114,11 @@ circumstances this could cause a blocking read to block waiting for more data rather than return the data it had already received. (markt) </fix> + <add> + Add a new attribute <code>noEqualsCookie</code> to the + <code>Rfc6265CookieProcessor</code>. The default behaviour is unchanged. + (markt) + </add> </changelog> </subsection> <subsection name="Jasper"> diff --git a/webapps/docs/config/cookie-processor.xml b/webapps/docs/config/cookie-processor.xml index d09b5224b4..49e53a3513 100644 --- a/webapps/docs/config/cookie-processor.xml +++ b/webapps/docs/config/cookie-processor.xml @@ -99,6 +99,15 @@ <attributes> + <attribute name="noEqualsCookie" required="false"> + <p>Determines how a cookie received from a user agent should be + interpreted when the name value pair does not contain an equals sign. + The default value is <code>name</code> which means that the cookie will + be treated as a cookie with a name but no value. The other option is + <code>ignore</code> which means the cookie will be ignored. From Tomcat + 12 onwards the default will be <code>ignore</code>.</p> + </attribute> + <attribute name="partitioned" required="false"> <p>Should the Partitioned flag be set on cookies? Defaults to <code>false</code>.</p> <p>Note: The name of the attribute used to indicate a partitioned cookie as part of --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org