On Mon, 19 May 2025 13:31:04 GMT, David Beaumont <d...@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
