From 7be8ac337480775618fb3627aa535e691deec309 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) or transferring into directory --- .../images/builder/traits/FilesTrait.java | 15 ++ .../testcontainers/utility/MountableFile.java | 1 + .../builder/ImageFromDockerfileTest.java | 46 +++++++ .../utility/DirectoryTarResourceTest.java | 130 +++++++++++++++--- 4 files changed, 172 insertions(+), 20 deletions(-) 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 4c1204f752a..f3064ad9b91 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; /** @@ -49,6 +50,20 @@ 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("./") || + 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 a8a9f465a19..dc58e400df7 100644 --- a/core/src/main/java/org/testcontainers/utility/MountableFile.java +++ b/core/src/main/java/org/testcontainers/utility/MountableFile.java @@ -358,6 +358,7 @@ private void recursiveTar( tarEntry.setMode(getUnixFileMode(itemPath)); tarArchive.putArchiveEntry(tarEntry); + log.trace("Transferring {} '{}'", sourceFile.isFile() ? "file" : "directory", sourceFile); if (sourceFile.isFile()) { Files.copy(sourceFile.toPath(), tarArchive); } diff --git a/core/src/test/java/org/testcontainers/images/builder/ImageFromDockerfileTest.java b/core/src/test/java/org/testcontainers/images/builder/ImageFromDockerfileTest.java index 0ce8ec041bf..5821fa29a2a 100644 --- a/core/src/test/java/org/testcontainers/images/builder/ImageFromDockerfileTest.java +++ b/core/src/test/java/org/testcontainers/images/builder/ImageFromDockerfileTest.java @@ -3,10 +3,16 @@ import com.github.dockerjava.api.DockerClient; import com.github.dockerjava.api.command.InspectImageResponse; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.testcontainers.DockerClientFactory; import org.testcontainers.utility.Base58; +import java.io.File; +import java.nio.file.Paths; + import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; class ImageFromDockerfileTest { @@ -44,4 +50,44 @@ void shouldNotAddSessionLabelIfDeleteOnExitIsFalse() { dockerClient.removeImageCmd(imageId).exec(); } } + + @ParameterizedTest + @ValueSource( + strings = { + "..", + ".", + "../", + "./", + "xxx/", + "yyy/xxx/", + "/xxx/", + "/yyy/xxx/", + "/..", + "/.", + "/../", + "/./", + ".", + "..", + "aa/.", + "aa/..", + "bb/./", + "bb/../", + "cc./", + "cc../", + } + ) + void unableToTransferFileWithDotsToDockerDaemon(String tarPath) { + assertThatThrownBy(() -> { + new ImageFromDockerfile() + .withFileFromFile(tarPath, new File("src/test/resources/mappable-resource/test-resource.txt")); + }) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Unable to store file '" + + Paths.get("src", "test", "resources", "mappable-resource", "test-resource.txt") + + "' to docker path '" + + tarPath + + "'" + ); + } } diff --git a/core/src/test/java/org/testcontainers/utility/DirectoryTarResourceTest.java b/core/src/test/java/org/testcontainers/utility/DirectoryTarResourceTest.java index bfd3641adeb..d40341ac39c 100644 --- a/core/src/test/java/org/testcontainers/utility/DirectoryTarResourceTest.java +++ b/core/src/test/java/org/testcontainers/utility/DirectoryTarResourceTest.java @@ -1,5 +1,6 @@ package org.testcontainers.utility; +import org.apache.commons.lang3.StringUtils; import org.assertj.core.api.Condition; import org.junit.jupiter.api.Test; import org.testcontainers.containers.GenericContainer; @@ -7,6 +8,10 @@ import org.testcontainers.images.builder.ImageFromDockerfile; import java.io.File; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.function.Consumer; import java.util.function.Predicate; import static org.assertj.core.api.Assertions.assertThat; @@ -18,26 +23,28 @@ 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.17") - .copy("/tmp/foo", "/foo") - .cmd("cat /foo/test/resources/test-recursive-file.txt") - .build(); - }) - .withFileFromFile("/tmp/foo", directory) - ) - .withStartupCheckStrategy(new OneShotStartupCheckStrategy()); - - container.start(); - - final String results = container.getLogs(); - - assertThat(results) - .as("The container has a file that was copied in via a recursive copy") - .contains("Used for DirectoryTarResourceTest"); + try ( + GenericContainer container = new GenericContainer( + new ImageFromDockerfile() + .withDockerfileFromBuilder(builder -> { + builder + .from("alpine:3.17") + .copy("/tmp/foo", "/foo") + .cmd("cat /foo/test/resources/test-recursive-file.txt") + .build(); + }) + .withFileFromFile("/tmp/foo", directory) + ) + .withStartupCheckStrategy(new OneShotStartupCheckStrategy()) + ) { + container.start(); + + final String results = container.getLogs(); + + assertThat(results) + .as("The container has a file that was copied in via a recursive copy") + .contains("Used for DirectoryTarResourceTest"); + } } @Test @@ -94,4 +101,87 @@ void simpleRecursiveClasspathResourceTest() { .contains("content.txt"); } } + + @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", "-lapR", "/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(logLines.stream().filter(StringUtils::isEmpty).count()) + .describedAs("Three groups of dirs") + .isEqualTo(2); + + final LsOutput lsOutput = LsOutput.parse(logLines); + + assertThat(lsOutput.parentDir).satisfiesOnlyOnce(endsWith(" bar1")); + assertThat(lsOutput.parentDir).satisfiesOnlyOnce(endsWith(" bar2")); + assertThat(lsOutput.parentDir).satisfiesOnlyOnce(endsWith(" bar3")); + assertThat(lsOutput.parentDir).satisfiesOnlyOnce(endsWith(" .bar4")); + assertThat(lsOutput.parentDir).satisfiesOnlyOnce(endsWith(" ..bar5")); + assertThat(lsOutput.parentDir).satisfiesOnlyOnce(endsWith(" bar6")); + assertThat(lsOutput.parentDir).satisfiesOnlyOnce(endsWith(" x7/")); + assertThat(lsOutput.parentDir).satisfiesOnlyOnce(endsWith(" x8/")); + assertThat(lsOutput.parentDir).satisfiesOnlyOnce(endsWith(" bar9")); + assertThat(lsOutput.subDir1).satisfiesOnlyOnce(endsWith(" bar7")); + assertThat(lsOutput.subDir2).satisfiesOnlyOnce(endsWith(" bar8")); + } + } + + private static class LsOutput { + + private final List parentDir; + + private final List subDir1; + + private final List subDir2; + + private static LsOutput parse(List logLines) { + List parentDir = null; + List subDir1 = null; + int start = 0; + for (int i = 0; i < logLines.size(); i++) { + if (logLines.get(i).isEmpty()) { + if (parentDir == null) { + parentDir = new ArrayList<>(logLines.subList(start, i)); + start = i; + } else if (subDir1 == null) { + subDir1 = new ArrayList<>(logLines.subList(start, i)); + start = i; + } + } + } + List subDir2 = new ArrayList<>(logLines.subList(start, logLines.size())); + + return new LsOutput(parentDir, subDir1, subDir2); + } + + private LsOutput(List parentDir, List subDir1, List subDir2) { + this.parentDir = parentDir; + this.subDir1 = subDir1; + this.subDir2 = subDir2; + } + } + + public static Consumer endsWith(String suffix) { + return value -> assertThat(value).endsWith(suffix); + } }