RFR 8223771: (zipfs) FileSystemProvider.newFileSystem(Path, Map) should throw IOException when called with a file that cannot be open

[Lance Andersen]

I updated the patch: http://cr.openjdk.java.net/~lancea/8223771/webrev.01/ <http://cr.openjdk.java.net/~lancea/8223771/webrev.01/> as well as the CSR to reflect the change and lack of a property.

I reverted the change  which just returned the filename when the Exception is thrown.  My reasoning for just returning the filename was based on https://www.oracle.com/technetwork/java/seccodeguide-139067.html#2 <https://www.oracle.com/technetwork/java/seccodeguide-139067.html#2> .


Thank you again for the review!

> On Oct 3, 2019, at 9:44 AM, Lance @ Oracle <lance.andersen at oracle.com> wrote:
> 
> 
> Thank you for the feedback Alan 
> 
> 
> On Oct 3, 2019, at 9:03 AM, Alan Bateman <Alan.Bateman at oracle.com <mailto: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 <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

 <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20191003/b375cfb5/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: oracle_sig_logo.gif
Type: image/gif
Size: 658 bytes
Desc: not available
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20191003/b375cfb5/oracle_sig_logo.gif>