Skip to content

Read only ZIP file system. #24506

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

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Read only ZIP file system. #24506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8c2bf36
Read only ZIP file system.
david-beaumont Apr 7, 2025
dec46fb
Include javac changes for read-only ZIP files.
david-beaumont Apr 9, 2025
4302846
Rewording module info.
david-beaumont Apr 9, 2025
ce10ed5
More simplification.
david-beaumont Apr 9, 2025
e8aff8f
Tweaking spec and code to clarify how create and readOnly mode are mu…
david-beaumont Apr 16, 2025
0d99b32
Merge branch 'master' into jdk_8350880/squashed
david-beaumont Apr 23, 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
36 changes: 33 additions & 3 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/file/FSInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,17 @@

package com.sun.tools.javac.file;

import java.io.IOError;
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.nio.file.spi.FileSystemProvider;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.StringTokenizer;
import java.util.jar.Attributes;
import java.util.jar.JarFile;
Expand Down Expand Up @@ -163,4 +161,36 @@ public synchronized FileSystemProvider getJarFSProvider() {
return null;
}

// Should match the properties/values in ZipFileSystem.java.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a helper here (it seemed like the right place). Happy to move or inline stuff.

private static final String ACCESS_MODE_KEY = "accessMode";
private static final String READ_ONLY_MODE = "readOnly";
private static final Map<String, ?> READ_ONLY_ZIPFS_ENV = Map.of(ACCESS_MODE_KEY, READ_ONLY_MODE);

/**
* Returns a {@link java.nio.file.FileSystem FileSystem} environment map
* suitable for creating a read-only JAR file-system via
* {@link jdk.nio.zipfs.ZipFileSystemProvider ZipFileSystemProvider}.
*
* @param releaseVersion the release version to use when creating a
* file-system from a multi-release JAR (or
* {@code null} to ignore release versions).
*/
public static Map<String, ?> getReadOnlyJarEnv(String releaseVersion) {
return releaseVersion == null
? READ_ONLY_ZIPFS_ENV
: Map.of(ACCESS_MODE_KEY, READ_ONLY_MODE, "releaseVersion", releaseVersion);
}

/**
* Returns a {@link java.nio.file.FileSystem FileSystem} environment map
* suitable for creating a read-only ZIP/JAR file-system via
* {@link jdk.nio.zipfs.ZipFileSystemProvider ZipFileSystemProvider}.
* <p>
* This is equivalent to {@code getReadOnlyJarEnv(null)} when a JAR is
* known to <em>not</em> be multi-release, but also suitable for any
* non-JAR ZIP file.
*/
public static Map<String, ?> getReadOnlyZipEnv() {
return READ_ONLY_ZIPFS_ENV;
}
}
6 changes: 3 additions & 3 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/file/JavacFileManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,17 +561,17 @@ private final class ArchiveContainer implements Container {

public ArchiveContainer(Path archivePath) throws IOException, ProviderNotFoundException {
this.archivePath = archivePath;

if (multiReleaseValue != null && archivePath.toString().endsWith(".jar")) {
Map<String,String> env = Collections.singletonMap("multi-release", multiReleaseValue);
FileSystemProvider jarFSProvider = fsInfo.getJarFSProvider();
Assert.checkNonNull(jarFSProvider, "should have been caught before!");
try {
this.fileSystem = jarFSProvider.newFileSystem(archivePath, env);
this.fileSystem = jarFSProvider.newFileSystem(archivePath, FSInfo.getReadOnlyJarEnv(multiReleaseValue));
} catch (ZipException ze) {
throw new IOException("ZipException opening \"" + archivePath.getFileName() + "\": " + ze.getMessage(), ze);
}
} else {
this.fileSystem = FileSystems.newFileSystem(archivePath, (ClassLoader)null);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing code is passing null, but that constructor path is identical to the ones not passing a classloader at all.
I'm not sure if the old code was calling the version for additional clarity-of-intent, but the new code has the same effect wrt class loaders.

this.fileSystem = FileSystems.newFileSystem(archivePath, FSInfo.getReadOnlyZipEnv());
}
packages = new HashMap<>();
for (Path root : fileSystem.getRootDirectories()) {
Expand Down
15 changes: 6 additions & 9 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/file/Locations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,10 @@
import com.sun.tools.javac.resources.CompilerProperties.LintWarnings;
import jdk.internal.jmod.JmodFile;

import com.sun.tools.javac.code.Lint;
import com.sun.tools.javac.code.Lint.LintCategory;
import com.sun.tools.javac.main.Option;
import com.sun.tools.javac.resources.CompilerProperties.Errors;
import com.sun.tools.javac.resources.CompilerProperties.Warnings;
import com.sun.tools.javac.util.DefinedBy;
import com.sun.tools.javac.util.DefinedBy.Api;
import com.sun.tools.javac.util.JCDiagnostic.Warning;
import com.sun.tools.javac.util.ListBuffer;
import com.sun.tools.javac.util.Log;
import com.sun.tools.javac.jvm.ModuleNameReader;
Expand Down Expand Up @@ -141,7 +137,7 @@ public class Locations {

Map<Path, FileSystem> fileSystems = new LinkedHashMap<>();
List<Closeable> closeables = new ArrayList<>();
private Map<String,String> fsEnv = Collections.emptyMap();
private String releaseVersion = null;

Locations() {
initHandlers();
Expand Down Expand Up @@ -233,7 +229,8 @@ private Iterable<Path> getPathEntries(String searchPath, Path emptyPathDefault)
}

public void setMultiReleaseValue(String multiReleaseValue) {
fsEnv = Collections.singletonMap("releaseVersion", multiReleaseValue);
// Null is implicitly allowed and unsets the value.
this.releaseVersion = multiReleaseValue;
}

private boolean contains(Collection<Path> searchPath, Path file) throws IOException {
Expand Down Expand Up @@ -384,7 +381,7 @@ public void addFile(Path file, boolean warn) {
/* Not a recognized extension; open it to see if
it looks like a valid zip file. */
try {
FileSystems.newFileSystem(file, (ClassLoader)null).close();
FileSystems.newFileSystem(file, FSInfo.getReadOnlyZipEnv()).close();
if (warn) {
lint.logIfEnabled(LintWarnings.UnexpectedArchiveFile(file));
}
Expand Down Expand Up @@ -1387,7 +1384,7 @@ private Pair<String,Path> inferModuleName(Path p) {
log.error(Errors.NoZipfsForArchive(p));
return null;
}
try (FileSystem fs = jarFSProvider.newFileSystem(p, fsEnv)) {
try (FileSystem fs = jarFSProvider.newFileSystem(p, FSInfo.getReadOnlyJarEnv(releaseVersion))) {
Path moduleInfoClass = fs.getPath("module-info.class");
if (Files.exists(moduleInfoClass)) {
String moduleName = readModuleName(moduleInfoClass);
Expand Down Expand Up @@ -1463,7 +1460,7 @@ private Pair<String,Path> inferModuleName(Path p) {
log.error(Errors.LocnCantReadFile(p));
return null;
}
fs = jarFSProvider.newFileSystem(p, Collections.emptyMap());
fs = jarFSProvider.newFileSystem(p, FSInfo.getReadOnlyZipEnv());
try {
Path moduleInfoClass = fs.getPath("classes/module-info.class");
String moduleName = readModuleName(moduleInfoClass);
Expand Down
14 changes: 9 additions & 5 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/main/JavaCompiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1756,12 +1756,16 @@ public void generate(Queue<Pair<Env<AttrContext>, JCClassDecl>> queue, Queue<Jav
if (results != null && file != null)
results.add(file);
} catch (IOException
| UncheckedIOException
| FileSystemNotFoundException
| InvalidPathException
| ReadOnlyFileSystemException ex) {
| UncheckedIOException
| FileSystemNotFoundException
| InvalidPathException
| ReadOnlyFileSystemException ex) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is just about adding slightly more useful information to the (common) case when no exception message exists. It helped me when debugging.

String msg = ex.getMessage();
if (msg == null || msg.isEmpty()) {
msg = ex.getClass().getSimpleName();
}
log.error(cdef.pos(),
Errors.ClassCantWrite(cdef.sym, ex.getMessage()));
Errors.ClassCantWrite(cdef.sym, msg));
return;
} finally {
log.useSource(prev);
Expand Down
10 changes: 6 additions & 4 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/platform/JDKPlatformProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,15 @@
import javax.annotation.processing.Processor;
import javax.tools.ForwardingJavaFileObject;
import javax.tools.JavaFileManager;
import javax.tools.JavaFileManager.Location;
import javax.tools.JavaFileObject;
import javax.tools.JavaFileObject.Kind;
import javax.tools.StandardJavaFileManager;
import javax.tools.StandardLocation;

import com.sun.source.util.Plugin;
import com.sun.tools.javac.code.Source;
import com.sun.tools.javac.code.Source.Feature;
import com.sun.tools.javac.file.CacheFSInfo;
import com.sun.tools.javac.file.FSInfo;
import com.sun.tools.javac.file.JavacFileManager;
import com.sun.tools.javac.jvm.Target;
import com.sun.tools.javac.main.Option;
Expand Down Expand Up @@ -117,7 +116,7 @@ public PlatformDescription getPlatformTrusted(String platformName) {
SUPPORTED_JAVA_PLATFORM_VERSIONS = new TreeSet<>(NUMERICAL_COMPARATOR);
Path ctSymFile = findCtSym();
if (Files.exists(ctSymFile)) {
try (FileSystem fs = FileSystems.newFileSystem(ctSymFile, (ClassLoader)null);
try (FileSystem fs = FileSystems.newFileSystem(ctSymFile, FSInfo.getReadOnlyZipEnv());
DirectoryStream<Path> dir =
Files.newDirectoryStream(fs.getRootDirectories().iterator().next())) {
for (Path section : dir) {
Expand Down Expand Up @@ -249,7 +248,10 @@ public String inferBinaryName(Location location, JavaFileObject file) {
try {
FileSystem fs = ctSym2FileSystem.get(file);
if (fs == null) {
ctSym2FileSystem.put(file, fs = FileSystems.newFileSystem(file, (ClassLoader)null));
// This is expected to be a zip file system, and we always make these read-only.
fs = FileSystems.newFileSystem(file, FSInfo.getReadOnlyZipEnv());
assert fs.isReadOnly();
ctSym2FileSystem.put(file, fs);
}

Path root = fs.getRootDirectories().iterator().next();
Expand Down
Loading