Applying read-only configuration for Javac internal use of ZIP/JAR file systems.

david-beaumont · david-beaumont · commit bc2f52329494 · 2025-06-18T21:19:56.000+02:00
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
@@ -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 static 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,13 +561,9 @@ 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");
-
+            // Common parameters for opening ZIP/JAR file systems in Javac.
+            Map<String, ?> env = FSInfo.readOnlyJarFSEnv(multiReleaseValue);
             if (multiReleaseValue != null && archivePath.toString().endsWith(".jar")) {
-                env.put("multi-release", multiReleaseValue);
                 FileSystemProvider jarFSProvider = fsInfo.getJarFSProvider();
                 Assert.checkNonNull(jarFSProvider, "should have been caught before!");
                 try {
@@ -577,8 +573,10 @@ public ArchiveContainer(Path archivePath) throws IOException, ProviderNotFoundEx
                 }
             } 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.
+                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
@@ -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;
@@ -96,6 +94,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 +123,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 +255,11 @@ 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.
+                        assert 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
@@ -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/jvm/ClassRefDupInConstantPoolTest.java b/test/langtools/tools/javac/jvm/ClassRefDupInConstantPoolTest.java
@@ -25,19 +25,37 @@
  * @test
  * @bug 8015927
  * @summary Class reference duplicates in constant pool
- * @clean ClassRefDupInConstantPoolTest$Duplicates
+ * @library /tools/lib
+ * @modules jdk.compiler/com.sun.tools.javac.api
+ *          jdk.compiler/com.sun.tools.javac.main
+ * @build toolbox.JavacTask toolbox.ToolBox
  * @run main ClassRefDupInConstantPoolTest
  */
 
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.TreeSet;
 
+import toolbox.JavacTask;
+import toolbox.ToolBox;
+
 import java.lang.classfile.*;
 import java.lang.classfile.constantpool.*;
 
 public class ClassRefDupInConstantPoolTest {
+
+    private static final String DUPLICATE_REFS_CLASS =
+            """
+            class Duplicates {
+                String concat(String s1, String s2) {
+                    return s1 + (s2 == s1 ? " " : s2);
+                }
+            }""";
+
     public static void main(String[] args) throws Exception {
-        ClassModel cls = ClassFile.of().parse(ClassRefDupInConstantPoolTest.class.
-                                       getResourceAsStream("ClassRefDupInConstantPoolTest$Duplicates.class").readAllBytes());
+        new JavacTask(new ToolBox()).sources(DUPLICATE_REFS_CLASS).run();
+
+        ClassModel cls = ClassFile.of().parse(Files.readAllBytes(Path.of("Duplicates.class")));
         ConstantPool pool = cls.constantPool();
 
         int duplicates = 0;
diff --git a/test/langtools/tools/javac/platform/VerifyCTSymClassFiles.java b/test/langtools/tools/javac/platform/VerifyCTSymClassFiles.java
@@ -27,13 +27,16 @@
  * @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
  * @build toolbox.ToolBox VerifyCTSymClassFiles
  * @run main VerifyCTSymClassFiles
  */
 
+import com.sun.tools.javac.file.FSInfo;
+
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.lang.classfile.ClassFile;
@@ -60,7 +63,12 @@ void checkClassFiles() throws IOException {
             //no ct.sym, nothing to check:
             return ;
         }
-        try (FileSystem fs = FileSystems.newFileSystem(ctSym)) {
+        // Expected to always be a ZIP filesystem.
+        try (FileSystem fs = FileSystems.newFileSystem(ctSym, FSInfo.readOnlyJarFSEnv(null))) {
+            // 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));
