Skip to content

8347712: IllegalStateException on multithreaded ZipFile access with non-UTF8 charset #3720

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+375 −52
Open

8347712: IllegalStateException on multithreaded ZipFile access with non-UTF8 charset #3720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
943dd71
8347712: IllegalStateException on multithreaded ZipFile access with n…
jaikiran May 14, 2025
6fa35f8
resolve conflicts
Jul 8, 2025
e405c77
Fix testcase compatibility with jdk17
Jul 9, 2025
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
14 changes: 12 additions & 2 deletions src/java.base/share/classes/java/util/zip/ZipCoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2009, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2009, 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
Expand Down Expand Up @@ -37,7 +37,10 @@
import sun.nio.cs.UTF_8;

/**
* Utility class for zipfile name and comment decoding and encoding
* Utility class for ZIP file entry name and comment decoding and encoding.
* <p>
* The {@code ZipCoder} for UTF-8 charset is thread safe, {@code ZipCoder}
* for other charsets require external synchronization.
*/
class ZipCoder {

Expand Down Expand Up @@ -157,6 +160,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()
Expand Down
148 changes: 98 additions & 50 deletions src/java.base/share/classes/java/util/zip/ZipFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1995, 2021, 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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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);
}
Expand Down Expand Up @@ -369,7 +375,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;
Expand Down Expand Up @@ -402,6 +408,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}.
* <p>
* 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;
Expand Down Expand Up @@ -592,7 +627,7 @@ public Stream<? extends ZipEntry> 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);
}

Expand Down Expand Up @@ -642,7 +677,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 &&
Expand Down Expand Up @@ -710,11 +745,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() {
Expand Down Expand Up @@ -1152,6 +1188,7 @@ public void setExtraAttributes(ZipEntry ze, int extraAttrs) {
isWindows = VM.getSavedProperty("os.name").contains("Windows");
}

// 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
Expand All @@ -1161,7 +1198,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;

Expand Down Expand Up @@ -1197,8 +1233,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;
Expand Down Expand Up @@ -1229,15 +1266,14 @@ 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;
entries[index] = hash;
entries[index + 1] = next;
entries[index + 2] = pos;
} catch (Exception e) {
zerror("invalid CEN header (bad entry name)");
}
Expand Down Expand Up @@ -1382,26 +1418,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())) {
Expand All @@ -1425,12 +1481,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);
}
Expand All @@ -1442,7 +1498,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) {
if (files.containsKey(key)) { // someone else put in first
Expand All @@ -1465,8 +1521,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 (isWindows) {
Expand All @@ -1480,7 +1535,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);
Expand Down Expand Up @@ -1637,8 +1692,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) {
Expand Down Expand Up @@ -1700,12 +1755,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.
Expand Down Expand Up @@ -1770,10 +1827,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;
}
Expand All @@ -1789,7 +1847,7 @@ private int getEntryPos(String name, boolean addSlash) {
int pos = getEntryPos(idx);

try {
ZipCoder zc = zipCoderForPos(pos);
final ZipCoder zc = zipCoderFor(cen, pos, zipCoder);
String entry = zc.toString(cen, pos + CENHDR, CENNAM(cen, pos));

// If addSlash is true we'll test for name+/ in addition to
Expand All @@ -1806,7 +1864,7 @@ private int getEntryPos(String name, boolean addSlash) {
&& entry.startsWith(name) &&
entry.charAt(entryLen - 1) == '/') {
// Found the entry "name+/", now find the CEN entry pos
int exactPos = getEntryPos(name, false);
int exactPos = getEntryPos(name, false, zipCoder);
return exactPos == -1 ? pos : exactPos;
}
} catch (IllegalArgumentException iae) {
Expand All @@ -1818,16 +1876,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.
Expand Down
Loading