# HG changeset patch # User lancea # Date 1572373338 14400 # Node ID e492513d36307ab5f068b89beffff344f0a635c1 # Parent 5a0e0d0b3a278f048e8a98583858bc95fca4ba04 8231766: Files.copy and Files.move do not honor requested compression method when copying or moving within the same zip file Reviewed-by: clanger, bpb, alanb diff -r 5a0e0d0b3a27 -r e492513d3630 src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java --- a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java Tue Oct 29 13:51:14 2019 -0400 +++ b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java Tue Oct 29 14:22:18 2019 -0400 @@ -739,7 +739,20 @@ Files.copy(eSrc.file, u.file, REPLACE_EXISTING); } } + } else if (eSrc.type == Entry.CEN && eSrc.method != defaultCompressionMethod) { + + /** + * We are copying a file within the same Zip file using a + * different compression method. + */ + try (InputStream in = newInputStream(src); + OutputStream out = newOutputStream(dst, + CREATE, TRUNCATE_EXISTING, WRITE)) { + in.transferTo(out); + } + u = getEntry(dst); } + if (!hasCopyAttrs) u.mtime = u.atime= u.ctime = System.currentTimeMillis(); update(u); @@ -789,7 +802,8 @@ return os; } return getOutputStream(supportPosix ? - new PosixEntry((PosixEntry)e, Entry.NEW) : new Entry(e, Entry.NEW)); + new PosixEntry((PosixEntry)e, Entry.NEW, defaultCompressionMethod) + : new Entry(e, Entry.NEW, defaultCompressionMethod)); } else { if (!hasCreate && !hasCreateNew) throw new NoSuchFileException(getString(path)); @@ -2338,6 +2352,11 @@ this.file = file; } + Entry(Entry e, int type, int compressionMethod) { + this(e, type); + this.method = compressionMethod; + } + Entry(Entry e, int type) { name(e.name); this.isdir = e.isdir; @@ -2905,6 +2924,11 @@ super(name, file, type, attrs); } + PosixEntry(PosixEntry e, int type, int compressionMethod) { + super(e, type); + this.method = compressionMethod; + } + PosixEntry(PosixEntry e, int type) { super(e, type); this.owner = e.owner; diff -r 5a0e0d0b3a27 -r e492513d3630 test/jdk/jdk/nio/zipfs/CopyMoveTests.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/jdk/jdk/nio/zipfs/CopyMoveTests.java Tue Oct 29 14:22:18 2019 -0400 @@ -0,0 +1,478 @@ +/* + * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + * + */ + +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.*; +import java.security.SecureRandom; +import java.util.Arrays; +import java.util.Map; +import java.util.zip.ZipEntry; +import java.util.zip.ZipFile; + +import static java.lang.String.format; +import static java.util.stream.Collectors.joining; +import static org.testng.Assert.*; + +/** + * @test + * @bug 8223771 + * @summary Test Files::copy and Files::move with Zip FS + * @modules jdk.zipfs + * @run testng/othervm CopyMoveTests + */ +public class CopyMoveTests { + // Enable debugging output + private static final boolean DEBUG = false; + // Path to current directory + private static final Path HERE = Path.of("."); + // Value to use when creating Zip Entries + private static final String ZIP_FILE_VALUE = "US Open 2019"; + // Value used to create the OS file to be copied into/from a Zip File + private static final String OS_FILE_VALUE = "Hello World!"; + private static final SecureRandom random = new SecureRandom(); + + /* + * DataProvider used to verify that a FileAlreadyExistsException is + * thrown with copying a file without the REPLACE_EXISTING option + */ + @DataProvider(name = "zipfsMap") + private Object[][] zipfsMap() { + return new Object[][]{ + {Map.of("create", "true"), ZipEntry.DEFLATED}, + {Map.of("create", "true", "noCompression", "true"), + ZipEntry.STORED}, + {Map.of("create", "true", "noCompression", "false"), + ZipEntry.DEFLATED} + }; + } + + /* + * DataProvider used to verify that an entry may be copied or moved within + * a Zip file system with the correct compression method + */ + @DataProvider(name = "copyMoveMap") + private Object[][] copyMoveMap() { + return new Object[][]{ + {Map.of("create", "true"), ZipEntry.DEFLATED, ZipEntry.STORED}, + {Map.of("create", "true", "noCompression", "true"), + ZipEntry.STORED, ZipEntry.DEFLATED}, + {Map.of("create", "true", "noCompression", "false"), + ZipEntry.DEFLATED, ZipEntry.STORED} + }; + } + + /** + * Validate that an entry that is copied within a Zip file is copied with + * the correct compression + * + * @param createMap Zip FS properties to use when creating the Zip File + * @param compression The compression used when writing the initial entries + * @param expectedCompression The compression to be used when copying the entry + * @throws Exception If an error occurs + */ + @Test(dataProvider = "copyMoveMap", enabled = true) + public void copyTest(Map createMap, int compression, + int expectedCompression) throws Exception { + Entry e0 = Entry.of("Entry-0", compression, ZIP_FILE_VALUE); + Entry e1 = Entry.of("Entry-1", compression, ZIP_FILE_VALUE); + Entry e00 = Entry.of("Entry-00", expectedCompression, ZIP_FILE_VALUE); + + Path zipFile = generatePath(HERE, "test", ".zip"); + Files.deleteIfExists(zipFile); + + // Create the Zip File with the initial entries + createZipFile(zipFile, createMap, e0, e1); + try (FileSystem zipfs = FileSystems.newFileSystem(zipFile, + Map.of("noCompression", expectedCompression == ZipEntry.STORED))) { + Files.copy(zipfs.getPath(e0.name), zipfs.getPath(e00.name)); + } + // Verify entries e0, e1 and e00 exist + verify(zipFile, e0, e1, e00); + Files.deleteIfExists(zipFile); + + } + + /** + * Validate that an entry that is copied from one Zip file to another, + * is copied with the correct compression + * + * @param createMap Zip FS properties to use when creating the Zip File + * @param compression The compression used when writing the initial entries + * @param expectedCompression The compression to be used when copying the entry + * @throws Exception If an error occurs + */ + @Test(dataProvider = "copyMoveMap", enabled = true) + public void copyZipToZipTest(Map createMap, int compression, + int expectedCompression) throws Exception { + Entry e0 = Entry.of("Entry-0", compression, ZIP_FILE_VALUE); + Entry e1 = Entry.of("Entry-1", compression, ZIP_FILE_VALUE); + Entry e00 = Entry.of("Entry-00", expectedCompression, ZIP_FILE_VALUE); + + Path zipFile = generatePath(HERE, "test", ".zip"); + Path zipFile2 = generatePath(HERE, "test", ".zip"); + Files.deleteIfExists(zipFile); + Files.deleteIfExists(zipFile2); + + createZipFile(zipFile, createMap, e0, e1); + try (FileSystem zipfs = FileSystems.newFileSystem(zipFile, createMap); + FileSystem zipfsTarget = FileSystems.newFileSystem(zipFile2, + Map.of("create", "true", "noCompression", + expectedCompression == ZipEntry.STORED))) { + Files.copy(zipfs.getPath(e0.name), zipfsTarget.getPath(e00.name)); + } + // Only 1 entry copied to the secondary Zip file + verify(zipFile2, e00); + // Verify entries e0 and e1 remain in the original Zip file + verify(zipFile, e0, e1); + Files.deleteIfExists(zipFile); + Files.deleteIfExists(zipFile2); + } + + /** + * Validate that an external file copied to a Zip file is copied with + * the correct compression + * + * @param createMap Zip FS properties to use when creating the Zip File + * @param compression The compression used when writing the initial entries + * @param expectedCompression The compression to be used when copying the entry + * @throws Exception If an error occurs + */ + @Test(dataProvider = "copyMoveMap", enabled = true) + public void copyFromOsTest(Map createMap, int compression, + int expectedCompression) throws Exception { + + Path osFile = generatePath(HERE, "test", ".txt"); + Files.deleteIfExists(osFile); + Files.writeString(osFile, OS_FILE_VALUE); + Entry e0 = Entry.of("Entry-0", compression, ZIP_FILE_VALUE); + Entry e1 = Entry.of("Entry-1", compression, ZIP_FILE_VALUE); + Entry e00 = Entry.of("Entry-00", expectedCompression, OS_FILE_VALUE); + + Path zipFile = generatePath(HERE, "test", ".zip"); + Files.deleteIfExists(zipFile); + + createZipFile(zipFile, createMap, e0, e1); + try (FileSystem zipfs = FileSystems.newFileSystem(zipFile, + Map.of("noCompression", expectedCompression == ZipEntry.STORED))) { + Files.copy(osFile, zipfs.getPath(e00.name)); + } + verify(zipFile, e0, e1, e00); + Files.deleteIfExists(osFile); + Files.deleteIfExists(zipFile); + } + + /** + * Validate that an entry that is copied from a Zip file to an OS file contains + * the correct bytes and the file remains in the Zip file + * + * @param createMap Zip FS properties to use when creating the Zip File + * @param compression The compression used when writing the initial entries + * @param expectedCompression The compression to be used when moving the entry + * @throws Exception If an error occurs + */ + @Test(dataProvider = "copyMoveMap", enabled = true) + public void CopyFromZipTest(Map createMap, int compression, + int expectedCompression) throws Exception { + + Entry e0 = Entry.of("Entry-0", compression, ZIP_FILE_VALUE); + Entry e1 = Entry.of("Entry-1", compression, ZIP_FILE_VALUE); + + Path zipFile = generatePath(HERE, "test", ".zip"); + Path osFile = generatePath(HERE, "test", ".txt"); + Files.deleteIfExists(zipFile); + Files.deleteIfExists(osFile); + + createZipFile(zipFile, createMap, e0, e1); + try (FileSystem zipfs = FileSystems.newFileSystem(zipFile, Map.of())) { + Files.copy(zipfs.getPath(e0.name), osFile); + } + + // Entries e0 and e1 should exist + verify(zipFile, e0, e1); + // Check to see if the file exists and the bytes match + assertTrue(Files.isRegularFile(osFile)); + assertEquals(Files.readAllBytes(osFile), e0.bytes); + Files.deleteIfExists(zipFile); + Files.deleteIfExists(osFile); + } + + /** + * Validate that an entry that is moved within a Zip file is moved with + * the correct compression + * + * @param createMap Zip FS properties to use when creating the Zip File + * @param compression The compression used when writing the initial entries + * @param expectedCompression The compression to be used when moving the entry + * @throws Exception If an error occurs + */ + @Test(dataProvider = "copyMoveMap", enabled = true) + public void moveTest(Map createMap, int compression, + int expectedCompression) throws Exception { + + Entry e0 = Entry.of("Entry-0", compression, ZIP_FILE_VALUE); + Entry e1 = Entry.of("Entry-1", compression, ZIP_FILE_VALUE); + Entry e00 = Entry.of("Entry-00", expectedCompression, ZIP_FILE_VALUE); + + Path zipFile = generatePath(HERE, "test", ".zip"); + Files.deleteIfExists(zipFile); + + createZipFile(zipFile, createMap, e0, e1); + try (FileSystem zipfs = FileSystems.newFileSystem(zipFile, + Map.of("noCompression", expectedCompression == ZipEntry.STORED))) { + Files.move(zipfs.getPath(e0.name), zipfs.getPath(e00.name)); + } + // Entry e0 should not exist but Entry e00 should + verify(zipFile, e1, e00); + Files.deleteIfExists(zipFile); + } + + /** + * Validate that an entry that is moved one Zip file to another is moved with + * the correct compression + * + * @param createMap Zip FS properties to use when creating the Zip File + * @param compression The compression used when writing the initial entries + * @param expectedCompression The compression to be used when moving the entry + * @throws Exception If an error occurs + */ + @Test(dataProvider = "copyMoveMap", enabled = true) + public void moveZipToZipTest(Map createMap, int compression, + int expectedCompression) throws Exception { + + Entry e0 = Entry.of("Entry-0", compression, ZIP_FILE_VALUE); + Entry e1 = Entry.of("Entry-1", compression, ZIP_FILE_VALUE); + Entry e00 = Entry.of("Entry-00", expectedCompression, ZIP_FILE_VALUE); + + Path zipFile = generatePath(HERE, "test", ".zip"); + Path zipFile2 = generatePath(HERE, "test", ".zip"); + Files.deleteIfExists(zipFile); + Files.deleteIfExists(zipFile2); + + createZipFile(zipFile, createMap, e0, e1); + try (FileSystem zipfs = FileSystems.newFileSystem(zipFile, + Map.of("noCompression", expectedCompression == ZipEntry.STORED)); + FileSystem zipfsTarget = FileSystems.newFileSystem(zipFile2, + Map.of("create", "true", "noCompression", + expectedCompression == ZipEntry.STORED))) { + Files.move(zipfs.getPath(e0.name), zipfsTarget.getPath(e00.name)); + } + // Only Entry e00 should exist + verify(zipFile2, e00); + // Only Entry e1 should exist + verify(zipFile, e1); + Files.deleteIfExists(zipFile); + Files.deleteIfExists(zipFile2); + } + + /** + * Validate that an entry that is moved from a Zip file to an OS file contains + * the correct bytes and is removed from the Zip file + * + * @param createMap Zip FS properties to use when creating the Zip File + * @param compression The compression used when writing the initial entries + * @param expectedCompression The compression to be used when moving the entry + * @throws Exception If an error occurs + */ + @Test(dataProvider = "copyMoveMap", enabled = true) + public void moveFromZipTest(Map createMap, int compression, + int expectedCompression) throws Exception { + + Entry e0 = Entry.of("Entry-0", compression, ZIP_FILE_VALUE); + Entry e1 = Entry.of("Entry-1", compression, ZIP_FILE_VALUE); + + Path zipFile = generatePath(HERE, "test", ".zip"); + Path osFile = generatePath(HERE, "test", ".txt"); + Files.deleteIfExists(zipFile); + Files.deleteIfExists(osFile); + + createZipFile(zipFile, createMap, e0, e1); + try (FileSystem zipfs = FileSystems.newFileSystem(zipFile, Map.of())) { + Files.move(zipfs.getPath(e0.name), osFile); + } + + // Only Entry e1 should exist + verify(zipFile, e1); + // Check to see if the file exists and the bytes match + assertTrue(Files.isRegularFile(osFile)); + assertEquals(Files.readAllBytes(osFile), e0.bytes); + Files.deleteIfExists(zipFile); + Files.deleteIfExists(osFile); + } + + /** + * Validate that a FileAlreadyExistsException is thrown when copying a + * file and not specifying the REPLACE_EXISTING option. + * + * @param createMap Properties used for creating the ZIP Filesystem + * @throws Exception if an error occurs + */ + @Test(dataProvider = "zipfsMap", enabled = true) + public void testFAEWithCopy(Map createMap, + int compression) throws Exception { + if (DEBUG) { + System.out.printf("ZIP FS Map = %s%n ", formatMap(createMap)); + } + Entry e0 = Entry.of("Entry-0", compression, ZIP_FILE_VALUE); + Entry e1 = Entry.of("Entry-1", compression, ZIP_FILE_VALUE); + + Path zipFile = generatePath(HERE, "test", ".zip"); + Files.deleteIfExists(zipFile); + + createZipFile(zipFile, createMap, e0, e1); + try (FileSystem zipfs = + FileSystems.newFileSystem(zipFile, createMap)) { + assertThrows(FileAlreadyExistsException.class, () -> + Files.copy(zipfs.getPath(e0.name), + zipfs.getPath(e1.name))); + } + Files.deleteIfExists(zipFile); + } + + /** + * Generate a temporary file Path + * + * @param dir Directory used to create the path + * @param prefix The prefix string used to create the path + * @param suffix The suffix string used to create the path + * @return Path that was generated + */ + private static Path generatePath(Path dir, String prefix, String suffix) { + long n = random.nextLong(); + String s = prefix + Long.toUnsignedString(n) + suffix; + Path name = dir.getFileSystem().getPath(s); + // the generated name should be a simple file name + if (name.getParent() != null) + throw new IllegalArgumentException("Invalid prefix or suffix"); + return dir.resolve(name); + } + + /** + * Utility method to return a formatted String of the key:value entries for + * a Map + * + * @param env Map to format + * @return Formatted string of the Map entries + */ + private static String formatMap(Map env) { + return env.entrySet().stream() + .map(e -> format("(%s:%s)", e.getKey(), e.getValue())) + .collect(joining(", ")); + } + + /** + * Create a Zip file using the Zip File System with the specified + * Zip File System properties + * + * @param zipFile Path to the Zip File to create + * @param env Properties used for creating the Zip Filesystem + * @param entries The entries to add to the Zip File + * @throws IOException If an error occurs while creating the Zip file + */ + private void createZipFile(Path zipFile, Map env, + Entry... entries) throws IOException { + if (DEBUG) { + System.out.printf("Creating Zip file: %s with the Properties: %s%n", + zipFile, formatMap(env)); + } + try (FileSystem zipfs = + FileSystems.newFileSystem(zipFile, env)) { + for (Entry e : entries) { + Files.writeString(zipfs.getPath(e.name), new String(e.bytes)); + } + } + } + + /** + * Represents an entry in a Zip file. An entry encapsulates a name, a + * compression method, and its contents/data. + */ + static class Entry { + private final String name; + private final int method; + private final byte[] bytes; + + Entry(String name, int method, String contents) { + this.name = name; + this.method = method; + this.bytes = contents.getBytes(StandardCharsets.UTF_8); + } + + static Entry of(String name, int method, String contents) { + return new Entry(name, method, contents); + } + + /** + * Returns a new Entry with the same name and compression method as this + * Entry but with the given content. + */ + Entry content(String contents) { + return new Entry(name, method, contents); + } + } + + /** + * Verify that the given path is a Zip file containing exactly the + * given entries. + */ + private static void verify(Path zipfile, Entry... entries) throws IOException { + // check entries with zip API + try (ZipFile zf = new ZipFile(zipfile.toFile())) { + // check entry count + assertEquals(entries.length, zf.size()); + + // check compression method and content of each entry + for (Entry e : entries) { + ZipEntry ze = zf.getEntry(e.name); + //System.out.printf("Name: %s, method: %s, Expected Method: %s%n", e.name, ze.getMethod(), e.method); + assertNotNull(ze); + assertEquals(e.method, ze.getMethod()); + try (InputStream in = zf.getInputStream(ze)) { + byte[] bytes = in.readAllBytes(); + assertTrue(Arrays.equals(bytes, e.bytes)); + } + } + } + + // check entries with FileSystem API + try (FileSystem fs = FileSystems.newFileSystem(zipfile)) { + // check entry count + Path top = fs.getPath("/"); + long count = Files.find(top, Integer.MAX_VALUE, + (path, attrs) -> attrs.isRegularFile()).count(); + assertEquals(entries.length, count); + + // check content of each entry + for (Entry e : entries) { + Path file = fs.getPath(e.name); + byte[] bytes = Files.readAllBytes(file); + assertTrue(Arrays.equals(bytes, e.bytes)); + } + } + } +} diff -r 5a0e0d0b3a27 -r e492513d3630 test/jdk/jdk/nio/zipfs/UpdateEntryTest.java --- a/test/jdk/jdk/nio/zipfs/UpdateEntryTest.java Tue Oct 29 13:51:14 2019 -0400 +++ b/test/jdk/jdk/nio/zipfs/UpdateEntryTest.java Tue Oct 29 14:22:18 2019 -0400 @@ -33,6 +33,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; +import java.util.Map; import java.util.spi.ToolProvider; import java.util.zip.CRC32; import java.util.zip.ZipEntry; @@ -116,14 +117,15 @@ // Create JAR file with a STORED(non-compressed) entry Files.writeString(Path.of(storedFileName), "foobar"); - int rc = JAR_TOOL.run(System.out, System.err, + JAR_TOOL.run(System.out, System.err, "cM0vf", jarFileName, storedFileName); - // Replace the STORED entry + // Replace the STORED entry using the default(DEFLATED) compression + // method. try (FileSystem fs = FileSystems.newFileSystem(zipFile)) { Files.writeString(fs.getPath(storedFileName), replacedValue); } - Entry e1 = Entry.of(storedFileName, ZipEntry.STORED, replacedValue); + Entry e1 = Entry.of(storedFileName, ZipEntry.DEFLATED, replacedValue); verify(zipFile, e1); } @@ -159,8 +161,12 @@ String newContents = "hi"; + // Set the required compression method + Map map = Map.of("noCompression", + e1.method != ZipEntry.DEFLATED); + // replace contents of e1 - try (FileSystem fs = FileSystems.newFileSystem(zipfile)) { + try (FileSystem fs = FileSystems.newFileSystem(zipfile, map)) { Path foo = fs.getPath(e1.name); Files.writeString(foo, newContents); }