RFR: 8350880: (zipfs) Add support for read-only zip file systems [v6]

[Jaikiran Pai]

On Mon, 19 May 2025 13:31:04 GMT, David Beaumont <duke at openjdk.org> wrote:

>> test/jdk/jdk/nio/zipfs/NewFileSystemTests.java line 208:
>> 
>>> 206:         // Multi-release JARs, when opened with a specified version are inherently read-only.
>>> 207:         Path multiReleaseJar = createMultiReleaseJar();
>>> 208:         try (FileSystem fs = FileSystems.newFileSystem(multiReleaseJar, Map.of("accessMode", "readWrite"))) {
>> 
>> I wonder if the ZIP filesystem implementation should throw an exception in this case. In the case where the application code has explicitly stated `accessMode=readWrite`, if the underlying file `Path` isn't writable, then we currently throw an `IOException`. I think we should specify and implement the same for the case where `accessMode=readWrite` and the file is a multi-release JAR file.
>
> It does throw if accessMode=readWrite and the result is not writable for any reason.
> 
> 
> this.readOnly = forceReadOnly || multiReleaseVersion.isPresent() || !Files.isWritable(zfpath);
> if (readOnly && accessMode == ZipAccessMode.READ_WRITE) {

Actually looking at the current proposed implementation in ZipFileSystem where we have:



this.readOnly = forceReadOnly || multiReleaseVersion.isPresent() || !Files.isWritable(zfpath);
if (readOnly && accessMode == ZipAccessMode.READ_WRITE) {
    String reason = multiReleaseVersion.isPresent()
            ? "the multi-release JAR file is not writable"
            : "the ZIP file is not writable";
    throw new IOException(reason);
}


I'm surprised this test currently passes. Shouldn't this line in the test lead to an `IOException`:

FileSystems.newFileSystem(multiReleaseJar, Map.of("accessMode", "readWrite")))


Did I misread something?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2095732163