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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
