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

[Jan Lahoda]

On Wed, 4 Jun 2025 12:10:03 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/FSInfo.java line 183:
> 
>> 181:      *                       {@code null} to ignore release versioning).
>> 182:      */
>> 183:     public static Map<String, ?> readOnlyJarFSEnv(String releaseVersion) {
> 
> I named this after the getJarFSProvider() method above, since they are used in close proximity, but would be happy to consider other names.

I am a bit uneasy about having the method static, as this class's API is normally instance methods. I think that an instance of FSInfo (`fsInfo`) is available in both `JavacFileManager` and `Locations`, so it would be better to simply have an instance method. (The static field is OK, that's not an API.)

> src/jdk.compiler/share/classes/com/sun/tools/javac/platform/JDKPlatformProvider.java line 261:
> 
>> 259:                         // If for any reason this was not opened from a ZIP file,
>> 260:                         // then the resulting file system would not be read-only.
>> 261:                         assert fs.isReadOnly();
> 
> Not sure if an assert is the right thing here or if this should be an always-on runtime check (it can be provoked by having a non-ZIP symbol file, but it's not sanctioned in any way).

We typically don't use `assert` in javac anymore, as it can be disabled. Either the condition is important, then we typically use `com.sun.tools.javac.util.Assert.check`, or we don't do the check. I think using `Assert` here would be appropriate.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25639#discussion_r2139255923
PR Review Comment: https://git.openjdk.org/jdk/pull/25639#discussion_r2139263454