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

[Langer, Christoph]

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.andersen at oracle.com>
Sent: Samstag, 12. Januar 2019 15:01
To: Langer, Christoph <christoph.langer at sap.com>
Cc: Alan Bateman <Alan.Bateman at oracle.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

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

[cid:image001.gif at 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.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/20190114/80acace4/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.gif
Type: image/gif
Size: 658 bytes
Desc: image001.gif
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20190114/80acace4/image001.gif>