svn commit: r1809298 - /tomcat/tc7.0.x/trunk/java/org/apache/naming/resources/FileDirContext.java

[ma...@apache.org]

Author: markt
Date: Fri Sep 22 10:39:51 2017
New Revision: 1809298

URL: http://svn.apache.org/viewvc?rev=1809298&view=rev
Log:
Code clean-up as a result of code reviews
- Minor performance optimisation
- Simplify code
- Additional commentary

Modified:
    tomcat/tc7.0.x/trunk/java/org/apache/naming/resources/FileDirContext.java

Modified: tomcat/tc7.0.x/trunk/java/org/apache/naming/resources/FileDirContext.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/naming/resources/FileDirContext.java?rev=1809298&r1=1809297&r2=1809298&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/naming/resources/FileDirContext.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/naming/resources/FileDirContext.java Fri Sep 22 10:39:51 2017
@@ -815,53 +815,69 @@ public class FileDirContext extends Base
         // If the requested names ends in '/', the Java File API will return a
         // matching file if one exists. This isn't what we want as it is not
         // consistent with the Servlet spec rules for request mapping.
-        if (file.isFile() && name.endsWith("/")) {
+        if (name.endsWith("/") && file.isFile()) {
             return null;
         }
 
-        if (!mustExist || file.exists() && 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 (allowLinking)
-                return file;
+        // If allow linking is enabled, files are not limited to being located
+        // under the fileBase so all further checks are disabled.
+        if (allowLinking)
+            return file;
+
+        // Check that this file is located under the web application root
+        String canPath = null;
+        try {
+            canPath = file.getCanonicalPath();
+        } catch (IOException e) {
+            // Ignore
+        }
+        if (canPath == null || !canPath.startsWith(absoluteBase)) {
+            return null;
+        }
 
-            // Check that this file belongs to our root path
-            String canPath = null;
-            try {
-                canPath = file.getCanonicalPath();
-            } catch (IOException e) {
-                // Ignore
-            }
-            if (canPath == null)
-                return null;
-
-            // Check to see if going outside of the web application root
-            if (!canPath.startsWith(absoluteBase)) {
-                return null;
-            }
-
-            // Case sensitivity check - this is now always done
-            String fileAbsPath = file.getAbsolutePath();
-            if (fileAbsPath.endsWith("."))
-                fileAbsPath = fileAbsPath + "/";
-            String absPath = normalize(fileAbsPath);
-            canPath = normalize(canPath);
-            if ((absoluteBase.length() < absPath.length())
-                && (absoluteBase.length() < canPath.length())) {
-                absPath = absPath.substring(absoluteBase.length() + 1);
-                if (absPath.equals(""))
-                    absPath = "/";
-                canPath = canPath.substring(absoluteBase.length() + 1);
-                if (canPath.equals(""))
-                    canPath = "/";
-                if (!canPath.equals(absPath))
-                    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 be normalized as
+        // well.
+        String absPath = normalize(file.getAbsolutePath());
+        if ((absoluteBase.length() > absPath.length())) {
+            return null;
+        }
 
-        } else {
+        // 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(absoluteBase.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
+        //
+        // absPath is normalized so canPath needs to be normalized as well
+        // Can't normalize canPath earlier as canonicalBase is not normalized
+        if (canPath.length() > 0) {
+            canPath = normalize(canPath);
+        }
+        if (!canPath.equals(absPath)) {
             return null;
         }
-        return file;
 
+        return file;
     }
 
 



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