RFR 8223771: (zipfs) FileSystemProvider.newFileSystem(Path, Map) should throw IOException when called with a file that cannot be open
Lance @ Oracle
lance.andersen at oracle.com
Thu Oct 3 13:44:04 UTC 2019
Thank you for the feedback Alan
> On Oct 3, 2019, at 9:03 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
>
>> On 02/10/2019 17:45, Lance Andersen wrote:
>> Hi all,
>>
>> Please review a proposed fix to address that Zip FS does not throw an IOException if the file cannot be opened and is not being created. Zip FS has always thrown FileSystemAlreadyExistsException.
>>
>> As part of the proposed fix, I added a flag, jdk.nio.zipfs.useFileSystemNotFoundException, to allow for FileSystemAlreadyExistsException to still be thrown if needed.
>>
>> The bug can be found: https://bugs.openjdk.java.net/browse/JDK-8223771
> You can throw the more specific NoSuchFileException here as this code path is for the case that the file has been confirmed not to exist. The change also bring up the question on whether the exception message includes the full file path or just the name (I can't tell if this is the reason to reduce the message to just the file name).
Ok good point, I will change it to throw NoSuchFileException
As far as the file name I am happy to provide the full path but thought it might be best to not include the full path as have had to address similar issues where there was concern of leaking the path, which do you suggest?
>
> It's hard to say if this bug fix needs a system property to restore old behavior or not. I can't imagine there is a lot of code depending on the FileSystemAlreadyExistsException so I'm wondering if it might be simpler to leave this out initially (it can easily be added if needed). If a property is included then the name will need discussion as "useFileSystemNotFoundException" might not be the best name for this.
Ok I can remove it for now and we can revisit I just thought given the exception had always been thrown, that it would be best to add this now but undocumented.
>
> Separately, we should probably clean up all the places in this code that use AccessController.doPrivileged as the cast to PrivilegedAction is really ugly. The code would be a lot more readable if it creates the PrivilegedAction and then invokes AccessController.doPrivileged(pa). That voids the cast.
Will log a bug and address this next
>
> -Alan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20191003/c4d19c62/attachment.html>
More information about the nio-dev
mailing list