This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 6753caa ALLOW_ENCODED_SLASH -> encodedSolidusHandling
6753caa is described below
commit 6753caa65ddcdb52f0d9b36a67db61df133ce82e
Author: Mark Thomas <[email protected]>
AuthorDate: Tue Mar 31 21:02:01 2020 +0100
ALLOW_ENCODED_SLASH -> encodedSolidusHandling
Replace the system property with a Connector attribute and add an
additional handling option of passing the %nn encoded sequence through
---
java/org/apache/catalina/connector/Connector.java | 24 +++++++++
.../apache/catalina/connector/CoyoteAdapter.java | 2 +-
java/org/apache/catalina/connector/Request.java | 6 ++-
.../tomcat/util/buf/EncodedSolidusHandling.java | 51 ++++++++++++++++++
.../apache/tomcat/util/buf/LocalStrings.properties | 2 +
java/org/apache/tomcat/util/buf/UDecoder.java | 61 +++++++++++++++++-----
.../org/apache/catalina/connector/TestRequest.java | 25 ++++-----
.../apache/catalina/core/TestAsyncContextImpl.java | 14 ++---
webapps/docs/changelog.xml | 8 +++
webapps/docs/config/ajp.xml | 15 ++++++
webapps/docs/config/http.xml | 15 ++++++
webapps/docs/config/systemprops.xml | 9 ++--
12 files changed, 190 insertions(+), 42 deletions(-)
diff --git a/java/org/apache/catalina/connector/Connector.java
b/java/org/apache/catalina/connector/Connector.java
index 0efad50..534246f 100644
--- a/java/org/apache/catalina/connector/Connector.java
+++ b/java/org/apache/catalina/connector/Connector.java
@@ -42,6 +42,7 @@ import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.IntrospectionUtils;
import org.apache.tomcat.util.buf.B2CConverter;
import org.apache.tomcat.util.buf.CharsetUtil;
+import org.apache.tomcat.util.buf.EncodedSolidusHandling;
import org.apache.tomcat.util.net.SSLHostConfig;
import org.apache.tomcat.util.net.openssl.OpenSSLImplementation;
import org.apache.tomcat.util.res.StringManager;
@@ -112,6 +113,11 @@ public class Connector extends LifecycleMBeanBase {
// Default for Connector depends on this system property
setThrowOnFailure(Boolean.getBoolean("org.apache.catalina.startup.EXIT_ON_INIT_FAILURE"));
+
+ // Default for Connector depends on this (deprecated) system property
+ if
(Boolean.parseBoolean(System.getProperty("org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH",
"false"))) {
+ encodedSolidusHandling = EncodedSolidusHandling.DECODE;
+ }
}
@@ -268,6 +274,9 @@ public class Connector extends LifecycleMBeanBase {
private Charset uriCharset = StandardCharsets.UTF_8;
+ private EncodedSolidusHandling encodedSolidusHandling =
EncodedSolidusHandling.REJECT;
+
+
/**
* URI encoding as body.
*/
@@ -884,6 +893,21 @@ public class Connector extends LifecycleMBeanBase {
}
+ public String getEncodedSolidusHandling() {
+ return encodedSolidusHandling.getValue();
+ }
+
+
+ public void setEncodedSolidusHandling(String encodedSolidusHandling) {
+ this.encodedSolidusHandling =
EncodedSolidusHandling.fromString(encodedSolidusHandling);
+ }
+
+
+ public EncodedSolidusHandling getEncodedSolidusHandlingInternal() {
+ return encodedSolidusHandling;
+ }
+
+
// --------------------------------------------------------- Public Methods
/**
diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java
b/java/org/apache/catalina/connector/CoyoteAdapter.java
index 8f538e3..0f5a289 100644
--- a/java/org/apache/catalina/connector/CoyoteAdapter.java
+++ b/java/org/apache/catalina/connector/CoyoteAdapter.java
@@ -630,7 +630,7 @@ public class CoyoteAdapter implements Adapter {
// URI decoding
// %xx decoding of the URL
try {
- req.getURLDecoder().convert(decodedURI.getByteChunk(), false);
+ req.getURLDecoder().convert(decodedURI.getByteChunk(),
connector.getEncodedSolidusHandlingInternal());
} catch (IOException ioe) {
response.sendError(400, "Invalid URI: " + ioe.getMessage());
}
diff --git a/java/org/apache/catalina/connector/Request.java
b/java/org/apache/catalina/connector/Request.java
index 705b609..6d913f0 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -96,6 +96,7 @@ import org.apache.tomcat.InstanceManager;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.buf.B2CConverter;
import org.apache.tomcat.util.buf.ByteChunk;
+import org.apache.tomcat.util.buf.EncodedSolidusHandling;
import org.apache.tomcat.util.buf.MessageBytes;
import org.apache.tomcat.util.buf.StringUtils;
import org.apache.tomcat.util.buf.UDecoder;
@@ -2181,8 +2182,9 @@ public class Request implements HttpServletRequest {
while (pos < len) {
if (uri[pos] == '/') {
return pos;
- } else if (UDecoder.ALLOW_ENCODED_SLASH && uri[pos] == '%' && pos
+ 2 < len &&
- uri[pos+1] == '2' && (uri[pos + 2] == 'f' || uri[pos + 2]
== 'F')) {
+ } else if (connector.getEncodedSolidusHandlingInternal() ==
EncodedSolidusHandling.DECODE &&
+ uri[pos] == '%' && pos + 2 < len && uri[pos+1] == '2' &&
+ (uri[pos + 2] == 'f' || uri[pos + 2] == 'F')) {
return pos;
}
pos++;
diff --git a/java/org/apache/tomcat/util/buf/EncodedSolidusHandling.java
b/java/org/apache/tomcat/util/buf/EncodedSolidusHandling.java
new file mode 100644
index 0000000..6d75dbf
--- /dev/null
+++ b/java/org/apache/tomcat/util/buf/EncodedSolidusHandling.java
@@ -0,0 +1,51 @@
+/*
+ * 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.buf;
+
+import java.util.Locale;
+
+import org.apache.tomcat.util.res.StringManager;
+
+public enum EncodedSolidusHandling {
+ DECODE("decode"),
+ REJECT("reject"),
+ PASS_THROUGH("passthrough");
+
+ private static final StringManager sm =
StringManager.getManager(EncodedSolidusHandling.class);
+
+ private final String value;
+
+ EncodedSolidusHandling(String value) {
+ this.value = value;
+ }
+
+ public String getValue() {
+ return value;
+ }
+
+ public static EncodedSolidusHandling fromString(String from) {
+ String trimmedLower = from.trim().toLowerCase(Locale.ENGLISH);
+
+ for (EncodedSolidusHandling value : EncodedSolidusHandling.values()) {
+ if (value.getValue().equals(trimmedLower)) {
+ return value;
+ }
+ }
+
+ throw new
IllegalStateException(sm.getString("encodedSolidusHandling.invalid", from));
+ }
+}
diff --git a/java/org/apache/tomcat/util/buf/LocalStrings.properties
b/java/org/apache/tomcat/util/buf/LocalStrings.properties
index 7badd5b..2dd62b9 100644
--- a/java/org/apache/tomcat/util/buf/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/buf/LocalStrings.properties
@@ -22,6 +22,8 @@ byteBufferUtils.cleaner=Cannot use direct ByteBuffer cleaner,
memory leaking may
chunk.overflow=Buffer overflow and no sink is set, limit [{0}] and buffer
length [{1}]
+encodedSolidusHandling.invalid=The value [{0}] is not recognised
+
hexUtils.fromHex.nonHex=The input must consist only of hex digits
hexUtils.fromHex.oddDigits=The input must consist of an even number of hex
digits
diff --git a/java/org/apache/tomcat/util/buf/UDecoder.java
b/java/org/apache/tomcat/util/buf/UDecoder.java
index 5421a18..f560221 100644
--- a/java/org/apache/tomcat/util/buf/UDecoder.java
+++ b/java/org/apache/tomcat/util/buf/UDecoder.java
@@ -37,8 +37,9 @@ public final class UDecoder {
private static final StringManager sm =
StringManager.getManager(UDecoder.class);
+ @Deprecated
public static final boolean ALLOW_ENCODED_SLASH =
-
Boolean.parseBoolean(System.getProperty("org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH",
"false"));
+
Boolean.parseBoolean(System.getProperty("org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH",
"false"));
private static class DecodeException extends CharConversionException {
private static final long serialVersionUID = 1L;
@@ -64,21 +65,45 @@ public final class UDecoder {
private static final IOException EXCEPTION_SLASH = new DecodeException(
"noSlash");
- public UDecoder()
- {
+
+ /**
+ * URLDecode, will modify the source. Assumes source bytes are encoded
using
+ * a superset of US-ASCII as per RFC 7230. "%2f" will be rejected unless
the
+ * input is a query string.
+ *
+ * @param mb The URL encoded bytes
+ * @param query {@code true} if this is a query string. For a query string
+ * '+' will be decoded to ' '
+ *
+ * @throws IOException Invalid %xx URL encoding
+ */
+ public void convert(ByteChunk mb, boolean query) throws IOException {
+ if (query) {
+ convert(mb, true, EncodedSolidusHandling.DECODE);
+ } else {
+ convert(mb, false, EncodedSolidusHandling.REJECT);
+ }
}
+
/**
* URLDecode, will modify the source. Assumes source bytes are encoded
using
* a superset of US-ASCII as per RFC 7230.
*
- * @param mb The URL encoded bytes
- * @param query <code>true</code> if this is a query string
+ * @param mb The URL encoded bytes
+ * @param encodedSolidusHandling How should the %2f sequence handled by
+ * the decoder? For query strings this
+ * parameter will be ignored and the
+ * %2f sequence will be decoded
* @throws IOException Invalid %xx URL encoding
*/
- public void convert( ByteChunk mb, boolean query )
- throws IOException
- {
+ public void convert(ByteChunk mb, EncodedSolidusHandling
encodedSolidusHandling) throws IOException {
+ convert(mb, false, encodedSolidusHandling);
+ }
+
+
+ private void convert(ByteChunk mb, boolean query, EncodedSolidusHandling
encodedSolidusHandling) throws IOException {
+
int start=mb.getOffset();
byte buff[]=mb.getBytes();
int end=mb.getEnd();
@@ -97,8 +122,6 @@ public final class UDecoder {
idx=idx2;
}
- final boolean noSlash = !(ALLOW_ENCODED_SLASH || query);
-
for( int j=idx; j<end; j++, idx++ ) {
if( buff[ j ] == '+' && query) {
buff[idx]= (byte)' ' ;
@@ -117,10 +140,22 @@ public final class UDecoder {
j+=2;
int res=x2c( b1, b2 );
- if (noSlash && (res == '/')) {
- throw EXCEPTION_SLASH;
+ if (res == '/') {
+ switch (encodedSolidusHandling) {
+ case DECODE: {
+ buff[idx]=(byte)res;
+ break;
+ }
+ case REJECT: {
+ throw EXCEPTION_SLASH;
+ }
+ case PASS_THROUGH: {
+ idx += 2;
+ }
+ }
+ } else {
+ buff[idx]=(byte)res;
}
- buff[idx]=(byte)res;
}
}
diff --git a/test/org/apache/catalina/connector/TestRequest.java
b/test/org/apache/catalina/connector/TestRequest.java
index 5204994..963e8f4 100644
--- a/test/org/apache/catalina/connector/TestRequest.java
+++ b/test/org/apache/catalina/connector/TestRequest.java
@@ -42,7 +42,6 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.junit.Assert;
-import org.junit.BeforeClass;
import org.junit.Ignore;
import org.junit.Test;
@@ -55,6 +54,7 @@ import org.apache.catalina.startup.Tomcat;
import org.apache.catalina.startup.TomcatBaseTest;
import org.apache.tomcat.unittest.TesterRequest;
import org.apache.tomcat.util.buf.ByteChunk;
+import org.apache.tomcat.util.buf.EncodedSolidusHandling;
import org.apache.tomcat.util.descriptor.web.FilterDef;
import org.apache.tomcat.util.descriptor.web.FilterMap;
import org.apache.tomcat.util.descriptor.web.LoginConfig;
@@ -64,12 +64,6 @@ import org.apache.tomcat.util.descriptor.web.LoginConfig;
*/
public class TestRequest extends TomcatBaseTest {
- @BeforeClass
- public static void setup() {
- // Some of these tests need this and it used statically so set it once
-
System.setProperty("org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH",
"true");
- }
-
/**
* Test case for https://bz.apache.org/bugzilla/show_bug.cgi?id=37794
* POST parameters are not returned from a call to
@@ -804,12 +798,12 @@ public class TestRequest extends TomcatBaseTest {
@Test
public void testBug57215c() throws Exception {
- doBug56501("/path", "/%2Fpath", "/%2Fpath");
+ doBug56501("/path", "/%2Fpath", "/%2Fpath",
EncodedSolidusHandling.DECODE);
}
@Test
public void testBug57215d() throws Exception {
- doBug56501("/path", "/%2Fpath%2F", "/%2Fpath");
+ doBug56501("/path", "/%2Fpath%2F", "/%2Fpath",
EncodedSolidusHandling.DECODE);
}
@Test
@@ -819,15 +813,22 @@ public class TestRequest extends TomcatBaseTest {
@Test
public void testBug57215f() throws Exception {
- doBug56501("/path", "/foo/..%2fpath", "/foo/..%2fpath");
+ doBug56501("/path", "/foo/..%2fpath", "/foo/..%2fpath",
EncodedSolidusHandling.DECODE);
}
- private void doBug56501(String deployPath, String requestPath, String
expected)
- throws Exception {
+ private void doBug56501(String deployPath, String requestPath, String
expected) throws Exception {
+ doBug56501(deployPath, requestPath, expected,
EncodedSolidusHandling.REJECT);
+ }
+
+
+ private void doBug56501(String deployPath, String requestPath, String
expected,
+ EncodedSolidusHandling encodedSolidusHandling) throws Exception {
// Setup Tomcat instance
Tomcat tomcat = getTomcatInstance();
+
tomcat.getConnector().setEncodedSolidusHandling(encodedSolidusHandling.getValue());
+
// No file system docBase required
Context ctx = tomcat.addContext(deployPath, null);
ctx.setAllowMultipleLeadingForwardSlashInPath(true);
diff --git a/test/org/apache/catalina/core/TestAsyncContextImpl.java
b/test/org/apache/catalina/core/TestAsyncContextImpl.java
index 2140203..3f29ec2 100644
--- a/test/org/apache/catalina/core/TestAsyncContextImpl.java
+++ b/test/org/apache/catalina/core/TestAsyncContextImpl.java
@@ -49,7 +49,6 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.junit.Assert;
-import org.junit.Before;
import org.junit.Test;
import org.apache.catalina.Context;
@@ -63,6 +62,7 @@ import org.apache.catalina.startup.TomcatBaseTest;
import org.apache.catalina.valves.TesterAccessLogValve;
import org.apache.tomcat.unittest.TesterContext;
import org.apache.tomcat.util.buf.ByteChunk;
+import org.apache.tomcat.util.buf.EncodedSolidusHandling;
import org.apache.tomcat.util.collections.CaseInsensitiveKeyMap;
import org.apache.tomcat.util.descriptor.web.ErrorPage;
import org.easymock.EasyMock;
@@ -2670,21 +2670,13 @@ public class TestAsyncContextImpl extends
TomcatBaseTest {
}
- @Override
- @Before
- public void setUp() throws Exception {
- super.setUp();
- // Required by testBug61185()
- // Does not impact other tests in this class
-
System.setProperty("org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH",
"true");
- }
-
-
@Test
public void testBug61185() throws Exception {
// Setup Tomcat instance
Tomcat tomcat = getTomcatInstance();
+
tomcat.getConnector().setEncodedSolidusHandling(EncodedSolidusHandling.DECODE.getValue());
+
// No file system docBase required
Context ctx = tomcat.addContext("", null);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 3986c71..174f121 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -97,6 +97,14 @@
request attributes <code>org.apache.coyote.connectionID</code> and
<code>org.apache.coyote.streamID</code> respectively. (markt)
</add>
+ <add>
+ Replace the system property
+ <code>org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH</code>
+ with the Connector attribute <code>encodedSolidusHandling</code> that
+ adds an additional option to pass the <code>%2f</code> sequence through
+ to the application without decoding it in addition to rejecting such
+ sequences and decoding such sequences. (markt)
+ </add>
</changelog>
</subsection>
<subsection name="Jasper">
diff --git a/webapps/docs/config/ajp.xml b/webapps/docs/config/ajp.xml
index fa8fc0f..5b20d6f 100644
--- a/webapps/docs/config/ajp.xml
+++ b/webapps/docs/config/ajp.xml
@@ -113,6 +113,21 @@
By default, DNS lookups are disabled.</p>
</attribute>
+ <attribute name="encodedSolidusHandling" required="false">
+ <p>When set to <code>reject</code> request paths containing a
+ <code>%2f</code> sequence will be rejected with a 400 response. When set
+ to <code>decode</code> request paths containing a <code>%2f</code>
+ sequence will have that sequence decoded to <code>/</code> at the same
+ time other <code>%nn</code> sequences are decoded. When set to
+ <code>passthrough</code> request paths containing a <code>%2f</code>
+ sequence will be processed with the <code>%2f</code> sequence unchanged.
+ If not specified the default value is <code>reject</code>. This default
+ may be modified if the deprecated <a href="systemprops.html">system
+ property</a>
+ <code>org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH</code> is
+ set.</p>
+ </attribute>
+
<attribute name="maxHeaderCount" required="false">
<p>The maximum number of headers in a request that are allowed by the
container. A request that contains more headers than the specified limit
diff --git a/webapps/docs/config/http.xml b/webapps/docs/config/http.xml
index 4efff4b..ed62ffb 100644
--- a/webapps/docs/config/http.xml
+++ b/webapps/docs/config/http.xml
@@ -114,6 +114,21 @@
By default, DNS lookups are disabled.</p>
</attribute>
+ <attribute name="encodedSolidusHandling" required="false">
+ <p>When set to <code>reject</code> request paths containing a
+ <code>%2f</code> sequence will be rejected with a 400 response. When set
+ to <code>decode</code> request paths containing a <code>%2f</code>
+ sequence will have that sequence decoded to <code>/</code> at the same
+ time other <code>%nn</code> sequences are decoded. When set to
+ <code>passthrough</code> request paths containing a <code>%2f</code>
+ sequence will be processed with the <code>%2f</code> sequence unchanged.
+ If not specified the default value is <code>reject</code>. This default
+ may be modified if the deprecated <a href="systemprops.html">system
+ property</a>
+ <code>org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH</code> is
+ set.</p>
+ </attribute>
+
<attribute name="maxHeaderCount" required="false">
<p>The maximum number of headers in a request that are allowed by the
container. A request that contains more headers than the specified limit
diff --git a/webapps/docs/config/systemprops.xml
b/webapps/docs/config/systemprops.xml
index f060639..1cff7e8 100644
--- a/webapps/docs/config/systemprops.xml
+++ b/webapps/docs/config/systemprops.xml
@@ -278,9 +278,12 @@
<property
name="org.apache.tomcat.util.buf. UDecoder.ALLOW_ENCODED_SLASH">
- <p>If this is <code>true</code> '%2F' will be permitted as a path
- delimiter.</p>
- <p>If not specified, the default value of <code>false</code> will be
used.</p>
+ <p>Use of this system property is deprecated. It will be removed from
+ Tomcat 10 onwards.</p>
+ <p>If this system property is set to <code>true</code>, the default for
+ the <code>encodedSolidusHandling</code> attribute of all Connectors will
+ be changed from <code>reject</code> to <code>decode</code>. If decoded,
it
+ will be treated a path delimiter.</p>
</property>
</properties>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
