+ * The {@code ZipCoder} for UTF-8 charset is thread safe, {@code ZipCoder} + * for other charsets require external synchronization. */ class ZipCoder { @@ -182,6 +185,13 @@ protected CharsetDecoder decoder() { return dec; } + /** + * {@return the {@link Charset} used by this {@code ZipCoder}} + */ + final Charset charset() { + return this.cs; + } + private CharsetEncoder encoder() { if (enc == null) { enc = cs.newEncoder() diff --git a/src/java.base/share/classes/java/util/zip/ZipFile.java b/src/java.base/share/classes/java/util/zip/ZipFile.java index cb9070fc885..07a7ef17b2e 100644 --- a/src/java.base/share/classes/java/util/zip/ZipFile.java +++ b/src/java.base/share/classes/java/util/zip/ZipFile.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1995, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1995, 2025, 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 @@ -96,6 +96,8 @@ public class ZipFile implements ZipConstants, Closeable { private final String name; // zip file name + // Used when decoding entry names and comments + private final ZipCoder zipCoder; private volatile boolean closeRequested; // The "resource" used by this zip file that needs to be @@ -248,7 +250,8 @@ public ZipFile(File file, int mode, Charset charset) throws IOException this.name = name; long t0 = System.nanoTime(); - this.res = new CleanableResource(this, ZipCoder.get(charset), file, mode); + this.zipCoder = ZipCoder.get(charset); + this.res = new CleanableResource(this, zipCoder, file, mode); PerfCounter.getZipFileOpenTime().addElapsedTimeFrom(t0); PerfCounter.getZipFileCount().increment(); @@ -319,7 +322,7 @@ public String getComment() { if (res.zsrc.comment == null) { return null; } - return res.zsrc.zc.toString(res.zsrc.comment); + return zipCoder.toString(res.zsrc.comment); } } @@ -336,7 +339,10 @@ public ZipEntry getEntry(String name) { ZipEntry entry = null; synchronized (this) { ensureOpen(); - int pos = res.zsrc.getEntryPos(name, true); + // Look up the name and CEN header position of the entry. + // The resolved name may include a trailing slash. + // See Source::getEntryPos for details. + int pos = res.zsrc.getEntryPos(name, true, zipCoder); if (pos != -1) { entry = getZipEntry(name, pos); } @@ -376,7 +382,7 @@ public InputStream getInputStream(ZipEntry entry) throws IOException { if (Objects.equals(lastEntryName, entry.name)) { pos = lastEntryPos; } else { - pos = zsrc.getEntryPos(entry.name, false); + pos = zsrc.getEntryPos(entry.name, false, zipCoder); } if (pos == -1) { return null; @@ -409,6 +415,35 @@ public InputStream getInputStream(ZipEntry entry) throws IOException { } } + /** + * Determines and returns a {@link ZipCoder} to use for decoding + * name and comment fields of the ZIP entry identified by the {@code pos} + * in the ZIP file's {@code cen}. + *
+ * A ZIP entry's name and comment fields may be encoded using UTF-8, in
+ * which case this method returns a UTF-8 capable {@code ZipCoder}. If the
+ * entry doesn't require UTF-8, then this method returns the {@code fallback}
+ * {@code ZipCoder}.
+ *
+ * @param cen the CEN
+ * @param pos the ZIP entry's position in CEN
+ * @param fallback the fallback ZipCoder to return if the entry doesn't require UTF-8
+ */
+ private static ZipCoder zipCoderFor(final byte[] cen, final int pos, final ZipCoder fallback) {
+ if (fallback.isUTF8()) {
+ // the fallback ZipCoder is capable of handling UTF-8,
+ // so no need to parse the entry flags to determine if
+ // the entry has UTF-8 flag.
+ return fallback;
+ }
+ if ((CENFLG(cen, pos) & USE_UTF8) != 0) {
+ // entry requires a UTF-8 ZipCoder
+ return ZipCoder.UTF8;
+ }
+ // entry doesn't require a UTF-8 ZipCoder
+ return fallback;
+ }
+
private static class InflaterCleanupAction implements Runnable {
private final Inflater inf;
private final CleanableResource res;
@@ -599,7 +634,7 @@ public Stream stream() {
private String getEntryName(int pos) {
byte[] cen = res.zsrc.cen;
int nlen = CENNAM(cen, pos);
- ZipCoder zc = res.zsrc.zipCoderForPos(pos);
+ ZipCoder zc = zipCoderFor(cen, pos, zipCoder);
return zc.toString(cen, pos + CENHDR, nlen);
}
@@ -649,7 +684,7 @@ private ZipEntry getZipEntry(String name, int pos) {
int elen = CENEXT(cen, pos);
int clen = CENCOM(cen, pos);
- ZipCoder zc = res.zsrc.zipCoderForPos(pos);
+ ZipCoder zc = zipCoderFor(cen, pos, zipCoder);
if (name != null) {
// only need to check for mismatch of trailing slash
if (nlen > 0 &&
@@ -717,11 +752,12 @@ private static class CleanableResource implements Runnable {
Source zsrc;
- CleanableResource(ZipFile zf, ZipCoder zc, File file, int mode) throws IOException {
+ CleanableResource(ZipFile zf, ZipCoder zipCoder, File file, int mode) throws IOException {
+ assert zipCoder != null : "null ZipCoder";
this.cleanable = CleanerFactory.cleaner().register(zf, this);
this.istreams = Collections.newSetFromMap(new WeakHashMap<>());
this.inflaterCache = new ArrayDeque<>();
- this.zsrc = Source.get(file, (mode & OPEN_DELETE) != 0, zc);
+ this.zsrc = Source.get(file, (mode & OPEN_DELETE) != 0, zipCoder);
}
void clean() {
@@ -1157,6 +1193,7 @@ public void setExtraAttributes(ZipEntry ze, int extraAttrs) {
);
}
+ // Implementation note: This class is thread safe.
private static class Source {
// While this is only used from ZipFile, defining it there would cause
// a bootstrap cycle that would leave this initialized as null
@@ -1166,7 +1203,6 @@ private static class Source {
private static final int[] EMPTY_META_VERSIONS = new int[0];
private final Key key; // the key in files
- private final @Stable ZipCoder zc; // zip coder used to decode/encode
private int refs = 1;
@@ -1202,8 +1238,9 @@ private static class Source {
private int[] entries; // array of hashed cen entry
// Checks the entry at offset pos in the CEN, calculates the Entry values as per above,
- // then returns the length of the entry name.
- private int checkAndAddEntry(int pos, int index)
+ // then returns the length of the entry name. Uses the given zipCoder for processing the
+ // entry name and the entry comment (if any).
+ private int checkAndAddEntry(final int pos, final int index, final ZipCoder zipCoder)
throws ZipException
{
byte[] cen = this.cen;
@@ -1234,22 +1271,21 @@ private int checkAndAddEntry(int pos, int index)
}
try {
- ZipCoder zcp = zipCoderForPos(pos);
- int hash = zcp.checkedHash(cen, entryPos, nlen);
+ int hash = zipCoder.checkedHash(cen, entryPos, nlen);
int hsh = (hash & 0x7fffffff) % tablelen;
int next = table[hsh];
table[hsh] = index;
// Record the CEN offset and the name hash in our hash cell.
- entries[index++] = hash;
- entries[index++] = next;
- entries[index ] = pos;
- // Validate comment if it exists
- // if the bytes representing the comment cannot be converted to
+ entries[index] = hash;
+ entries[index + 1] = next;
+ entries[index + 2] = pos;
+ // Validate comment if it exists.
+ // If the bytes representing the comment cannot be converted to
// a String via zcp.toString, an Exception will be thrown
int clen = CENCOM(cen, pos);
if (clen > 0) {
int start = entryPos + nlen + elen;
- zcp.toString(cen, start, clen);
+ zipCoder.toString(cen, start, clen);
}
} catch (Exception e) {
zerror("invalid CEN header (bad entry name or comment)");
@@ -1395,26 +1431,46 @@ private static boolean isZip64ExtBlockSizeValid(int blockSize) {
private int[] table; // Hash chain heads: indexes into entries
private int tablelen; // number of hash heads
+ /**
+ * A class representing a key to the Source of a ZipFile.
+ * The Key is composed of:
+ * - The BasicFileAttributes.fileKey() if available, or the Path of the ZIP file
+ * if the fileKey() is not available.
+ * - The ZIP file's last modified time (to allow for cases
+ * where a ZIP file is re-opened after it has been modified).
+ * - The Charset that was provided when constructing the ZipFile instance.
+ * The unique combination of these components identifies a Source of a ZipFile.
+ */
private static class Key {
- final BasicFileAttributes attrs;
- File file;
- final boolean utf8;
-
- public Key(File file, BasicFileAttributes attrs, ZipCoder zc) {
+ private final BasicFileAttributes attrs;
+ private final File file;
+ // the Charset that was provided when constructing the ZipFile instance
+ private final Charset charset;
+
+ /**
+ * Constructs a {@code Key} to a {@code Source} of a {@code ZipFile}
+ *
+ * @param file the ZIP file
+ * @param attrs the attributes of the ZIP file
+ * @param charset the Charset that was provided when constructing the ZipFile instance
+ */
+ public Key(File file, BasicFileAttributes attrs, Charset charset) {
this.attrs = attrs;
this.file = file;
- this.utf8 = zc.isUTF8();
+ this.charset = charset;
}
+ @Override
public int hashCode() {
- long t = utf8 ? 0 : Long.MAX_VALUE;
+ long t = charset.hashCode();
t += attrs.lastModifiedTime().toMillis();
return ((int)(t ^ (t >>> 32))) + file.hashCode();
}
+ @Override
public boolean equals(Object obj) {
if (obj instanceof Key key) {
- if (key.utf8 != utf8) {
+ if (!charset.equals(key.charset)) {
return false;
}
if (!attrs.lastModifiedTime().equals(key.attrs.lastModifiedTime())) {
@@ -1438,12 +1494,12 @@ public boolean equals(Object obj) {
private static final java.nio.file.FileSystem builtInFS =
DefaultFileSystemProvider.theFileSystem();
- static Source get(File file, boolean toDelete, ZipCoder zc) throws IOException {
+ static Source get(File file, boolean toDelete, ZipCoder zipCoder) throws IOException {
final Key key;
try {
key = new Key(file,
Files.readAttributes(builtInFS.getPath(file.getPath()),
- BasicFileAttributes.class), zc);
+ BasicFileAttributes.class), zipCoder.charset());
} catch (InvalidPathException ipe) {
throw new IOException(ipe);
}
@@ -1455,7 +1511,7 @@ static Source get(File file, boolean toDelete, ZipCoder zc) throws IOException {
return src;
}
}
- src = new Source(key, toDelete, zc);
+ src = new Source(key, toDelete, zipCoder);
synchronized (files) {
Source prev = files.putIfAbsent(key, src);
@@ -1477,8 +1533,7 @@ static void release(Source src) throws IOException {
}
}
- private Source(Key key, boolean toDelete, ZipCoder zc) throws IOException {
- this.zc = zc;
+ private Source(Key key, boolean toDelete, ZipCoder zipCoder) throws IOException {
this.key = key;
if (toDelete) {
if (OperatingSystem.isWindows()) {
@@ -1492,7 +1547,7 @@ private Source(Key key, boolean toDelete, ZipCoder zc) throws IOException {
this.zfile = new RandomAccessFile(key.file, "r");
}
try {
- initCEN(-1);
+ initCEN(-1, zipCoder);
byte[] buf = new byte[4];
readFullyAt(buf, 0, 4, 0);
this.startsWithLoc = (LOCSIG(buf) == LOCSIG);
@@ -1649,8 +1704,8 @@ private End findEND() throws IOException {
throw new ZipException("zip END header not found");
}
- // Reads zip file central directory.
- private void initCEN(int knownTotal) throws IOException {
+ // Reads ZIP file central directory.
+ private void initCEN(final int knownTotal, final ZipCoder zipCoder) throws IOException {
// Prefer locals for better performance during startup
byte[] cen;
if (knownTotal == -1) {
@@ -1712,12 +1767,14 @@ private void initCEN(int knownTotal) throws IOException {
// This will only happen if the zip file has an incorrect
// ENDTOT field, which usually means it contains more than
// 65535 entries.
- initCEN(countCENHeaders(cen, limit));
+ initCEN(countCENHeaders(cen, limit), zipCoder);
return;
}
+ // the ZipCoder for any non-UTF8 entries
+ final ZipCoder entryZipCoder = zipCoderFor(cen, pos, zipCoder);
// Checks the entry and adds values to entries[idx ... idx+2]
- int nlen = checkAndAddEntry(pos, idx);
+ int nlen = checkAndAddEntry(pos, idx, entryZipCoder);
idx += 3;
// Adds name to metanames.
@@ -1782,10 +1839,11 @@ private static void zerror(String msg) throws ZipException {
}
/*
- * Returns the {@code pos} of the zip cen entry corresponding to the
- * specified entry name, or -1 if not found.
+ * Returns the resolved name and position of the ZIP cen entry corresponding
+ * to the specified entry name, or {@code null} if not found.
*/
- private int getEntryPos(String name, boolean addSlash) {
+ private int getEntryPos(final String name, final boolean addSlash,
+ final ZipCoder zipCoder) {
if (total == 0) {
return -1;
}
@@ -1804,8 +1862,7 @@ private int getEntryPos(String name, boolean addSlash) {
int noff = pos + CENHDR;
int nlen = CENNAM(cen, pos);
- ZipCoder zc = zipCoderForPos(pos);
-
+ final ZipCoder zc = zipCoderFor(cen, pos, zipCoder);
// Compare the lookup name with the name encoded in the CEN
switch (zc.compare(name, cen, noff, nlen, addSlash)) {
case EXACT_MATCH:
@@ -1831,16 +1888,6 @@ private int getEntryPos(String name, boolean addSlash) {
return -1;
}
- private ZipCoder zipCoderForPos(int pos) {
- if (zc.isUTF8()) {
- return zc;
- }
- if ((CENFLG(cen, pos) & USE_UTF8) != 0) {
- return ZipCoder.UTF8;
- }
- return zc;
- }
-
/**
* Returns true if the bytes represent a non-directory name
* beginning with "META-INF/", disregarding ASCII case.
diff --git a/test/jdk/java/util/zip/ZipFile/ZipFileCharsetTest.java b/test/jdk/java/util/zip/ZipFile/ZipFileCharsetTest.java
new file mode 100644
index 00000000000..e97088247f4
--- /dev/null
+++ b/test/jdk/java/util/zip/ZipFile/ZipFileCharsetTest.java
@@ -0,0 +1,125 @@
+/*
+ * Copyright (c) 2025, 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 java.io.IOException;
+import java.nio.charset.Charset;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
+import java.util.zip.ZipOutputStream;
+
+import org.junit.jupiter.api.Test;
+import static java.nio.charset.StandardCharsets.ISO_8859_1;
+import static java.nio.charset.StandardCharsets.US_ASCII;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+
+/*
+ * @test
+ * @bug 8355975
+ * @summary verify that the internal ZIP structure caching in java.util.zip.ZipFile
+ * uses the correct Charset when parsing the ZIP structure of a ZIP file
+ * @run junit ZipFileCharsetTest
+ */
+public class ZipFileCharsetTest {
+
+ private static final String ISO_8859_15_NAME = "ISO-8859-15";
+
+ /**
+ * The internal implementation of java.util.zip.ZipFile maintains a cache
+ * of the ZIP structure of each ZIP file that's currently open. This cache
+ * helps prevent repeat parsing of the ZIP structure of the same underlying
+ * ZIP file, every time a ZipFile instance is created for the same ZIP file.
+ * The cache uses an internal key to map a ZIP file to the corresponding
+ * ZIP structure that's cached.
+ * A ZipFile can be constructed by passing a Charset which will be used to
+ * decode the entry names (and comment) in a ZIP file.
+ * The test verifies that when multiple ZipFile instances are
+ * constructed using different Charsets but the same underlying ZIP file,
+ * then the internal caching implementation of ZipFile doesn't end up using
+ * a wrong Charset for parsing the ZIP structure of the ZIP file.
+ */
+ @Test
+ void testCachedZipFileSource() throws Exception {
+ // ISO-8859-15 is not a standard charset in Java. We skip this test
+ // when it is unavailable
+ assumeTrue(Charset.availableCharsets().containsKey(ISO_8859_15_NAME),
+ "skipping test since " + ISO_8859_15_NAME + " charset isn't available");
+
+ // We choose the byte 0xA4 for entry name in the ZIP file.
+ // 0xA4 is "Euro sign" in ISO-8859-15 charset and
+ // "Currency sign (generic)" in ISO-8859-1 charset.
+ final byte[] entryNameBytes = new byte[]{(byte) 0xA4}; // intentional cast
+ final Charset euroSignCharset = Charset.forName(ISO_8859_15_NAME);
+ final Charset currencySignCharset = ISO_8859_1;
+
+ final String euroSign = new String(entryNameBytes, euroSignCharset);
+ final String currencySign = new String(entryNameBytes, currencySignCharset);
+
+ // create a ZIP file whose entry name is encoded using ISO-8859-15 charset
+ final Path zip = createZIP("euro", euroSignCharset, entryNameBytes);
+
+ // Construct a ZipFile instance using the (incorrect) charset ISO-8859-1.
+ // While that ZipFile instance is still open (and the ZIP file structure
+ // still cached), construct another instance for the same ZIP file, using
+ // the (correct) charset ISO-8859-15.
+ try (ZipFile incorrect = new ZipFile(zip.toFile(), currencySignCharset);
+ ZipFile correct = new ZipFile(zip.toFile(), euroSignCharset)) {
+
+ // correct encoding should resolve the entry name to euro sign
+ // and the entry should be thus be located
+ assertNotNull(correct.getEntry(euroSign), "euro sign entry missing in " + correct);
+ // correct encoding should not be able to find an entry name
+ // with the currency sign
+ assertNull(correct.getEntry(currencySign), "currency sign entry unexpectedly found in "
+ + correct);
+
+ // incorrect encoding should resolve the entry name to currency sign
+ // and the entry should be thus be located by the currency sign name
+ assertNotNull(incorrect.getEntry(currencySign), "currency sign entry missing in "
+ + incorrect);
+ // incorrect encoding should not be able to find an entry name
+ // with the euro sign
+ assertNull(incorrect.getEntry(euroSign), "euro sign entry unexpectedly found in "
+ + incorrect);
+ }
+ }
+
+ /**
+ * Creates and return ZIP file whose entry names are encoded using the given {@code charset}
+ */
+ private static Path createZIP(final String fileNamePrefix, final Charset charset,
+ final byte[] entryNameBytes) throws IOException {
+ final Path zip = Files.createTempFile(Path.of("."), fileNamePrefix, ".zip");
+ // create a ZIP file whose entry name(s) use the given charset
+ try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(zip), charset)) {
+ zos.putNextEntry(new ZipEntry(new String(entryNameBytes, charset)));
+ final byte[] entryContent = "doesnotmatter".getBytes(US_ASCII);
+ zos.write(entryContent);
+ zos.closeEntry();
+ }
+ return zip;
+ }
+}
diff --git a/test/jdk/java/util/zip/ZipFile/ZipFileSharedSourceTest.java b/test/jdk/java/util/zip/ZipFile/ZipFileSharedSourceTest.java
new file mode 100644
index 00000000000..d4d057ede7b
--- /dev/null
+++ b/test/jdk/java/util/zip/ZipFile/ZipFileSharedSourceTest.java
@@ -0,0 +1,137 @@
+/*
+ * Copyright (c) 2025, 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 java.io.OutputStream;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
+import java.util.zip.ZipOutputStream;
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import static java.nio.charset.StandardCharsets.US_ASCII;
+
+/*
+ * @test
+ * @bug 8347712
+ * @summary verify that different instances of java.util.zip.ZipFile do not share
+ * the same instance of (non-thread-safe) java.nio.charset.CharsetEncoder/CharsetDecoder
+ * @run junit ZipFileSharedSourceTest
+ */
+public class ZipFileSharedSourceTest {
+
+ static Path createZipFile(final Charset charset) throws Exception {
+ final Path zipFilePath = Files.createTempFile(Path.of("."), "8347712", ".zip");
+ try (OutputStream os = Files.newOutputStream(zipFilePath);
+ ZipOutputStream zos = new ZipOutputStream(os, charset)) {
+ final int numEntries = 10240;
+ for (int i = 1; i <= numEntries; i++) {
+ final ZipEntry entry = new ZipEntry("entry-" + i);
+ zos.putNextEntry(entry);
+ zos.write("foo bar".getBytes(US_ASCII));
+ zos.closeEntry();
+ }
+ }
+ return zipFilePath;
+ }
+
+ static List