From 0ceb366dc26e2e4f6252da9dd8930b016a5d46ba Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Wed, 6 Aug 2025 08:55:47 +0000 Subject: [PATCH] 8356645: Javac should utilize new ZIP file system read-only access mode Reviewed-by: jlahoda --- .../com/sun/tools/javac/file/FSInfo.java | 34 ++++++++++++++++--- .../tools/javac/file/JavacFileManager.java | 14 ++++---- .../com/sun/tools/javac/file/Locations.java | 15 ++++---- .../javac/platform/JDKPlatformProvider.java | 22 +++++++++--- .../tools/javac/api/file/SJFM_TestBase.java | 5 +-- .../javac/platform/VerifyCTSymClassFiles.java | 14 ++++++-- 6 files changed, 73 insertions(+), 31 deletions(-) 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 index e2ab9b1679c..420ef59a647 100644 --- 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 class FSInfo { return null; } + // Must match the keys/values expected by ZipFileSystem.java. + private static final Map 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 readOnlyJarFSEnv(String releaseVersion) { + if (releaseVersion == null) { + return READ_ONLY_JARFS_ENV; + } + // Multi-release JARs need an additional attribute. + Map 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 index 28274d26542..885d377e137 100644 --- 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,15 +561,10 @@ public class JavacFileManager extends BaseFileManager implements StandardJavaFil public ArchiveContainer(Path archivePath) throws IOException, ProviderNotFoundException { this.archivePath = archivePath; - Map 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 env = fsInfo.readOnlyJarFSEnv(multiReleaseValue); try { this.fileSystem = jarFSProvider.newFileSystem(archivePath, env); } catch (ZipException ze) { @@ -577,8 +572,11 @@ public class JavacFileManager extends BaseFileManager implements StandardJavaFil } } 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 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 index 5ff55d4be3a..ff02de705fb 100644 --- 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.code.Lint; 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 fileSystems = new LinkedHashMap<>(); List closeables = new ArrayList<>(); - private Map fsEnv = Collections.emptyMap(); + private String releaseVersion = null; Locations() { initHandlers(); @@ -233,7 +229,8 @@ public class Locations { } 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 searchPath, Path file) throws IOException { @@ -480,7 +477,7 @@ public class Locations { } /** - * @see JavaFileManager#getLocationForModule(Location, JavaFileObject, String) + * @see JavaFileManager#getLocationForModule(Location, JavaFileObject) */ Location getLocationForModule(Path file) throws IOException { return null; @@ -1387,7 +1384,7 @@ public class Locations { 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 @@ public class Locations { 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 index 4c24f9892a6..2360fce1f75 100644 --- 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 java.util.TreeSet; 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.CacheFSInfo; 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 class JDKPlatformProvider implements PlatformProvider { private static final String[] symbolFileLocation = { "lib", "ct.sym" }; + // These must match attributes defined in ZipFileSystem.java. + private static final Map 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 SUPPORTED_JAVA_PLATFORM_VERSIONS; public static final Comparator NUMERICAL_COMPARATOR = (s1, s2) -> { int i1; @@ -117,7 +124,7 @@ public class JDKPlatformProvider implements PlatformProvider { 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 dir = Files.newDirectoryStream(fs.getRootDirectories().iterator().next())) { for (Path section : dir) { @@ -249,7 +256,12 @@ public class JDKPlatformProvider implements PlatformProvider { 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 index fb3860edb75..52fd36afc5d 100644 --- 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.nio.file.Paths; 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 @@ class SJFM_TestBase { List 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 index 3c563904b16..aeafb1ee08b 100644 --- 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.FileSystems; 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 @@ public class VerifyCTSymClassFiles { //no ct.sym, nothing to check: return ; } - try (FileSystem fs = FileSystems.newFileSystem(ctSym)) { + // Expected to always be a ZIP filesystem. + Map 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));