Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51704

Make calls to File.mkdirs() more robust and handle errors in a few places where they were ignored and should not have been.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1162169 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/java/org/apache/catalina/core/StandardContext.java b/java/org/apache/catalina/core/StandardContext.java
index 5d98b9b..e115380 100644 (file)
--- a/java/org/apache/catalina/core/StandardContext.java
+++ b/java/org/apache/catalina/core/StandardContext.java
@@ -6057,7 +6057,7 @@ public class StandardContext extends ContainerBase
                         workDir, catalinaHomePath, getName()), e);
             }
         }
-        if (!dir.exists() && !dir.mkdirs()) {
+        if (!dir.mkdirs() && !dir.isDirectory()) {
             log.warn(sm.getString("standardContext.workCreateFail", dir,
                     getName()));
         }

diff --git a/java/org/apache/catalina/loader/WebappClassLoader.java b/java/org/apache/catalina/loader/WebappClassLoader.java
index 72b2a48..dbb15a2 100644 (file)
--- a/java/org/apache/catalina/loader/WebappClassLoader.java
+++ b/java/org/apache/catalina/loader/WebappClassLoader.java
@@ -3011,8 +3011,11 @@ public class WebappClassLoader
                                             throw new IllegalArgumentException(
                                                     sm.getString("webappClassLoader.validationErrorJarPath",
                                                             jarEntry2.getName()), ioe);
-                                        }                                 
-                                        resourceFile.getParentFile().mkdirs();
+                                        }
+                                        File parentFile = resourceFile.getParentFile();
+                                        if (!parentFile.mkdirs() && !parentFile.exists()) {
+                                            // Ignore the error (like the IOExceptions below)
+                                        }
                                         FileOutputStream os = null;
                                         InputStream is = null;
                                         try {

diff --git a/java/org/apache/catalina/loader/WebappLoader.java b/java/org/apache/catalina/loader/WebappLoader.java
index 9bd5a7f..245dd43 100644 (file)
--- a/java/org/apache/catalina/loader/WebappLoader.java
+++ b/java/org/apache/catalina/loader/WebappLoader.java
@@ -883,10 +883,10 @@ public class WebappLoader extends LifecycleMBeanBase
             } else {
 
                 classRepository = new File(workDir, classesPath);
-                if (!classRepository.isDirectory()) {
-                    if (!classRepository.mkdirs())
-                        throw new IOException(
-                                sm.getString("webappLoader.mkdirFailure"));
+                if (!classRepository.mkdirs() &&
+                        !classRepository.isDirectory()) {
+                    throw new IOException(
+                            sm.getString("webappLoader.mkdirFailure"));
                 }
                 if (!copyDir(classes, classRepository)) {
                     throw new IOException(
@@ -935,10 +935,9 @@ public class WebappLoader extends LifecycleMBeanBase
             } else {
                 copyJars = true;
                 destDir = new File(workDir, libPath);
-                if (!destDir.isDirectory()) {
-                    if (!destDir.mkdirs())
-                        throw new IOException(
-                                sm.getString("webappLoader.mkdirFailure"));
+                if (!destDir.mkdirs() && !destDir.isDirectory()) {
+                    throw new IOException(
+                            sm.getString("webappLoader.mkdirFailure"));
                 }
             }
 

diff --git a/java/org/apache/catalina/manager/ManagerServlet.java b/java/org/apache/catalina/manager/ManagerServlet.java
index c8e82e1..04ae352 100644 (file)
--- a/java/org/apache/catalina/manager/ManagerServlet.java
+++ b/java/org/apache/catalina/manager/ManagerServlet.java
@@ -636,7 +636,7 @@ public class ManagerServlet extends HttpServlet implements ContainerServlet {
         File deployedPath = deployed;
         if (tag != null) {
             deployedPath = new File(versioned, tag);
-            if (!deployedPath.isDirectory() && !deployedPath.mkdirs()) {
+            if (!deployedPath.mkdirs() && !deployedPath.isDirectory()) {
                 writer.println(smClient.getString("managerServlet.mkdirFail",
                         deployedPath));
                 return;
@@ -830,7 +830,7 @@ public class ManagerServlet extends HttpServlet implements ContainerServlet {
                 addServiced(name);
                 try {
                     if (config != null) {
-                        if (!configBase.isDirectory() && !configBase.mkdirs()) {
+                        if (!configBase.mkdirs() && !configBase.isDirectory()) {
                             writer.println(smClient.getString(
                                     "managerServlet.mkdirFail",configBase));
                             return;

diff --git a/java/org/apache/catalina/manager/host/HostManagerServlet.java b/java/org/apache/catalina/manager/host/HostManagerServlet.java
index dc8c7dd..c666709 100644 (file)
--- a/java/org/apache/catalina/manager/host/HostManagerServlet.java
+++ b/java/org/apache/catalina/manager/host/HostManagerServlet.java
@@ -381,13 +381,11 @@ public class HostManagerServlet
         } catch (IOException e) {
             appBaseFile = file;
         }
-        if (!appBaseFile.exists()) {
-            if (!appBaseFile.mkdirs()) {
-                writer.println(smClient.getString(
-                        "hostManagerServlet.appBaseCreateFail",
-                        appBaseFile.toString(), name));
-                return;
-            }
+        if (!appBaseFile.mkdirs() && !appBaseFile.isDirectory()) {
+            writer.println(smClient.getString(
+                    "hostManagerServlet.appBaseCreateFail",
+                    appBaseFile.toString(), name));
+            return;
         }
         
         // Create base for config files
@@ -704,10 +702,8 @@ public class HostManagerServlet
         if (installedHost != null) {
             configBase = new File(configBase, hostName);
         }
-        if (!configBase.exists()) {
-            if (!configBase.mkdirs()) {
-                return null;
-            }
+        if (!configBase.mkdirs() && !configBase.isDirectory()) {
+            return null;
         }
         return configBase;
     }

diff --git a/java/org/apache/catalina/servlets/CGIServlet.java b/java/org/apache/catalina/servlets/CGIServlet.java
index df8f20e..f9a025b 100644 (file)
--- a/java/org/apache/catalina/servlets/CGIServlet.java
+++ b/java/org/apache/catalina/servlets/CGIServlet.java
@@ -1149,9 +1149,11 @@ public final class CGIServlet extends HttpServlet {
             String dirPath = destPath.toString().substring(
                     0,destPath.toString().lastIndexOf("/"));
             File dir = new File(dirPath);
-            if (!dir.mkdirs() && debug >= 2) {
-                log("expandCGIScript: failed to create directories for '" +
-                        dir.getAbsolutePath() + "'");
+            if (!dir.mkdirs() && !dir.isDirectory()) {
+                if (debug >= 2) {
+                    log("expandCGIScript: failed to create directories for '" +
+                            dir.getAbsolutePath() + "'");
+                }
                 return;
             }
 

diff --git a/java/org/apache/catalina/session/FileStore.java b/java/org/apache/catalina/session/FileStore.java
index 8afadd5..60c3fea 100644 (file)
--- a/java/org/apache/catalina/session/FileStore.java
+++ b/java/org/apache/catalina/session/FileStore.java
@@ -395,7 +395,7 @@ public final class FileStore extends StoreBase {
      * session persistence directory, if any.  The directory will be
      * created if it does not already exist.
      */
-    private File directory() {
+    private File directory() throws IOException {
 
         if (this.directory == null) {
             return (null);
@@ -419,8 +419,14 @@ public final class FileStore extends StoreBase {
             }
         }
         if (!file.exists() || !file.isDirectory()) {
-            file.delete();
-            file.mkdirs();
+            if (!file.delete() && file.exists()) {
+                throw new IOException(
+                        sm.getString("fileStore.deleteFailed", file));
+            }
+            if (!file.mkdirs() && !file.isDirectory()) {
+                throw new IOException(
+                        sm.getString("fileStore.createFailed", file));
+            }
         }
         this.directoryFile = file;
         return (file);
@@ -435,7 +441,7 @@ public final class FileStore extends StoreBase {
      * @param id The ID of the Session to be retrieved. This is
      *    used in the file naming.
      */
-    private File file(String id) {
+    private File file(String id) throws IOException {
 
         if (this.directory == null) {
             return (null);

diff --git a/java/org/apache/catalina/session/LocalStrings.properties b/java/org/apache/catalina/session/LocalStrings.properties
index c3d0c74..6037ed0 100644 (file)
--- a/java/org/apache/catalina/session/LocalStrings.properties
+++ b/java/org/apache/catalina/session/LocalStrings.properties
@@ -18,6 +18,8 @@ applicationSession.value.iae=null value
 fileStore.saving=Saving Session {0} to file {1}
 fileStore.loading=Loading Session {0} from file {1}
 fileStore.removing=Removing Session {0} at file {1}
+fileStore.deleteFailed=Unable to delete file [{0}] which is preventing the creation of the session storage location
+fileStore.createFailed=Unable to create directory [{0}] for the storage of session data 
 JDBCStore.close=Exception closing database connection {0}
 JDBCStore.saving=Saving Session {0} to database {1}
 JDBCStore.loading=Loading Session {0} from database {1}

diff --git a/java/org/apache/catalina/startup/ExpandWar.java b/java/org/apache/catalina/startup/ExpandWar.java
index 4bdd6f6..f9f1b4f 100644 (file)
--- a/java/org/apache/catalina/startup/ExpandWar.java
+++ b/java/org/apache/catalina/startup/ExpandWar.java
@@ -115,7 +115,10 @@ public class ExpandWar {
                 if (last >= 0) {
                     File parent = new File(docBase,
                                            name.substring(0, last));
-                    parent.mkdirs();
+                    if (!parent.mkdirs() && !parent.isDirectory()) {
+                        throw new IOException(
+                                sm.getString("expandWar.createFailed", parent));
+                    }
                 }
                 if (name.endsWith("/")) {
                     continue;

diff --git a/java/org/apache/catalina/startup/HostConfig.java b/java/org/apache/catalina/startup/HostConfig.java
index 2cefbb9..2223259 100644 (file)
--- a/java/org/apache/catalina/startup/HostConfig.java
+++ b/java/org/apache/catalina/startup/HostConfig.java
@@ -1290,7 +1290,7 @@ public class HostConfig
         if (host.getCreateDirs()) {
             File[] dirs = new File[] {host.getAppBaseFile(),configBase()};
             for (int i=0; i<dirs.length; i++) {
-                if ( (!dirs[i].isDirectory()) && (!dirs[i].mkdirs())) {
+                if (!dirs[i].mkdirs() && !dirs[i].isDirectory()) {
                     log.error(sm.getString("hostConfig.createDirs",dirs[i]));
                 }
             }

diff --git a/java/org/apache/catalina/startup/LocalStrings.properties b/java/org/apache/catalina/startup/LocalStrings.properties
index f7c44e8..b1f0e8a 100644 (file)
--- a/java/org/apache/catalina/startup/LocalStrings.properties
+++ b/java/org/apache/catalina/startup/LocalStrings.properties
@@ -70,6 +70,7 @@ engineConfig.cce=Lifecycle event data object {0} is not an Engine
 engineConfig.start=EngineConfig: Processing START
 engineConfig.stop=EngineConfig: Processing STOP
 expandWar.copy=Error copying {0} to {1}
+expandWar.createFailed=Unable to create the directory [{0}]
 expandWar.deleteFailed=[{0}] could not be completely deleted. The presence of the remaining files may cause problems
 expandWar.illegalPath=The archive [{0}] is malformed and will be ignored: an entry contains an illegal path [{1}] which was not expanded to [{2}] since that is outside of the defined docBase [{3}]
 hostConfig.appBase=Application base [{1}] for host [{0}] does not exist or is not a directory. deployOnStartUp and autoDeploy have been set to false to prevent deployment errors. Other errors may still occur.

diff --git a/java/org/apache/catalina/valves/AccessLogValve.java b/java/org/apache/catalina/valves/AccessLogValve.java
index 1490e34..e7764b4 100644 (file)
--- a/java/org/apache/catalina/valves/AccessLogValve.java
+++ b/java/org/apache/catalina/valves/AccessLogValve.java
@@ -1081,10 +1081,8 @@ public class AccessLogValve extends ValveBase implements AccessLog {
         File dir = new File(directory);
         if (!dir.isAbsolute())
             dir = new File(System.getProperty(Globals.CATALINA_BASE_PROP), directory);
-        if (!dir.exists()) {
-            if (!dir.mkdirs()) {
-                log.error(sm.getString("accessLogValve.openDirFail", dir));
-            }
+        if (!dir.mkdirs() && !dir.isDirectory()) {
+            log.error(sm.getString("accessLogValve.openDirFail", dir));
         }
 
         // Open the current log file
@@ -1097,10 +1095,8 @@ public class AccessLogValve extends ValveBase implements AccessLog {
             pathname = new File(dir.getAbsoluteFile(), prefix + suffix);
         }
         File parent = pathname.getParentFile();
-        if (!parent.exists()) {
-            if (!parent.mkdirs()) {
-                log.error(sm.getString("accessLogValve.openDirFail", parent));
-            }
+        if (!parent.mkdirs() && !parent.isDirectory()) {
+            log.error(sm.getString("accessLogValve.openDirFail", parent));
         }
 
         Charset charset = null;

diff --git a/java/org/apache/jasper/JspCompilationContext.java b/java/org/apache/jasper/JspCompilationContext.java
index 1775974..d9a685d 100644 (file)
--- a/java/org/apache/jasper/JspCompilationContext.java
+++ b/java/org/apache/jasper/JspCompilationContext.java
@@ -709,7 +709,7 @@ public class JspCompilationContext {
     protected boolean makeOutputDir() {
         synchronized(outputDirLock) {
             File outDirFile = new File(outputDir);
-            return (outDirFile.exists() || outDirFile.mkdirs());
+            return (outDirFile.mkdirs() || outDirFile.isDirectory());
         }
     }
 

diff --git a/java/org/apache/juli/FileHandler.java b/java/org/apache/juli/FileHandler.java
index 6569fb0..60742d5 100644 (file)
--- a/java/org/apache/juli/FileHandler.java
+++ b/java/org/apache/juli/FileHandler.java
@@ -363,7 +363,7 @@ public class FileHandler
 
         // Create the directory if necessary
         File dir = new File(directory);
-        if (!dir.exists() && !dir.mkdirs()) {
+        if (!dir.mkdirs() && !dir.isDirectory()) {
             reportError("Unable to create [" + dir + "]", null,
                     ErrorManager.OPEN_FAILURE);
             writer = null;
@@ -376,13 +376,11 @@ public class FileHandler
             File pathname = new File(dir.getAbsoluteFile(), prefix
                     + (rotatable ? date : "") + suffix);
             File parent = pathname.getParentFile();
-            if (!parent.exists()) {
-                if (!parent.mkdirs()) {
-                    reportError("Unable to create [" + parent + "]", null,
-                            ErrorManager.OPEN_FAILURE);
-                    writer = null;
-                    return;
-                }
+            if (!parent.mkdirs() && !parent.isDirectory()) {
+                reportError("Unable to create [" + parent + "]", null,
+                        ErrorManager.OPEN_FAILURE);
+                writer = null;
+                return;
             }
             String encoding = getEncoding();
             FileOutputStream fos = new FileOutputStream(pathname, true);

diff --git a/test/org/apache/catalina/connector/TestCoyoteAdapter.java b/test/org/apache/catalina/connector/TestCoyoteAdapter.java
index aaeed89..53d6c8e 100644 (file)
--- a/test/org/apache/catalina/connector/TestCoyoteAdapter.java
+++ b/test/org/apache/catalina/connector/TestCoyoteAdapter.java
@@ -87,7 +87,7 @@ public class TestCoyoteAdapter extends TomcatBaseTest {
         
         // Create the folder that will trigger the redirect
         File foo = new File(docBase, "foo");
-        if (!foo.exists() && !foo.mkdirs()) {
+        if (!foo.mkdirs() && !foo.isDirectory()) {
             fail("Unable to create foo directory in docBase");
         }
         

diff --git a/test/org/apache/catalina/core/TestAsyncContextImpl.java b/test/org/apache/catalina/core/TestAsyncContextImpl.java
index 32a0d93..66d129a 100644 (file)
--- a/test/org/apache/catalina/core/TestAsyncContextImpl.java
+++ b/test/org/apache/catalina/core/TestAsyncContextImpl.java
@@ -411,7 +411,7 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
         
         // Create the folder that will trigger the redirect
         File foo = new File(docBase, "async");
-        if (!foo.exists() && !foo.mkdirs()) {
+        if (!foo.mkdirs() && !foo.isDirectory()) {
             fail("Unable to create async directory in docBase");
         }
         

diff --git a/test/org/apache/catalina/core/TestStandardContext.java b/test/org/apache/catalina/core/TestStandardContext.java
index ac9505b..2f9cc89 100644 (file)
--- a/test/org/apache/catalina/core/TestStandardContext.java
+++ b/test/org/apache/catalina/core/TestStandardContext.java
@@ -76,7 +76,7 @@ public class TestStandardContext extends TomcatBaseTest {
         Tomcat tomcat = getTomcatInstance();
         
         File docBase = new File(tomcat.getHost().getAppBaseFile(), "ROOT");
-        if (!docBase.exists() && !docBase.mkdirs()) {
+        if (!docBase.mkdirs() && !docBase.isDirectory()) {
             fail("Unable to create docBase");
         }
         

diff --git a/test/org/apache/catalina/servlets/TestDefaultServlet.java b/test/org/apache/catalina/servlets/TestDefaultServlet.java
index 18abf56..5ac2f11 100644 (file)
--- a/test/org/apache/catalina/servlets/TestDefaultServlet.java
+++ b/test/org/apache/catalina/servlets/TestDefaultServlet.java
@@ -164,7 +164,7 @@ public class TestDefaultServlet extends TomcatBaseTest {
     public void testCustomErrorPage() throws Exception {
         File appDir = new File(getTemporaryDirectory(), "MyApp");
         File webInf = new File(appDir, "WEB-INF");
-        if (!webInf.mkdirs()) {
+        if (!webInf.mkdirs() && !webInf.isDirectory()) {
             fail("Unable to create directory [" + webInf + "]");
         }
         Writer w = new OutputStreamWriter(new FileOutputStream(new File(appDir,
@@ -249,7 +249,7 @@ public class TestDefaultServlet extends TomcatBaseTest {
     public void testCustomErrorPageMissing() throws Exception {
         File appDir = new File(getTemporaryDirectory(), "MyApp");
         File webInf = new File(appDir, "WEB-INF");
-        if (!webInf.mkdirs()) {
+        if (!webInf.mkdirs() && !webInf.isDirectory()) {
             fail("Unable to create directory [" + webInf + "]");
         }
         Writer w = new OutputStreamWriter(new FileOutputStream(new File(appDir,

diff --git a/test/org/apache/catalina/startup/TomcatBaseTest.java b/test/org/apache/catalina/startup/TomcatBaseTest.java
index 7696bec..32d4d48 100644 (file)
--- a/test/org/apache/catalina/startup/TomcatBaseTest.java
+++ b/test/org/apache/catalina/startup/TomcatBaseTest.java
@@ -113,7 +113,7 @@ public abstract class TomcatBaseTest {
                 getBuildDirectory(), "conf/logging.properties").toString());
 
         tempDir = new File(System.getProperty("tomcat.test.temp", "output/tmp"));
-        if (!tempDir.exists() && !tempDir.mkdirs()) {
+        if (!tempDir.mkdirs() && !tempDir.isDirectory()) {
             fail("Unable to create temporary directory for test");
         }