Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix #680: Avoid trailing slashes in tar entry filenames #808

Merged
merged 4 commits into from
Jul 30, 2018
Merged

Fix #680: Avoid trailing slashes in tar entry filenames #808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5530fad
Fix #680: Ensure that tar entries for Dockerfile builder do not have …
rnorth Jul 29, 2018
5a68cb1
Add a regression test
rnorth Jul 29, 2018
eb4b990
Add cleanup for streams in test
rnorth Jul 29, 2018
5b5a1d5
Remove leading slash just in case
rnorth Jul 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion core/src/main/java/org/testcontainers/utility/MountableFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,14 @@ private void recursiveTar(String entryFilename, String rootPath, String itemPath
final File sourceRootFile = new File(rootPath).getCanonicalFile(); // e.g. /foo
final String relativePathToSourceFile = sourceRootFile.toPath().relativize(sourceFile.toPath()).toFile().toString(); // e.g. /bar/baz

final TarArchiveEntry tarEntry = new TarArchiveEntry(sourceFile, entryFilename + "/" + relativePathToSourceFile); // entry filename e.g. /xyz/bar/baz
final String tarEntryFilename;
if (relativePathToSourceFile.isEmpty()) {
tarEntryFilename = entryFilename; // entry filename e.g. xyz => xyz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder what will happen if the user will call with it /some/entry?
Maybe we should strip / prefix from tarEntryFilename before passing to TarArchiveEntry's constructor, for the additional safety

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I think it's allowed, isn't it? There's no source 'working directory' in this context, so 'abc' == '/abc'..?

I'll give it a try though.

} else {
tarEntryFilename = entryFilename + "/" + relativePathToSourceFile; // entry filename e.g. /xyz/bar/baz => /foo/bar/baz
}

final TarArchiveEntry tarEntry = new TarArchiveEntry(sourceFile, tarEntryFilename.replaceAll("^/", ""));

// TarArchiveEntry automatically sets the mode for file/directory, but we can update to ensure that the mode is set exactly (inc executable bits)
tarEntry.setMode(getUnixFileMode(itemPath));
Expand Down
38 changes: 34 additions & 4 deletions core/src/test/java/org/testcontainers/utility/MountableFileTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
package org.testcontainers.utility;

import lombok.Cleanup;
import org.apache.commons.compress.archivers.ArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
import org.jetbrains.annotations.NotNull;
import org.junit.Test;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.function.Consumer;

import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals;
import static org.rnorth.visibleassertions.VisibleAssertions.assertFalse;
import static org.rnorth.visibleassertions.VisibleAssertions.assertTrue;
import static org.rnorth.visibleassertions.VisibleAssertions.*;

public class MountableFileTest {

Expand Down Expand Up @@ -79,7 +84,7 @@ public void forHostPathWithPlus() throws Exception {
@Test
public void forClasspathResourceWithPermission() throws Exception {
final MountableFile mountableFile = MountableFile.forClasspathResource("mappable-resource/test-resource.txt",
TEST_FILE_MODE);
TEST_FILE_MODE);

performChecks(mountableFile);
assertEquals("Valid file mode.", BASE_FILE_MODE | TEST_FILE_MODE, mountableFile.getFileMode());
Expand All @@ -101,6 +106,31 @@ public void forHostDirPathWithPermission() throws Exception {
assertEquals("Valid dir mode.", BASE_DIR_MODE | TEST_FILE_MODE, mountableFile.getFileMode());
}

@Test
public void noTrailingSlashesInTarEntryNames() throws Exception {
final MountableFile mountableFile = MountableFile.forClasspathResource("mappable-resource/test-resource.txt");

@Cleanup final TarArchiveInputStream tais = intoTarArchive((taos) -> {
mountableFile.transferTo(taos, "/some/path.txt");
mountableFile.transferTo(taos, "/path.txt");
mountableFile.transferTo(taos, "path.txt");
});

ArchiveEntry entry;
while ((entry = tais.getNextEntry()) != null) {
assertFalse("no entries should have a trailing slash", entry.getName().endsWith("/"));
}
}

private TarArchiveInputStream intoTarArchive(Consumer<TarArchiveOutputStream> consumer) throws IOException {
@Cleanup final ByteArrayOutputStream baos = new ByteArrayOutputStream();
@Cleanup final TarArchiveOutputStream taos = new TarArchiveOutputStream(baos);
consumer.accept(taos);
taos.close();

return new TarArchiveInputStream(new ByteArrayInputStream(baos.toByteArray()));
}

@SuppressWarnings("ResultOfMethodCallIgnored")
@NotNull
private Path createTempFile(final String name) throws IOException {
Expand Down