8231451: ZipFileInputStream::skip handling of negative values with STORED entries
authorlancea
Wed, 30 Oct 2019 15:54:41 -0400
changeset 58864 fba8635290df
parent 58863 c16ac7a2eba4
child 58865 6d081cef7ea8
8231451: ZipFileInputStream::skip handling of negative values with STORED entries Reviewed-by: clanger, bpb, alanb
src/java.base/share/classes/java/util/zip/ZipFile.java
test/jdk/java/util/zip/ZipFile/ZipFileInputStreamSkipTest.java
--- a/src/java.base/share/classes/java/util/zip/ZipFile.java	Wed Oct 30 19:43:52 2019 +0100
+++ b/src/java.base/share/classes/java/util/zip/ZipFile.java	Wed Oct 30 15:54:41 2019 -0400
@@ -867,6 +867,7 @@
     private class ZipFileInputStream extends InputStream {
         private volatile boolean closeRequested;
         private   long pos;     // current position within entry data
+        private   long startingPos; // Start position for the entry data
         protected long rem;     // number of remaining bytes within entry
         protected long size;    // uncompressed size of this entry
 
@@ -938,6 +939,7 @@
                     throw new ZipException("ZipFile invalid LOC header (bad signature)");
                 }
                 pos += LOCHDR + LOCNAM(loc) + LOCEXT(loc);
+                startingPos = pos; // Save starting position for the entry
             }
             return pos;
         }
@@ -979,8 +981,19 @@
         public long skip(long n) throws IOException {
             synchronized (ZipFile.this) {
                 initDataOffset();
-                if (n > rem) {
-                    n = rem;
+                long newPos = pos + n;
+                if (n > 0) {
+                    // If we overflowed adding the skip value or are moving
+                    // past EOF, set the skip value to number of bytes remaining
+                    // to reach EOF
+                    if (newPos < 0 || n > rem) {
+                        n = rem;
+                    }
+                } else if (newPos < startingPos) {
+                    // Tried to position before BOF so set position to the
+                    // BOF and return the number of bytes we moved backwards
+                    // to reach BOF
+                    n = startingPos - pos;
                 }
                 pos += n;
                 rem -= n;
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/util/zip/ZipFile/ZipFileInputStreamSkipTest.java	Wed Oct 30 15:54:41 2019 -0400
@@ -0,0 +1,289 @@
+/*
+ * 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.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.FileSystem;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
+
+import static org.testng.Assert.*;
+
+/**
+ * @test
+ * @bug 8231451
+ * @summary Basic tests for ZipFileInputStream::skip
+ * @modules jdk.zipfs
+ * @run testng/othervm ZipFileInputStreamSkipTest
+ */
+public class ZipFileInputStreamSkipTest {
+
+    // Stored and Deflated Zip File paths used by the tests
+    private final Path STORED_ZIPFILE = Path.of("skipStoredEntries.zip");
+    private final Path DEFLATED_ZIPFILE = Path.of("skipDeflatedEntries.zip");
+
+    // Saved Entries added to the relevant Zip file
+    private final HashMap<String, Entry> STORED_ZIP_ENTRIES = new HashMap<>();
+    private final HashMap<String, Entry> DEFLATED_ZIP_ENTRIES = new HashMap<>();
+
+    /**
+     * Create the Zip Files used by the tests
+     *
+     * @throws IOException If an error occurs creating the Zip Files
+     */
+    @BeforeClass
+    private void createZip() throws IOException {
+        Entry e0 = Entry.of("Entry-0", ZipEntry.STORED, "Tennis Pro");
+        Entry e1 = Entry.of("Entry-1", ZipEntry.STORED,
+                "United States Tennis Association");
+        Entry e2 = Entry.of("Entry-2", ZipEntry.DEFLATED, "Cardio Tennis");
+        Entry e3 = Entry.of("Entry-3", ZipEntry.DEFLATED, "USTA League Championships");
+
+        // Add entries
+        STORED_ZIP_ENTRIES.put(e0.name, e0);
+        STORED_ZIP_ENTRIES.put(e1.name, e1);
+        DEFLATED_ZIP_ENTRIES.put(e2.name, e2);
+        DEFLATED_ZIP_ENTRIES.put(e3.name, e3);
+
+        Files.deleteIfExists(STORED_ZIPFILE);
+        Files.deleteIfExists(DEFLATED_ZIPFILE);
+
+        createZipFile(STORED_ZIPFILE,
+                Map.of("create", "true", "noCompression", "true"),
+                e0, e1);
+
+        createZipFile(DEFLATED_ZIPFILE, Map.of("create", "true"), e2, e3);
+    }
+
+    /**
+     * Delete Zip Files created for the test
+     *
+     * @throws IOException If an error occurs during cleanup
+     */
+    @AfterClass
+    private void cleanUp() throws IOException {
+        Files.deleteIfExists(STORED_ZIPFILE);
+        Files.deleteIfExists(DEFLATED_ZIPFILE);
+    }
+
+    /**
+     * Validate that you can skip forward within a STORED entry
+     * and then read the expected data for the entry
+     *
+     * @throws Exception If an error occurs during the test
+     */
+    @Test
+    private void testStoredSkip() throws Exception {
+
+        try (ZipFile zf = new ZipFile(STORED_ZIPFILE.toFile())) {
+            var entries = zf.entries();
+            while (entries.hasMoreElements()) {
+                var entry = entries.nextElement();
+                var entrySize = entry.getSize();
+                long midpoint = entrySize / 2;
+                Entry expected = STORED_ZIP_ENTRIES.get(entry.getName());
+                assertNotNull(expected);
+                try (InputStream in = zf.getInputStream(entry)) {
+
+                    // Check that if we specify 0, that we return the correct
+                    // skip value value
+                    assertEquals(in.skip(0), 0);
+
+                    // Try to skip past EOF and should return remaining bytes
+                    assertEquals(in.skip(entrySize + 100), entrySize);
+
+                    // Return to BOF and then specify a value which would
+                    // overflow the projected skip value and return the
+                    // number of bytes moved to reach EOF
+                    assertEquals(in.skip(-entrySize), -entrySize);
+                    assertEquals(in.skip(Long.MAX_VALUE), entrySize);
+
+                    // From midpoint, try to skip past EOF and then skip back
+                    // to BOF
+                    assertEquals(in.skip(-entrySize), -entrySize);
+                    assertEquals(in.skip(midpoint), midpoint);
+                    assertEquals(in.skip(1000), entrySize - midpoint);
+                    assertEquals(in.skip(-entrySize), -entrySize);
+
+                    // Read remaining bytes and validate against expected bytes
+                    byte[] bytes = in.readAllBytes();
+                    assertEquals(bytes, expected.bytes);
+                    assertEquals(bytes.length, expected.bytes.length);
+                }
+            }
+        }
+    }
+
+    /**
+     * Validate that you can skip backwards within a STORED entry
+     * and then read the expected data for the entry
+     *
+     * @throws Exception If an error occurs during the test
+     */
+    @Test
+    private void testStoredNegativeSkip() throws Exception {
+
+        try (ZipFile zf = new ZipFile(STORED_ZIPFILE.toFile())) {
+            var entries = zf.entries();
+            while (entries.hasMoreElements()) {
+                var entry = entries.nextElement();
+                var entrySize = entry.getSize();
+                var midpoint = entrySize / 2;
+                Entry expected = STORED_ZIP_ENTRIES.get(entry.getName());
+                assertNotNull(expected);
+                try (InputStream in = zf.getInputStream(entry)) {
+
+                    // Check that if you try to move past BOF
+                    // that we return the correct value
+                    assertEquals(in.skip(-1), 0);
+                    assertEquals(in.skip(-100), 0);
+                    assertEquals(in.skip(Long.MIN_VALUE), 0);
+
+                    // Go to midpoint in file; then specify a value before
+                    // BOF which should result in the number of
+                    // bytes to BOF returned
+                    assertEquals(in.skip(midpoint), midpoint);
+                    assertEquals(in.skip(-(midpoint + 10)), -midpoint);
+
+                    // From midpoint, move back a couple of bytes
+                    assertEquals(in.skip(midpoint), midpoint);
+                    assertEquals(in.skip(-2), -2);
+
+                    // Read the remaining bytes and compare to the expected bytes
+                    byte[] bytes = in.readAllBytes();
+                    assertEquals(bytes, Arrays.copyOfRange(expected.bytes,
+                            (int)midpoint - 2, (int) entrySize));
+                    assertEquals(bytes.length, entrySize - midpoint + 2);
+                }
+            }
+        }
+    }
+
+    /**
+     * Validate that you can skip forward within a DEFLATED entry
+     * and then read the expected data for the entry
+     *
+     * @throws Exception If an error occurs during the test
+     */
+    @Test
+    private void testDeflatedSkip() throws Exception {
+        try (ZipFile zf = new ZipFile(DEFLATED_ZIPFILE.toFile())) {
+            var toSkip = 5; // Bytes to Skip
+            var entries = zf.entries();
+            while (entries.hasMoreElements()) {
+                var entry = entries.nextElement();
+                Entry expected = DEFLATED_ZIP_ENTRIES.get(entry.getName());
+                assertNotNull(expected);
+                try (InputStream in = zf.getInputStream(entry)) {
+                    assertEquals(in.skip(toSkip), toSkip);
+                    byte[] bytes = in.readAllBytes();
+                    var ebytes = Arrays.copyOfRange(expected.bytes,
+                            toSkip, expected.bytes.length);
+                    assertEquals(bytes, ebytes);
+                    assertEquals(bytes.length, expected.bytes.length - toSkip);
+                }
+            }
+        }
+    }
+
+    /**
+     * Validate that an IllegalArgumentException is thrown if you specify
+     * a negative skip value for a DEFLATED entry.
+     *
+     * @throws Exception If an unexpected error occurs during the test
+     */
+    @Test
+    private void testDeflatedIOException() throws Exception {
+        try (ZipFile zf = new ZipFile(DEFLATED_ZIPFILE.toFile())) {
+            var entries = zf.entries();
+            while (entries.hasMoreElements()) {
+                var entry = entries.nextElement();
+                assertNotNull(entry);
+                try (InputStream in = zf.getInputStream(entry)) {
+                    // Cannot specify a negative value
+                    assertThrows(IllegalArgumentException.class, () -> in.skip((-1)));
+                }
+            }
+        }
+    }
+
+    /**
+     * Create a Zip File System using the specified properties and a Zip file
+     * with the specified number of entries
+     *
+     * @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<String, String> env,
+                               Entry... entries) throws IOException {
+        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);
+        }
+    }
+
+}