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

[Chen Liang]

On Wed, 4 Jun 2025 12:12:57 GMT, David Beaumont <duke at openjdk.org> wrote:

>> This PR seeks to integrate the new ZipFileSystem "accessMode" parameter to open internal ZIP/JAR files as read only, to act as defense in-depth against accidental modification.
>> 
>> Note that this currently also propagates the (currently undocumented) "zipinfo-time" parameter to several other places where ZIP/JAR files are opened, which is likely to improve performance. This was discussed and is expected to be safe (but it's something to be careful about).
>> This will, of course, be thoroughly tested before integration.
>> 
>> It also unifies several places to use a common helper method to obtain the environment map, adds more comments, and changes a small number of affected tests.
>> 
>> I'm also happy to update the original bug description to include the timestamp related changes as necessary.
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/file/Locations.java line 1463:
> 
>> 1461:                                 return null;
>> 1462:                             }
>> 1463:                             fs = jarFSProvider.newFileSystem(p, FSInfo.readOnlyJarFSEnv(null));
> 
> Happy to consider either commenting the null here (e.g. `readOnlyJarFSEnv(/* releaseVersion */ null)`), or adding a no-arg version of the method in FSInfo.

This seems like a fallback path, so don't think we should add a no-arg version to promote skipping the release version.

> test/langtools/tools/javac/jvm/ClassRefDupInConstantPoolTest.java line 46:
> 
>> 44: 
>> 45: public class ClassRefDupInConstantPoolTest {
>> 46: 
> 
> This test was failing and needed to be compiling its own class file rather than reading the one in the test library.

The class file is from this test file instead of the test libraries, and this pattern of shipping additional classes in the same compilation unit for class file handling in tests is common in jdk/classfile and other packages. Could the `clean` directive be the culprit?

> test/langtools/tools/javac/platform/VerifyCTSymClassFiles.java line 68:
> 
>> 66:         // Expected to always be a ZIP filesystem.
>> 67:         try (FileSystem fs = FileSystems.newFileSystem(ctSym, FSInfo.readOnlyJarFSEnv(null))) {
>> 68:             // Check that the file system is read only (not true if not a zip file system).
> 
> Another case where I'm not 100% sure if it should be an assert, or a runtime exception/error.

jtreg runs tests with -ea, so either works fine.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25639#discussion_r2127650560
PR Review Comment: https://git.openjdk.org/jdk/pull/25639#discussion_r2127643194
PR Review Comment: https://git.openjdk.org/jdk/pull/25639#discussion_r2127646760