fix archive builder inclusion of "/" as an entry, and improve api and javadoc
a "/" entry can cause the java FileTreeWalker to recurse infinitely and stack overflow Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/2009a041 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/2009a041 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/2009a041 Branch: refs/heads/master Commit: 2009a041de953f4467b1ebf9f9b8e1ddfce3d1ba Parents: a340dbc Author: Alex Heneveld <[email protected]> Authored: Thu Sep 10 14:00:15 2015 +0100 Committer: Alex Heneveld <[email protected]> Committed: Thu Sep 10 14:02:00 2015 +0100 ---------------------------------------------------------------------- .../brooklyn/util/core/file/ArchiveBuilder.java | 61 +++++++++++++------- .../util/core/file/ArchiveBuilderTest.java | 44 ++++++++------ .../java/org/apache/brooklyn/util/os/Os.java | 3 +- 3 files changed, 67 insertions(+), 41 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2009a041/core/src/main/java/org/apache/brooklyn/util/core/file/ArchiveBuilder.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/util/core/file/ArchiveBuilder.java b/core/src/main/java/org/apache/brooklyn/util/core/file/ArchiveBuilder.java index b880682..5213d8e 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/file/ArchiveBuilder.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/file/ArchiveBuilder.java @@ -36,6 +36,7 @@ import java.util.zip.ZipOutputStream; import org.apache.brooklyn.util.core.file.ArchiveUtils.ArchiveType; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.os.Os; +import org.apache.brooklyn.util.text.Strings; import com.google.common.annotations.Beta; import com.google.common.collect.Iterables; @@ -179,22 +180,24 @@ public class ArchiveBuilder { } /** - * Add the file located at the {@code fileSubPath}, relative to the {@code baseDir} on the local system, - * to the archive. + * Add a file located at the {@code fileSubPath}, relative to the {@code baseDir} on the local system, + * as {@code fileSubPath} in the archive. For most archives directories are supported. * <p> * Uses the {@code fileSubPath} as the name of the file in the archive. Note that the * file is found by concatenating the two path components using {@link Os#mergePaths(String...)}, * thus {@code fileSubPath} should not be absolute or point to a location above the current directory. * <p> - * Use {@link #entry(String, String)} directly or {@link #entries(Map)} for complete - * control over file locations and names in the archive. + * For a simpler addition mechanism, use {@link #addAt(File, String)}. + * <p> + * For complete control over file locations and names in the archive. + * use {@link #entry(String, String)} directly or {@link #entries(Map)} for complete * * @see #entry(String, String) */ public ArchiveBuilder addFromLocalBaseDir(File baseDir, String fileSubPath) { checkNotNull(baseDir, "baseDir"); checkNotNull(fileSubPath, "filePath"); - return entry(Os.mergePaths(".", fileSubPath), Os.mergePaths(baseDir.getPath(), fileSubPath)); + return entry(fileSubPath, Os.mergePaths(baseDir.getPath(), fileSubPath)); } /** @deprecated since 0.7.0 use {@link #addFromLocalBaseDir(File, String)}, or * one of the other add methods if adding relative to baseDir was not intended */ @Deprecated @@ -207,12 +210,22 @@ public class ArchiveBuilder { return addFromLocalBaseDir(baseDir, fileSubPath); } - /** adds the given file to the archive, preserving its name but putting under the given directory in the archive (may be <code>""</code> or <code>"./"</code>) */ + /** + * Adds the given file or directory to the archive, preserving its name but putting under the given directory in the archive (may be <code>""</code> or <code>"./"</code>). + * See also {@link #addDirContentsAt(File, String)} and {@link #addFromLocalBaseDir(File, String)}. */ public ArchiveBuilder addAt(File file, String archiveParentDir) { checkNotNull(archiveParentDir, "archiveParentDir"); checkNotNull(file, "file"); return entry(Os.mergePaths(archiveParentDir, file.getName()), file); } + + /** + * Adds the given file or directory to the root of the archive, preserving its name. + * To add the contents of a directory without the dir name, use {@link #addDirContentsAtRoot(File)}. + * See also {@link #addAt(File, String)}. */ + public ArchiveBuilder addAtRoot(File file) { + return addAt(file, ""); + } /** * Add the contents of the directory named {@code dirName} to the archive. @@ -226,9 +239,7 @@ public class ArchiveBuilder { /** * Add the contents of the directory {@code dir} to the archive. - * The directory's name is not included; use {@link #addAtRoot(File)} if you want that behaviour. - * <p> - * Uses {@literal .} as the parent directory name for the contents. + * The directory's name is not included; use {@link #addAt(File, String)} with <code>""</code> as the second argument if you want that behavior. * * @see #entry(String, File) */ @@ -237,6 +248,11 @@ public class ArchiveBuilder { if (!dir.isDirectory()) throw new IllegalArgumentException(dir+" is not a directory; cannot add contents to archive"); return entry(archiveParentDir, dir); } + /** See {@link #addDirContentsAt(File, String)} and {@link #addAtRoot(File)}. */ + public ArchiveBuilder addDirContentsAtRoot(File dir) { + return addDirContentsAt(dir, ""); + } + /** * As {@link #addDirContentsAt(File, String)}, * using {@literal .} as the parent directory name for the contents. @@ -389,17 +405,20 @@ public class ArchiveBuilder { String name = path.replace("\\", "/"); if (isDirectory) { - name += "/"; - JarEntry entry = new JarEntry(name); - - long lastModified=-1; - for (File source: sources) - if (source.lastModified()>lastModified) - lastModified = source.lastModified(); - - entry.setTime(lastModified); - target.putNextEntry(entry); - target.closeEntry(); + name = Strings.removeAllFromEnd(name, "/"); + if (name.length()>0) { + name += "/"; + JarEntry entry = new JarEntry(name); + + long lastModified=-1; + for (File source: sources) + if (source.lastModified()>lastModified) + lastModified = source.lastModified(); + + entry.setTime(lastModified); + target.putNextEntry(entry); + target.closeEntry(); + } for (File source: sources) { if (!source.isDirectory()) { @@ -407,7 +426,7 @@ public class ArchiveBuilder { } Iterable<File> children = Files.fileTreeTraverser().children(source); for (File child : children) { - addToArchive(Os.mergePaths(path, child.getName()), Collections.singleton(child), target); + addToArchive(Os.mergePaths(name, child.getName()), Collections.singleton(child), target); } } return; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2009a041/core/src/test/java/org/apache/brooklyn/util/core/file/ArchiveBuilderTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/util/core/file/ArchiveBuilderTest.java b/core/src/test/java/org/apache/brooklyn/util/core/file/ArchiveBuilderTest.java index d0ab8e2..26db9a0 100644 --- a/core/src/test/java/org/apache/brooklyn/util/core/file/ArchiveBuilderTest.java +++ b/core/src/test/java/org/apache/brooklyn/util/core/file/ArchiveBuilderTest.java @@ -24,6 +24,7 @@ import static org.testng.Assert.assertTrue; import java.io.File; import java.io.FileInputStream; +import java.io.FileNotFoundException; import java.io.IOException; import java.util.Arrays; import java.util.List; @@ -135,31 +136,32 @@ public class ArchiveBuilderTest { assertTrue(names.contains("./data04.txt")); input.close(); } + @Test - public void testCreateZipFromFiles() throws Exception { + public void testCreateZipFromFilesWithNoDir() throws Exception { ArchiveBuilder builder = ArchiveBuilder.zip(); for (String fileName : Arrays.asList("data01.txt", "data02.txt", "data03.txt")) { - builder.addAt(new File(tmpDir, fileName), "."); + builder.addAt(new File(tmpDir, fileName), ""); } - File archive = builder.create(); - archive.deleteOnExit(); + buildAndValidatePrefix(builder, "data"); + } - List<ZipEntry> entries = Lists.newArrayList(); - ZipInputStream input = new ZipInputStream(new FileInputStream(archive)); - ZipEntry entry = input.getNextEntry(); - while (entry != null) { - entries.add(entry); - entry = input.getNextEntry(); + @Test + public void testCreateZipFromFilesInSlash() throws Exception { + ArchiveBuilder builder = ArchiveBuilder.zip(); + for (String fileName : Arrays.asList("data01.txt", "data02.txt", "data03.txt")) { + builder.addAt(new File(tmpDir, fileName), "/"); } - assertEquals(entries.size(), 3); - Iterable<ZipEntry> directories = Iterables.filter(entries, isDirectory); - Iterable<ZipEntry> files = Iterables.filter(entries, Predicates.not(isDirectory)); - assertTrue(Iterables.isEmpty(directories)); - assertEquals(Iterables.size(files), 3); - for (ZipEntry file : files) { - assertTrue(file.getName().startsWith(Os.mergePathsUnix(".", "data"))); + buildAndValidatePrefix(builder, "/data"); + } + + @Test + public void testCreateZipFromFilesInDot() throws Exception { + ArchiveBuilder builder = ArchiveBuilder.zip(); + for (String fileName : Arrays.asList("data01.txt", "data02.txt", "data03.txt")) { + builder.addAt(new File(tmpDir, fileName), "."); } - input.close(); + buildAndValidatePrefix(builder, Os.mergePathsUnix(".", "data")); } @Test @@ -169,6 +171,10 @@ public class ArchiveBuilderTest { for (String fileName : Arrays.asList("data01.txt", "data02.txt", "data03.txt")) { builder.addFromLocalBaseDir(parentDir, Os.mergePaths(baseDir, fileName)); } + buildAndValidatePrefix(builder, Os.mergePaths(baseDir, "data")); + } + + private void buildAndValidatePrefix(ArchiveBuilder builder, String prefix) throws FileNotFoundException, IOException { File archive = builder.create(); archive.deleteOnExit(); @@ -185,7 +191,7 @@ public class ArchiveBuilderTest { assertTrue(Iterables.isEmpty(directories)); assertEquals(Iterables.size(files), 3); for (ZipEntry file : files) { - assertTrue(file.getName().startsWith(Os.mergePathsUnix(".", baseDir))); + assertTrue(file.getName().startsWith(prefix), "File is: "+file+"; missing "+prefix); } input.close(); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2009a041/utils/common/src/main/java/org/apache/brooklyn/util/os/Os.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/os/Os.java b/utils/common/src/main/java/org/apache/brooklyn/util/os/Os.java index 13ba63b..a15ae0b 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/os/Os.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/os/Os.java @@ -202,7 +202,8 @@ public class Os { } /** merges paths using forward slash as the "local OS file separator", because it is recognised on windows, - * making paths more consistent and avoiding problems with backslashes being escaped */ + * making paths more consistent and avoiding problems with backslashes being escaped. + * empty segments are omitted. */ public static String mergePaths(String ...items) { char separatorChar = '/'; StringBuilder result = new StringBuilder();
