RFR 8213031: (zipfs) Add support for POSIX file permissions

Sat Jan 12 14:00:49 UTC 2019

Hi Christoph
> On Jan 12, 2019, at 8:02 AM, Langer, Christoph <christoph.langer at sap.com> wrote:
> 
> Hi Alan,
> 
> as I did not hear back from you I continued to work on the POSIX file permission support for zipfs and specified/implemented the value of 'null' for zip entries with no permission information associated (vs. UnsupportedOperationException).
> 
> I updated the CSR: https://bugs.openjdk.java.net/browse/JDK-8213082
> And here is an updated webrev for the change: http://cr.openjdk.java.net/~clanger/webrevs/8213031.5/
> 
> I also changed the first part of the module-info documentation for jdk.zipfs, mentioning the URI scheme 'jar' and  corrected the information about how the zip file system provider can be accessed and new zip filesystems can be
> created.

I think the above should be addressed via JDK-8182117 which I will be addressing and keep this focused on the POSIX  file permission enhancements as I think it should be in its own CSR.

Best
Lance
> 
> Please kindly review this.
> 
> Thank you and best regards
> Christoph
> 
>> -----Original Message-----
>> From: Langer, Christoph
>> Sent: Dienstag, 8. Januar 2019 09:27
>> To: 'Alan Bateman' <Alan.Bateman at oracle.com>; Volker Simonis
>> <volker.simonis at gmail.com>
>> Cc: nio-dev <nio-dev at openjdk.java.net>; OpenJDK Dev list <security-
>> dev at openjdk.java.net>; Java Core Libs <core-libs-dev at openjdk.java.net>
>> Subject: RE: RFR 8213031: (zipfs) Add support for POSIX file permissions
>> 
>> Hi Alan, Volker,
>> 
>> thanks for bringing up these discussion points. I agree that we shall carefully
>> revisit that.
>> 
>> First of all, I'd like to point out that PosixFileAttributeView::readAttributes
>> won't ever throw UOE with the proposed changes to zipfs. It should always
>> return an object of type PosixFileAttributes which will be an incarnation of
>> ZipFileSystem.Entry.
>> 
>> The returned PosixFileAttributes(ZipFileSystem.Entry) object now
>> implements 3 additional methods: owner(), group() and permissions(). I can
>> see that UOE should only be thrown if something is not supported by an
>> implementation at all. So it perfectly fits to owner() and group() because this
>> is simply not implemented in zipfs (yet...). As for permissions, I then agree,
>> UOE probably isn't the right thing to do because permissions will now be
>> supported by zipfs.
>> Still, we need to distinguish between the case where no permission
>> information is present vs. an empty set of permissions that was explicitly
>> stored. You brought up IOException as an alternative. But I think this is not
>> really feasible for the following main reason: IOE is no RuntimeException, so
>> it has to be declared for a method when it is thrown.
>> PosixFileAttributes::permissions, however, does not declare IOE so far. And I
>> don't think we can/want to modify the PosixFileAttributes interface for the
>> sake of that zipfs implementation change.
>> 
>> I think, we should look into using/returning null for "no permission
>> information present".
>> 
>> With that, the point is: What happens if we pass null to another
>> PosixFileAttributeView::setPermissions implementation (or to
>> Files::setPosixFilePermissions, which ends up there). For zipfs, with the
>> proposed changeset, this would work perfectly fine. We translate the null
>> value into (int)-1 for the "posixPerms" field of "Entry" and will then not set
>> permission information in the zip file. But other places in the JDK that
>> implement PosixFileAttributeView need some rework. Those are:
>> src/java.base/unix/classes/sun/nio/fs/UnixFileAttributeViews.Posix
>> src/java.base/unix/classes/sun/nio/fs/UnixSecureDirectoryStream.PosixFile
>> AttributeViewImpl
>> 
>> And we should amend the specs/doc for the following methods to define
>> the handling of the null value as a noop:
>> PosixFileAttributeView::setPermissions
>> PosixFileAttributes::permissions
>> Files::setPosixFilePermissions
>> Files::getPosixFilePermissions
>> 
>> In the implementation I can see that we modify the aforementioned places
>> to handle null by translating it to (int)-1 in
>> UnixFileModeAttribute::toUnixMode and then using this value as noop in
>> 'setMode' resp. 'fchmod'.
>> 
>> Do you think this is the right way to go? Should we maybe do the spec
>> update of the permission methods in a separate change?
>> 
>> Best regards
>> Christoph
>> 
>>> -----Original Message-----
>>> From: Alan Bateman <Alan.Bateman at oracle.com>
>>> Sent: Montag, 7. Januar 2019 21:46
>>> To: Volker Simonis <volker.simonis at gmail.com>
>>> Cc: Langer, Christoph <christoph.langer at sap.com>; nio-dev <nio-
>>> dev at openjdk.java.net>; OpenJDK Dev list <security-
>>> dev at openjdk.java.net>; Java Core Libs <core-libs-dev at openjdk.java.net>
>>> Subject: Re: RFR 8213031: (zipfs) Add support for POSIX file permissions
>>> 
>>> On 07/01/2019 19:26, Volker Simonis wrote:
>>>> :
>>>> We considered this, but it is problematic because it is perfectly
>>>> valid to have a file with external file attributes where none of the
>>>> Posix attributes is actually set (i.e. an empty set of Posix files
>>>> attributes). This wouldn't be distinguishable from the case where a
>>>> file has no external file attributes. So it seems we have to resort to
>>>> throwing an IOE?
>>>> 
>>> Maybe although it would be a bit awkward to deal with. The issues around
>>> this remind me a bit about mounting fat32 file systems on Linux or Unix
>>> systems where the fields in the stat structure are populated with
>>> default values. PosixFileAttributeView::readAttributes is  essentially
>>> the equivalent of a stat call. This might be something to look at for
>>> the file owner at least.
>>> 
>>> -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.org/pipermail/security-dev/attachments/20190112/ba7b20e2/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.org/pipermail/security-dev/attachments/20190112/ba7b20e2/oracle_sig_logo.gif>