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 index fe953ee8d9d..f466fc1967e 100644 --- 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 @@ package org.eclipse.jetty.io; 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.CountDownLatch; 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 class IOTest } } + @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-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/AbstractWebAppMojo.java b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/AbstractWebAppMojo.java index f0a803d5d9d..6d67c55e9cd 100644 --- a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/AbstractWebAppMojo.java +++ b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/AbstractWebAppMojo.java @@ -792,7 +792,13 @@ public abstract class AbstractWebAppMojo extends AbstractMojo File target = new File(project.getBuild().getDirectory()); File tmp = new File(target,"tmp"); if (!tmp.exists()) - tmp.mkdirs(); + { + if (!tmp.mkdirs()) + { + throw new MojoFailureException("Unable to create temp directory: " + tmp); + } + } + webApp.setTempDirectory(tmp); } 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 index 66fd876fe06..a014b634f19 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/IO.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/IO.java @@ -358,10 +358,13 @@ public class IO * This delete will recursively delete directories - BE CAREFUL * * @param file The file (or directory) to be deleted. - * @return true if anything was deleted. (note: this does not mean that all content in a directory was deleted) + * @return true if file was deleted, or directory referenced was deleted. + * false if file doesn't exist, or was null. */ public static boolean delete(File file) { + if (file == null) + return false; if (!file.exists()) return false; if (file.isDirectory()) @@ -375,6 +378,27 @@ public class IO 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 index 4559e4d03e3..09999ab56d9 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java @@ -84,10 +84,12 @@ public class WebInfConfiguration extends AbstractConfiguration @Override public void deconfigure(WebAppContext context) throws Exception { - //if we're not persisting the temp dir contents delete it - if (!context.isPersistTempDirectory()) + File tempDirectory = context.getTempDirectory(); + + // if we're not persisting the temp dir contents delete it + 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 @@ -231,13 +233,15 @@ public class WebInfConfiguration extends AbstractConfiguration //if it is to be persisted, make sure it will be the same name //by not using File.createTempFile, which appends random digits tmpDir = new File(parent, temp); + configureTempDirectory(tmpDir, context); } else { - //ensure file will always be unique by appending random digits + // ensure dir will always be unique by having classlib generate random path name tmpDir = Files.createTempDirectory(parent.toPath(), temp).toFile(); + tmpDir.deleteOnExit(); + ensureTempDirUsable(tmpDir); } - configureTempDirectory(tmpDir, context); if (LOG.isDebugEnabled()) LOG.debug("Set temp dir {}", tmpDir); @@ -249,21 +253,30 @@ public class WebInfConfiguration extends AbstractConfiguration if (dir == null) throw new IllegalArgumentException("Null temp dir"); - //if dir exists and we don't want it persisted, delete it - if (dir.exists() && !context.isPersistTempDirectory()) + // if dir exists and we don't want it persisted, delete it + if (!context.isPersistTempDirectory() && dir.exists() && !IO.delete(dir)) { - if (!IO.delete(dir)) - throw new IllegalStateException("Failed to delete temp dir " + dir); + throw new IllegalStateException("Failed to delete temp dir " + dir); } - //if it doesn't exist make it + // if it doesn't exist make it if (!dir.exists()) - dir.mkdirs(); + { + if (!dir.mkdirs()) + { + throw new IllegalStateException("Unable to create temp dir " + dir); + } + } if (!context.isPersistTempDirectory()) dir.deleteOnExit(); - //is it useable + ensureTempDirUsable(dir); + } + + private void ensureTempDirUsable(File dir) + { + // is it useable if (!dir.canWrite() || !dir.isDirectory()) throw new IllegalStateException("Temp dir " + dir + " not useable: writeable=" + dir.canWrite() + ", dir=" + dir.isDirectory()); }