This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
     new 893aa61  ALLOW_ENCODED_SLASH -> encodedSolidusHandling
893aa61 is described below

commit 893aa61983a78afca072124b412274e185a4a577
Author: Mark Thomas <ma...@apache.org>
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, 188 insertions(+), 44 deletions(-)

diff --git a/java/org/apache/catalina/connector/Connector.java 
b/java/org/apache/catalina/connector/Connector.java
index d78afff..62e4384 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;
@@ -105,6 +106,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;
+        }
     }
 
 
@@ -282,6 +288,9 @@ public class Connector extends LifecycleMBeanBase  {
     private Charset uriCharset = StandardCharsets.UTF_8;
 
 
+    private EncodedSolidusHandling encodedSolidusHandling = 
EncodedSolidusHandling.REJECT;
+
+
     /**
      * URI encoding as body.
      */
@@ -924,6 +933,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 9bfe301..4a4e32a 100644
--- a/java/org/apache/catalina/connector/CoyoteAdapter.java
+++ b/java/org/apache/catalina/connector/CoyoteAdapter.java
@@ -626,7 +626,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 e36debf..74f9ad5 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -95,6 +95,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;
@@ -2149,8 +2150,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 73c7238..d917ca0 100644
--- a/java/org/apache/tomcat/util/buf/UDecoder.java
+++ b/java/org/apache/tomcat/util/buf/UDecoder.java
@@ -37,9 +37,6 @@ public final class UDecoder {
 
     private static final StringManager sm = 
StringManager.getManager(UDecoder.class);
 
-    public static final boolean ALLOW_ENCODED_SLASH =
-        
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;
         public DecodeException(String s) {
@@ -64,21 +61,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 +118,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 +136,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 b8467e4..be2ad40 100644
--- a/test/org/apache/catalina/connector/TestRequest.java
+++ b/test/org/apache/catalina/connector/TestRequest.java
@@ -42,7 +42,6 @@ import jakarta.servlet.http.HttpServletRequest;
 import jakarta.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 cd0d935..1e2122f 100644
--- a/test/org/apache/catalina/core/TestAsyncContextImpl.java
+++ b/test/org/apache/catalina/core/TestAsyncContextImpl.java
@@ -49,7 +49,6 @@ import jakarta.servlet.http.HttpServletRequest;
 import jakarta.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 49fd838..c6ebff4 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -150,6 +150,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 d71a5d1..a746da2 100644
--- a/webapps/docs/config/http.xml
+++ b/webapps/docs/config/http.xml
@@ -118,6 +118,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="enforceEncodingInGetWriter" required="false">
       <p>If this is <code>true</code> then
       a call to <code>Response.getWriter()</code> if no character encoding
diff --git a/webapps/docs/config/systemprops.xml 
b/webapps/docs/config/systemprops.xml
index 37c5d93..dbd9d70 100644
--- a/webapps/docs/config/systemprops.xml
+++ b/webapps/docs/config/systemprops.xml
@@ -120,9 +120,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: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to