From 6727a9ab968ad42faeb627496b201cd2c9ecce95 Mon Sep 17 00:00:00 2001 From: Ivo Smid Date: Sat, 18 Jan 2020 18:33:19 +0100 Subject: [PATCH] Do not transfer mountable file when tar name is "." or ".." (docker is unable to use this files) --- .../images/builder/traits/FilesTrait.java | 11 ++ .../testcontainers/utility/MountableFile.java | 3 +- .../builder/ImageFromDockerfileTest.java | 55 ++++++++ .../utility/DirectoryTarResourceTest.java | 128 ++++++++++++------ 4 files changed, 154 insertions(+), 43 deletions(-) create mode 100644 core/src/test/java/org/testcontainers/images/builder/ImageFromDockerfileTest.java diff --git a/core/src/main/java/org/testcontainers/images/builder/traits/FilesTrait.java b/core/src/main/java/org/testcontainers/images/builder/traits/FilesTrait.java index 61524b59f8a..3679a7f025d 100644 --- a/core/src/main/java/org/testcontainers/images/builder/traits/FilesTrait.java +++ b/core/src/main/java/org/testcontainers/images/builder/traits/FilesTrait.java @@ -3,6 +3,7 @@ import org.testcontainers.utility.MountableFile; import java.io.File; +import java.nio.file.Files; import java.nio.file.Path; /** @@ -50,6 +51,16 @@ default SELF withFileFromFile(String path, File file, Integer mode) { * @return self */ default SELF withFileFromPath(String path, Path filePath, Integer mode) { + final boolean fileStoredToDir = Files.isRegularFile(filePath) && + (path.endsWith("/.") + || path.endsWith("/..") + || path.endsWith("./") + || path.endsWith("../") + || ".".equals(path) + || "..".equals(path)); + if (fileStoredToDir) { + throw new IllegalArgumentException("Unable to store file '" + filePath + "' to docker path '" + path + "'"); + } final MountableFile mountableFile = MountableFile.forHostPath(filePath, mode); return ((SELF) this).withFileFromTransferable(path, mountableFile); } diff --git a/core/src/main/java/org/testcontainers/utility/MountableFile.java b/core/src/main/java/org/testcontainers/utility/MountableFile.java index 7b7367b9290..a8af13b981d 100644 --- a/core/src/main/java/org/testcontainers/utility/MountableFile.java +++ b/core/src/main/java/org/testcontainers/utility/MountableFile.java @@ -322,6 +322,7 @@ private void recursiveTar(String entryFilename, String rootPath, String itemPath tarEntry.setMode(getUnixFileMode(itemPath)); tarArchive.putArchiveEntry(tarEntry); + log.trace("Transferring {} '{}'", sourceFile.isFile() ? "file" : "directory", sourceFile); if (sourceFile.isFile()) { Files.copy(sourceFile.toPath(), tarArchive); } @@ -377,7 +378,7 @@ public static int getUnixFileMode(final Path path) { // Truncate mode bits for z/OS if ("OS/390".equals(SystemUtils.OS_NAME) || "z/OS".equals(SystemUtils.OS_NAME) || - "zOS".equals(SystemUtils.OS_NAME) ) { + "zOS".equals(SystemUtils.OS_NAME)) { unixMode &= TarConstants.MAXID; unixMode |= Files.isDirectory(path) ? 040000 : 0100000; } diff --git a/core/src/test/java/org/testcontainers/images/builder/ImageFromDockerfileTest.java b/core/src/test/java/org/testcontainers/images/builder/ImageFromDockerfileTest.java new file mode 100644 index 00000000000..b54cd2d88c8 --- /dev/null +++ b/core/src/test/java/org/testcontainers/images/builder/ImageFromDockerfileTest.java @@ -0,0 +1,55 @@ +package org.testcontainers.images.builder; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.runners.Enclosed; +import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import java.io.File; +import java.nio.file.Paths; + +@RunWith(Enclosed.class) +public class ImageFromDockerfileTest { + @RunWith(Parameterized.class) + public static class InvalidTarPathTests { + @Rule + public final ExpectedException expectedException = ExpectedException.none(); + + @Parameterized.Parameters(name = "{index} - {0}") + public static String[] getTestCases() { + return new String[]{ + "..", + ".", + "../", + "./", + "/..", + "/.", + "/../", + "/./", + ".", + "..", + "aa/.", + "aa/..", + "bb/./", + "bb/../" + }; + } + + @Parameterized.Parameter + public String tarPath; + + @Test + public void unableToTransferFileWithDotsToDockerDaemon() { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Unable to store file '" + + Paths.get("src", "test", "resources", "mappable-resource", "test-resource.txt") + + "' to docker path '" + tarPath + "'"); + + final ImageFromDockerfile imageFromDockerfile = new ImageFromDockerfile() + .withFileFromFile(tarPath, new File("src/test/resources/mappable-resource/test-resource.txt")); + imageFromDockerfile.resolve(); + } + } +} diff --git a/core/src/test/java/org/testcontainers/utility/DirectoryTarResourceTest.java b/core/src/test/java/org/testcontainers/utility/DirectoryTarResourceTest.java index 3789a654c20..92724357dfc 100644 --- a/core/src/test/java/org/testcontainers/utility/DirectoryTarResourceTest.java +++ b/core/src/test/java/org/testcontainers/utility/DirectoryTarResourceTest.java @@ -10,77 +10,121 @@ import java.io.File; import java.util.Arrays; +import java.util.List; import static org.hamcrest.CoreMatchers.allOf; import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.endsWith; import static org.rnorth.visibleassertions.VisibleAssertions.assertThat; import static org.rnorth.visibleassertions.VisibleAssertions.assertTrue; public class DirectoryTarResourceTest { - @Test public void simpleRecursiveFileTest() { // 'src' is expected to be the project base directory, so all source code/resources should be copied in File directory = new File("src"); - GenericContainer container = new GenericContainer( - new ImageFromDockerfile() - .withDockerfileFromBuilder(builder -> - builder.from("alpine:3.3") - .copy("/tmp/foo", "/foo") - .cmd("cat /foo/test/resources/test-recursive-file.txt") - .build() - ).withFileFromFile("/tmp/foo", directory)) - .withStartupCheckStrategy(new OneShotStartupCheckStrategy()); + try (GenericContainer container = new GenericContainer( + new ImageFromDockerfile() + .withDockerfileFromBuilder(builder -> + builder.from("alpine:3.3") + .copy("/tmp/foo", "/foo") + .cmd("cat /foo/test/resources/test-recursive-file.txt") + .build() + ).withFileFromFile("/tmp/foo", directory)) + .withStartupCheckStrategy(new OneShotStartupCheckStrategy())) { - container.start(); + container.start(); - final String results = container.getLogs(); + final String results = container.getLogs(); - assertTrue("The container has a file that was copied in via a recursive copy", results.contains("Used for DirectoryTarResourceTest")); + assertTrue("The container has a file that was copied in via a recursive copy", results.contains("Used for DirectoryTarResourceTest")); + } } @Test public void simpleRecursiveFileWithPermissionTest() { - GenericContainer container = new GenericContainer( - new ImageFromDockerfile() - .withDockerfileFromBuilder(builder -> - builder.from("alpine:3.3") - .copy("/tmp/foo", "/foo") - .cmd("ls", "-al", "/") - .build() - ).withFileFromFile("/tmp/foo", new File("/mappable-resource/test-resource.txt"), - 0754)) - .withStartupCheckStrategy(new OneShotStartupCheckStrategy()); - - container.start(); - String listing = container.getLogs(); - - assertThat("Listing shows that file is copied with mode requested.", + try (GenericContainer container = new GenericContainer( + new ImageFromDockerfile() + .withDockerfileFromBuilder(builder -> + builder.from("alpine:3.3") + .copy("/tmp/foo", "/foo") + .cmd("ls", "-al", "/") + .build() + ).withFileFromFile("/tmp/foo", new File("/mappable-resource/test-resource.txt"), + 0754)) + .withStartupCheckStrategy(new OneShotStartupCheckStrategy())) { + + container.start(); + String listing = container.getLogs(); + + assertThat("Listing shows that file is copied with mode requested.", Arrays.asList(listing.split("\\n")), - exactlyNItems(1, allOf(containsString("-rwxr-xr--"), containsString("foo")))); + exactlyOnce(allOf(containsString("-rwxr-xr--"), containsString("foo")))); + } + } + + @Test + public void transferFileDockerDaemon() { + final File theFile = new File("src/test/resources/mappable-resource/test-resource.txt"); + try (GenericContainer container = new GenericContainer( + new ImageFromDockerfile() + .withDockerfileFromBuilder(builder -> + builder.from("alpine:3.3") + .copy(".", "/foo/") + .cmd("ls", "-al", "/foo") + .build() + ).withFileFromFile("bar1", theFile) + .withFileFromFile("./bar2", theFile) + .withFileFromFile("../bar3", theFile) + .withFileFromFile(".bar4", theFile) + .withFileFromFile("..bar5", theFile) + .withFileFromFile("xxx/../bar6", theFile) + .withFileFromFile("x7/./bar7", theFile) + .withFileFromFile("x8/././bar8", theFile) + .withFileFromFile("x9/../../bar9", theFile)) + .withStartupCheckStrategy(new OneShotStartupCheckStrategy())) { + + container.start(); + + final List logLines = Arrays.asList(container.getLogs().split("\\n")); + assertThat("Listing shows that file is copied.", logLines, exactlyOnce(endsWith(" bar1"))); + assertThat("Listing shows that file is copied.", logLines, exactlyOnce(endsWith(" bar2"))); + assertThat("Listing shows that file is copied.", logLines, exactlyOnce(endsWith(" bar3"))); + assertThat("Listing shows that file is copied.", logLines, exactlyOnce(endsWith(" .bar4"))); + assertThat("Listing shows that file is copied.", logLines, exactlyOnce(endsWith(" ..bar5"))); + assertThat("Listing shows that file is copied.", logLines, exactlyOnce(endsWith(" bar6"))); + assertThat("Listing shows that file is copied.", logLines, exactlyOnce(endsWith(" x7"))); + assertThat("Listing shows that file is copied.", logLines, exactlyOnce(endsWith(" x8"))); + assertThat("Listing shows that file is copied.", logLines, exactlyOnce(endsWith(" bar9"))); + } } @Test public void simpleRecursiveClasspathResourceTest() { // This test combines the copying of classpath resources from JAR files with the recursive TAR approach, to allow JARed classpath resources to be copied in to an image - GenericContainer container = new GenericContainer( - new ImageFromDockerfile() - .withDockerfileFromBuilder(builder -> - builder.from("alpine:3.3") - .copy("/tmp/foo", "/foo") - .cmd("ls -lRt /foo") - .build() - ).withFileFromClasspath("/tmp/foo", "/recursive/dir")) // here we use /org/junit as a directory that really should exist on the classpath - .withStartupCheckStrategy(new OneShotStartupCheckStrategy()); + try (GenericContainer container = new GenericContainer( + new ImageFromDockerfile() + .withDockerfileFromBuilder(builder -> + builder.from("alpine:3.3") + .copy("/tmp/foo", "/foo") + .cmd("ls -lRt /foo") + .build() + ).withFileFromClasspath("/tmp/foo", "/recursive/dir")) // here we use /org/junit as a directory that really should exist on the classpath + .withStartupCheckStrategy(new OneShotStartupCheckStrategy())) { + + container.start(); - container.start(); + final String results = container.getLogs(); - final String results = container.getLogs(); + // ExternalResource.class is known to exist in a subdirectory of /org/junit so should be successfully copied in + assertTrue("The container has a file that was copied in via a recursive copy from a JAR resource", results.contains("content.txt")); + } + } - // ExternalResource.class is known to exist in a subdirectory of /org/junit so should be successfully copied in - assertTrue("The container has a file that was copied in via a recursive copy from a JAR resource", results.contains("content.txt")); + public static Matcher> exactlyOnce(Matcher elementMatcher) { + return exactlyNItems(1, elementMatcher); } public static Matcher> exactlyNItems(final int n, Matcher elementMatcher) {