RFR 8229888: Zip FS does not retain permissions when updating a Zip file

[Lance Andersen]

Here is a revised patch: http://cr.openjdk.java.net/~lancea/8229888/webrev.01/index.html <http://cr.openjdk.java.net/~lancea/8229888/webrev.01/index.html>. 


I spent some time trying leverage createTempFileinSameDirectoryAs but it was getting messy as this method is also used when creating temp files in a zip to zip copy.    I might try to revisit this later but the above seemed more straight forwarded

Best
Lance

> On Dec 3, 2019, at 6:45 PM, Lance Andersen <lance.andersen at oracle.com> wrote:
> 
> Thank you for the comments Alan.
> 
>> On Dec 3, 2019, at 4:04 PM, Alan Bateman <Alan.Bateman at oracle.com <mailto:Alan.Bateman at oracle.com>> wrote:
>> 
>> On 03/12/2019 00:59, Lance Andersen wrote:
>>> HI all,
>>> 
>>> Please review the proposed fix for an issue with Zip FS where the permissions set on the Zip file are not retained when the Zip file is updated.
>>> 
>>> The webrev can be found at: http://cr.openjdk.java.net/~lancea/8229888/webrev.00/index.html <http://cr.openjdk.java.net/~lancea/8229888/webrev.00/index.html>
>>> 
>> Testing the underlying FileStore to see if supports "posix" may be problematic or costly here as it requires finding the mount entry file the underlying file store. You should find that it is much more efficient to just call getPosixFileAttributes
> Did you mean:  Files.getPosixFilePermissions
> 
>> and catch UOE (using exceptions for control flow is not nice of course but you will avoid a lot of issues). In passing, I should mention that the non-stringy way to to check if POSIX file permissions are support is to invoke on supportsFileAttributeView(PosixFIleAttributeView.class).
> 
> 
> Would the following be more efficient than catching the IOE as I saw it used in TempFileHelper:
> 
> --------
>  boolean isPosix = FileSystems.getDefault().supportedFileAttributeViews().contains("posix");
> --------
> 
>> 
>> One other approach to look at is just letting Files.move copy the file attributes. It will copy file permissions and attempt to copy all other attributes (it only fails if it can't copy the basic file attributes to the file in its new location). Another point is that the Files.newOutputStream to create the file next to the original zip file can be created with initial file attributes. Combined with Files.move it means there is another way to do this. I only mention is because I assume there will be follow on issues to fix up other file attributes of the zip file that are lost when it is re-created.
> 
> If I am understanding this suggestion, it would be to modify the call to Files.createTempFile in the method createTempFileinSameDirectoryAs to pass the  original zip file permissions as an file attribute ?
> 
> Best
> Lance
>> 
>> -Alan
> 
> <oracle_sig_logo.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>
>  <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>
 <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/20191209/be04ed33/attachment.htm>
-------------- 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/20191209/be04ed33/oracle_sig_logo.gif>