Issue #5480 - Only delete non-persist temp dir if not-empty

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
jetty · Oct 20, 2020 · fa6c8f7 · fa6c8f7
1 parent c7ae5b5
commit fa6c8f7
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 2 deletions.
diff --git a/jetty-io/src/test/java/org/eclipse/jetty/io/IOTest.java b/jetty-io/src/test/java/org/eclipse/jetty/io/IOTest.java
@@ -20,6 +20,7 @@
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.File;
 import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.net.ServerSocket;
@@ -41,12 +42,15 @@
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 
+import org.eclipse.jetty.toolchain.test.FS;
+import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
 import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.IO;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
 import org.junit.jupiter.api.condition.OS;
 import org.junit.jupiter.api.extension.ExtendWith;
 
@@ -449,6 +453,64 @@ public void testGatherWrite() throws Exception
         }
     }
 
+    @Test
+    public void testDeleteNull()
+    {
+        assertFalse(IO.delete(null));
+    }
+
+    @Test
+    public void testDeleteNonExistentFile(TestInfo testInfo)
+    {
+        File dir = MavenTestingUtils.getTargetTestingDir(testInfo.getDisplayName());
+        FS.ensureEmpty(dir);
+        File noFile = new File(dir, "nada");
+        assertFalse(IO.delete(noFile));
+    }
+
+    @Test
+    public void testIsEmptyNull()
+    {
+        assertTrue(IO.isEmptyDir(null));
+    }
+
+    @Test
+    public void testIsEmptyDoesNotExist(TestInfo testInfo)
+    {
+        File dir = MavenTestingUtils.getTargetTestingDir(testInfo.getDisplayName());
+        FS.ensureEmpty(dir);
+        File noFile = new File(dir, "nada");
+        assertTrue(IO.isEmptyDir(noFile));
+    }
+
+    @Test
+    public void testIsEmptyExistButAsFile(TestInfo testInfo) throws IOException
+    {
+        File dir = MavenTestingUtils.getTargetTestingDir(testInfo.getDisplayName());
+        FS.ensureEmpty(dir);
+        File file = new File(dir, "nada");
+        FS.touch(file);
+        assertFalse(IO.isEmptyDir(file));
+    }
+
+    @Test
+    public void testIsEmptyExistAndIsEmpty(TestInfo testInfo)
+    {
+        File dir = MavenTestingUtils.getTargetTestingDir(testInfo.getDisplayName());
+        FS.ensureEmpty(dir);
+        assertTrue(IO.isEmptyDir(dir));
+    }
+
+    @Test
+    public void testIsEmptyExistAndHasContent(TestInfo testInfo) throws IOException
+    {
+        File dir = MavenTestingUtils.getTargetTestingDir(testInfo.getDisplayName());
+        FS.ensureEmpty(dir);
+        File file = new File(dir, "nada");
+        FS.touch(file);
+        assertFalse(IO.isEmptyDir(dir));
+    }
+
     @Test
     public void testSelectorWakeup() throws Exception
     {

diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/IO.java b/jetty-util/src/main/java/org/eclipse/jetty/util/IO.java
@@ -381,6 +381,27 @@ public static boolean delete(File file)
         return file.delete();
     }
 
+    /**
+     * Test if directory is empty.
+     *
+     * @param dir the directory
+     * @return true if directory is null, doesn't exist, or has no content.
+     * false if not a directory, or has contents
+     */
+    public static boolean isEmptyDir(File dir)
+    {
+        if (dir == null)
+            return true;
+        if (!dir.exists())
+            return true;
+        if (!dir.isDirectory())
+            return false;
+        String[] list = dir.list();
+        if (list == null)
+            return true;
+        return list.length <= 0;
+    }
+
     /**
      * Closes an arbitrary closable, and logs exceptions at ignore level
      *

diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java
@@ -357,9 +357,9 @@ public void deconfigure(WebAppContext context) throws Exception
         File tempDirectory = context.getTempDirectory();
 
         // if we're not persisting the temp dir contents delete it
-        if (!context.isPersistTempDirectory() && tempDirectory != null && tempDirectory.exists())
+        if (!context.isPersistTempDirectory() && !IO.isEmptyDir(tempDirectory))
         {
-            IO.delete(context.getTempDirectory());
+            IO.delete(tempDirectory);
         }
 
         //if it wasn't explicitly configured by the user, then unset it