Hi Christoph > On Jan 12, 2019, at 8:02 AM, Langer, Christoph <christoph.lan...@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.bate...@oracle.com>; Volker Simonis >> <volker.simo...@gmail.com> >> Cc: nio-dev <nio-...@openjdk.java.net>; OpenJDK Dev list <security- >> d...@openjdk.java.net>; Java Core Libs <core-libs-dev@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.bate...@oracle.com> >>> Sent: Montag, 7. Januar 2019 21:46 >>> To: Volker Simonis <volker.simo...@gmail.com> >>> Cc: Langer, Christoph <christoph.lan...@sap.com>; nio-dev <nio- >>> d...@openjdk.java.net>; OpenJDK Dev list <security- >>> d...@openjdk.java.net>; Java Core Libs <core-libs-dev@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.ander...@oracle.com <mailto:lance.ander...@oracle.com>
