8356645: Javac should utilize new ZIP file system read-only access mode

david-beaumont · lahodaj · commit 0ceb366dc26e · 2025-08-06T08:55:47.000Z
Reviewed-by: jlahoda
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/file/FSInfo.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/file/FSInfo.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 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
@@ -25,19 +25,19 @@
 
 package com.sun.tools.javac.file;
 
-import java.io.IOError;
 import java.io.IOException;
 import java.net.MalformedURLException;
+import java.net.URI;
 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.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.StringTokenizer;
 import java.util.jar.Attributes;
 import java.util.jar.JarFile;
@@ -163,4 +163,30 @@ public synchronized FileSystemProvider getJarFSProvider() {
         return null;
     }
 
+    // Must match the keys/values expected by ZipFileSystem.java.
+    private static final Map<String, String> READ_ONLY_JARFS_ENV = Map.of(
+            // Jar files opened by Javac should always be read-only.
+            "accessMode", "readOnly",
+            // ignores timestamps not stored in ZIP central directory, reducing I/O.
+            "zipinfo-time", "false");
+
+    /**
+     * Returns a {@link java.nio.file.FileSystem FileSystem} environment map
+     * suitable for creating read-only JAR file-systems with default timestamp
+     * information via {@link FileSystemProvider#newFileSystem(Path, Map)}
+     * or {@link java.nio.file.FileSystems#newFileSystem(Path, Map)}.
+     *
+     * @param releaseVersion the release version to use when creating a
+     *                       file-system from a multi-release JAR (or
+     *                       {@code null} to ignore release versioning).
+     */
+    public Map<String, ?> readOnlyJarFSEnv(String releaseVersion) {
+        if (releaseVersion == null) {
+            return READ_ONLY_JARFS_ENV;
+        }
+        // Multi-release JARs need an additional attribute.
+        Map<String, String> env = new HashMap<>(READ_ONLY_JARFS_ENV);
+        env.put("releaseVersion", releaseVersion);
+        return Collections.unmodifiableMap(env);
+    }
 }
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/file/JavacFileManager.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/file/JavacFileManager.java
@@ -561,24 +561,22 @@ private final class ArchiveContainer implements Container {
 
         public ArchiveContainer(Path archivePath) throws IOException, ProviderNotFoundException {
             this.archivePath = archivePath;
-            Map<String,String> env = new HashMap<>();
-            // ignores timestamps not stored in ZIP central directory, reducing I/O
-            // This key is handled by ZipFileSystem only.
-            env.put("zipinfo-time", "false");
-
             if (multiReleaseValue != null && archivePath.toString().endsWith(".jar")) {
-                env.put("multi-release", multiReleaseValue);
                 FileSystemProvider jarFSProvider = fsInfo.getJarFSProvider();
                 Assert.checkNonNull(jarFSProvider, "should have been caught before!");
+                Map<String, ?> env = fsInfo.readOnlyJarFSEnv(multiReleaseValue);
                 try {
                     this.fileSystem = jarFSProvider.newFileSystem(archivePath, env);
                 } catch (ZipException ze) {
                     throw new IOException("ZipException opening \"" + archivePath.getFileName() + "\": " + ze.getMessage(), ze);
                 }
             } else {
                 // Less common case is possible if the file manager was not initialized in JavacTask,
-                // or if non "*.jar" files are on the classpath.
-                this.fileSystem = FileSystems.newFileSystem(archivePath, env, (ClassLoader)null);
+                // or if non "*.jar" files are on the classpath. If this is not a ZIP/JAR file then it
+                // will ignore ZIP specific parameters in env, and may not end up being read-only.
+                // However, Javac should never attempt to write back to archives either way.
+                Map<String, ?> env = fsInfo.readOnlyJarFSEnv(null);
+                this.fileSystem = FileSystems.newFileSystem(archivePath, env);
             }
             packages = new HashMap<>();
             for (Path root : fileSystem.getRootDirectories()) {
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/file/Locations.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/file/Locations.java
@@ -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;
@@ -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();
@@ -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 {
@@ -480,7 +477,7 @@ Location getLocationForModule(String moduleName) throws IOException {
         }
 
         /**
-         * @see JavaFileManager#getLocationForModule(Location, JavaFileObject, String)
+         * @see JavaFileManager#getLocationForModule(Location, JavaFileObject)
          */
         Location getLocationForModule(Path file) throws IOException  {
             return null;
@@ -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.readOnlyJarFSEnv(releaseVersion))) {
                         Path moduleInfoClass = fs.getPath("module-info.class");
                         if (Files.exists(moduleInfoClass)) {
                             String moduleName = readModuleName(moduleInfoClass);
@@ -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.readOnlyJarFSEnv(null));
                             try {
                                 Path moduleInfoClass = fs.getPath("classes/module-info.class");
                                 String moduleName = readModuleName(moduleInfoClass);
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/platform/JDKPlatformProvider.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/platform/JDKPlatformProvider.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 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
@@ -51,10 +51,8 @@
 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;
@@ -64,6 +62,7 @@
 import com.sun.tools.javac.file.JavacFileManager;
 import com.sun.tools.javac.jvm.Target;
 import com.sun.tools.javac.main.Option;
+import com.sun.tools.javac.util.Assert;
 import com.sun.tools.javac.util.Context;
 import com.sun.tools.javac.util.Log;
 import com.sun.tools.javac.util.StringUtils;
@@ -96,6 +95,14 @@ public PlatformDescription getPlatformTrusted(String platformName) {
 
     private static final String[] symbolFileLocation = { "lib", "ct.sym" };
 
+    // These must match attributes defined in ZipFileSystem.java.
+    private static final Map<String, ?> CT_SYM_ZIP_ENV = Map.of(
+            // Symbol file should always be opened read-only.
+            "accessMode", "readOnly",
+            // Uses less accurate, but faster, timestamp information
+            // (nobody should care about timestamps in the CT symbol file).
+            "zipinfo-time", "false");
+
     private static final Set<String> SUPPORTED_JAVA_PLATFORM_VERSIONS;
     public static final Comparator<String> NUMERICAL_COMPARATOR = (s1, s2) -> {
         int i1;
@@ -117,7 +124,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, CT_SYM_ZIP_ENV);
                  DirectoryStream<Path> dir =
                          Files.newDirectoryStream(fs.getRootDirectories().iterator().next())) {
                 for (Path section : dir) {
@@ -249,7 +256,12 @@ 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));
+                        fs = FileSystems.newFileSystem(file, CT_SYM_ZIP_ENV);
+                        // If for any reason this was not opened from a ZIP file,
+                        // then the resulting file system would not be read-only.
+                        // NOTE: This check is disabled until JDK 25 bootstrap!
+                        // Assert.check(fs.isReadOnly());
+                        ctSym2FileSystem.put(file, fs);
                     }
 
                     Path root = fs.getRootDirectories().iterator().next();
diff --git a/test/langtools/tools/javac/api/file/SJFM_TestBase.java b/test/langtools/tools/javac/api/file/SJFM_TestBase.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 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
@@ -37,6 +37,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
 import java.util.stream.Collectors;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipOutputStream;
@@ -160,7 +161,7 @@ List<Path> getTestFilePaths() throws IOException {
     List<Path> getTestZipPaths() throws IOException {
         if (zipfs == null) {
             Path testZip = createSourceZip();
-            zipfs = FileSystems.newFileSystem(testZip);
+            zipfs = FileSystems.newFileSystem(testZip, Map.of("accessMode", "readOnly"));
             closeables.add(zipfs);
             zipPaths = Files.list(zipfs.getRootDirectories().iterator().next())
                 .filter(p -> p.getFileName().toString().endsWith(".java"))
diff --git a/test/langtools/tools/javac/platform/VerifyCTSymClassFiles.java b/test/langtools/tools/javac/platform/VerifyCTSymClassFiles.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2024, 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
@@ -23,10 +23,11 @@
 
 /**
  * @test
- * @bug 8331027
+ * @bug 8331027 8356645
  * @summary Verify classfile inside ct.sym
  * @library /tools/lib
  * @modules jdk.compiler/com.sun.tools.javac.api
+ *          jdk.compiler/com.sun.tools.javac.file
  *          jdk.compiler/com.sun.tools.javac.main
  *          jdk.compiler/com.sun.tools.javac.platform
  *          jdk.compiler/com.sun.tools.javac.util:+open
@@ -44,6 +45,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.Map;
 
 public class VerifyCTSymClassFiles {
 
@@ -60,7 +62,13 @@ void checkClassFiles() throws IOException {
             //no ct.sym, nothing to check:
             return ;
         }
-        try (FileSystem fs = FileSystems.newFileSystem(ctSym)) {
+        // Expected to always be a ZIP filesystem.
+        Map<String, ?> env = Map.of("accessMode", "readOnly");
+        try (FileSystem fs = FileSystems.newFileSystem(ctSym, env)) {
+            // Check that the file system is read only (not true if not a zip file system).
+            if (!fs.isReadOnly()) {
+                throw new AssertionError("Expected read-only file system");
+            }
             Files.walk(fs.getRootDirectories().iterator().next())
                  .filter(p -> Files.isRegularFile(p))
                  .forEach(p -> checkClassFile(p));
