Hi Lance, I was not aware of JDK-8182117 and by its description it does not fit exactly to the updates to jdk.zipfs module documentation that I propose. However, yes, it is probably a bit more natural to include that part in a potential patch for JDK-8182117. So, feel free to take this over into your work. Seeing how things are going with this POSIX permission topic, I’m sure you’ll be done with a patch for 8182117 much earlier than me 😉.
Best Christoph From: Lance Andersen <lance.ander...@oracle.com> Sent: Samstag, 12. Januar 2019 15:01 To: Langer, Christoph <christoph.lan...@sap.com> Cc: Alan Bateman <alan.bate...@oracle.com>; nio-dev <nio-...@openjdk.java.net>; OpenJDK Dev list <security-dev@openjdk.java.net>; Java Core Libs <core-libs-...@openjdk.java.net> Subject: Re: RFR 8213031: (zipfs) Add support for POSIX file permissions Hi Christoph On Jan 12, 2019, at 8:02 AM, Langer, Christoph <christoph.lan...@sap.com<mailto: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<mailto:alan.bate...@oracle.com>>; Volker Simonis <volker.simo...@gmail.com<mailto:volker.simo...@gmail.com>> Cc: nio-dev <nio-...@openjdk.java.net<mailto:nio-...@openjdk.java.net>>; OpenJDK Dev list <security- d...@openjdk.java.net<mailto:d...@openjdk.java.net>>; Java Core Libs <core-libs-...@openjdk.java.net<mailto:core-libs-...@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<mailto:alan.bate...@oracle.com>> Sent: Montag, 7. Januar 2019 21:46 To: Volker Simonis <volker.simo...@gmail.com<mailto:volker.simo...@gmail.com>> Cc: Langer, Christoph <christoph.lan...@sap.com<mailto:christoph.lan...@sap.com>>; nio-dev <nio- d...@openjdk.java.net<mailto:d...@openjdk.java.net>>; OpenJDK Dev list <security- d...@openjdk.java.net<mailto:d...@openjdk.java.net>>; Java Core Libs <core-libs-...@openjdk.java.net<mailto:core-libs-...@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 [cid:image001.gif@01D4ABE5.198BA670]<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>
