From 4f2e261505557fffbf9982def7f7cf2aa941ef3d Mon Sep 17 00:00:00 2001 From: Baptiste Mathus Date: Tue, 28 Nov 2017 18:08:32 +0100 Subject: [PATCH] Be more defensive if provided destPath is wrong The path could already exist, *but* not be a directory. Nice catch James. Associated tests. --- .../java/hudson/util/AtomicFileWriter.java | 15 +++++-- .../hudson/util/AtomicFileWriterTest.java | 45 +++++++++++++++++-- 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/hudson/util/AtomicFileWriter.java b/core/src/main/java/hudson/util/AtomicFileWriter.java index 2f2f8e70e7..d63be2648c 100644 --- a/core/src/main/java/hudson/util/AtomicFileWriter.java +++ b/core/src/main/java/hudson/util/AtomicFileWriter.java @@ -98,10 +98,19 @@ public class AtomicFileWriter extends Writer { } this.destPath = destinationPath; Path dir = this.destPath.getParent(); - try { - if (Files.notExists(dir)) { // This is required for dir symlink handling, see JDK-8130464 - Files.createDirectories(dir); + + if (Files.exists(dir) && !Files.isDirectory(dir)) { + throw new IOException(dir + " exists and is neither a directory nor a symlink to a directory"); + } + else { + if (Files.isSymbolicLink(dir)) { + LOGGER.log(Level.CONFIG, "{0} is a symlink to a directory", dir); + } else { + Files.createDirectories(dir); // Cannot be called on symlink, so we are pretty defensive... } + } + + try { tmpPath = Files.createTempFile(dir, "atomic", "tmp"); } catch (IOException e) { throw new IOException("Failed to create a temporary file in "+ dir,e); diff --git a/core/src/test/java/hudson/util/AtomicFileWriterTest.java b/core/src/test/java/hudson/util/AtomicFileWriterTest.java index 1a6a6d97cf..716805bd31 100644 --- a/core/src/test/java/hudson/util/AtomicFileWriterTest.java +++ b/core/src/test/java/hudson/util/AtomicFileWriterTest.java @@ -1,6 +1,6 @@ package hudson.util; -import org.junit.After; +import org.hamcrest.core.StringContains; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -10,15 +10,22 @@ import java.io.File; import java.io.IOException; import java.nio.charset.Charset; import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; -import static org.junit.Assert.*; +import static org.hamcrest.core.StringContains.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class AtomicFileWriterTest { + @Rule + public TemporaryFolder tmp = new TemporaryFolder(); File af; AtomicFileWriter afw; String expectedContent = "hello world"; - @Rule public TemporaryFolder tmp = new TemporaryFolder(); - @Before public void setUp() throws IOException { @@ -26,6 +33,19 @@ public class AtomicFileWriterTest { afw = new AtomicFileWriter(af.toPath(), Charset.defaultCharset()); } + @Test + public void symlinkToDirectory() throws Exception { + final File folder = tmp.newFolder(); + final File containingSymlink = tmp.newFolder(); + final Path zeSymlink = Files.createSymbolicLink(Paths.get(containingSymlink.getAbsolutePath(), "ze_symlink"), + folder.toPath()); + + + final Path childFileInSymlinkToDir = Paths.get(zeSymlink.toString(), "childFileInSymlinkToDir"); + + new AtomicFileWriter(childFileInSymlinkToDir, Charset.forName("UTF-8")); + } + @Test public void createFile() throws Exception { // Verify the file we created exists @@ -68,4 +88,21 @@ public class AtomicFileWriterTest { // Then assertTrue(Files.notExists(afw.getTemporaryPath())); } + + @Test + public void badPath() throws Exception { + final File newFile = tmp.newFile(); + File parentExistsAndIsAFile = new File(newFile, "badChild"); + + assertTrue(newFile.exists()); + assertFalse(parentExistsAndIsAFile.exists()); + + try { + new AtomicFileWriter(parentExistsAndIsAFile.toPath(), Charset.forName("UTF-8")); + fail("Expected a failure"); + } catch (IOException e) { + assertThat(e.getMessage(), + containsString("exists and is neither a directory nor a symlink to a directory")); + } + } }