Author: markt
Date: Thu Sep 21 20:09:32 2017
New Revision: 1809212

URL: http://svn.apache.org/viewvc?rev=1809212&view=rev
Log:
Refactor and add additional comments to make intended behaviour clearer.

Modified:
    
tomcat/trunk/java/org/apache/catalina/webresources/AbstractFileResourceSet.java

Modified: 
tomcat/trunk/java/org/apache/catalina/webresources/AbstractFileResourceSet.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/AbstractFileResourceSet.java?rev=1809212&r1=1809211&r2=1809212&view=diff
==============================================================================
--- 
tomcat/trunk/java/org/apache/catalina/webresources/AbstractFileResourceSet.java 
(original)
+++ 
tomcat/trunk/java/org/apache/catalina/webresources/AbstractFileResourceSet.java 
Thu Sep 21 20:09:32 2017
@@ -65,50 +65,60 @@ public abstract class AbstractFileResour
             return null;
         }
 
-        if (!mustExist || file.canRead()) {
+        // If the file/dir must exist but the identified file/dir can't be read
+        // then signal that the resource was not found
+        if (mustExist && !file.canRead()) {
+            return null;
+        }
 
-            if (getRoot().getAllowLinking()) {
-                return file;
-            }
-
-            // Check that this file is located under the WebResourceSet's base
-            String canPath = null;
-            try {
-                canPath = file.getCanonicalPath();
-            } catch (IOException e) {
-                // Ignore
-            }
-            if (canPath == null)
-                return null;
-
-            if (!canPath.startsWith(canonicalBase)) {
-                return null;
-            }
-
-            // Case sensitivity check
-            // Note: We know the resource is located somewhere under base at
-            //       point. The purpose of this code is to check in a case
-            //       sensitive manner, the path to the resource under base
-            //       agrees with what was requested
-            String absPath = normalize(file.getAbsolutePath());
-            if ((absoluteBase.length() < absPath.length())
-                    && (canonicalBase.length() < canPath.length())) {
-                absPath = absPath.substring(absoluteBase.length() + 1);
-                if (absPath.equals("")) {
-                    absPath = "/";
-                }
-                canPath = canPath.substring(canonicalBase.length() + 1);
-                if (canPath.equals("")) {
-                    canPath = "/";
-                }
-                if (!canPath.equals(absPath)) {
-                    return null;
-                }
-            }
+        // If allow linking is enabled, files are not limited to being located
+        // under the fileBase so all further checks are disabled.
+        if (getRoot().getAllowLinking()) {
+            return file;
+        }
 
-        } else {
+        // Check that this file is located under the WebResourceSet's base
+        String canPath = null;
+        try {
+            canPath = file.getCanonicalPath();
+        } catch (IOException e) {
+            // Ignore
+        }
+        if (canPath == null || !canPath.startsWith(canonicalBase)) {
             return null;
         }
+
+        // Ensure that the file is not outside the fileBase. This should not be
+        // possible for standard requests (the request is normalized early in
+        // the request processing) but might be possible for some access via 
the
+        // Servlet API (RequestDispatcher, HTTP/2 push etc.) therefore these
+        // checks are retained as an additional safety measure
+        // absoluteBase has been normalized so absPath needs to normalized as
+        // well.
+        String absPath = normalize(file.getAbsolutePath());
+        if (absoluteBase.length() > absPath.length() ||
+                canonicalBase.length() > canPath.length()) {
+            return null;
+        }
+
+        // Remove the fileBase location from the start of the paths since that
+        // was not part of the requested path and the remaining check only
+        // applies to the request path
+        absPath = absPath.substring(absoluteBase.length());
+        canPath = canPath.substring(canonicalBase.length());
+
+        // Case sensitivity check
+        // The normalized requested path should be an exact match the 
equivalent
+        // canonical path. If it is not, possible reasons include:
+        // - case differences on case insensitive file systems
+        // - Windows removing a trailing ' ' or '.' from the file name
+        //
+        // In all cases, a mis-match here results in the resource not being
+        // found
+        if (!canPath.equals(absPath)) {
+            return null;
+        }
+
         return file;
     }
 



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to