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

Langer, Christoph christoph.langer at sap.com
Thu May 23 07:41:17 UTC 2019


Hi,

here is a little update to my latest webrev: http://cr.openjdk.java.net/~clanger/webrevs/8213031.11/

I had to make modifications to fix test errors and enhance testing:
a) in initOwner/initGroup the UOE has to be catched and handled explicitly as it is never wrapped into PAE because it is a RuntimeException.
b) updated the test to expect the zip file's group as default group
c) added a sanity check to read and extract a zip file created by zipfs with posix permissions set via java.util.zip.ZipFile

Best regards
Christoph


> -----Original Message-----
> From: Langer, Christoph
> Sent: Dienstag, 21. Mai 2019 17:24
> To: Alan Bateman <Alan.Bateman at oracle.com>
> Cc: nio-dev <nio-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,
> 
> Thank you for your comments. Here comes the next update... increasing the
> turnaround time a little bit ��
> 
> http://cr.openjdk.java.net/~clanger/webrevs/8213031.10/
> 
> 
> > Thanks. I think you've addressed most of my points. The only thing that
> > isn't clear is the group owner as I thought we had converged on using
> > the zip file's group owner.  If I read the code correctly then it uses
> > the file owner (and makes the assumption that defaultOwner is
> > initialized before initGroup is called).
> 
> Ok, makes sense. I've updated the coding such that the zip's file owner
> would be the default owner, in case it can be retrieved.
> 
> > In passing, the name of the PosixFileAttributeView implementation should
> > probably be ZipPosixFileAttributeView rather than
> > ZipFilePosixAttributeViewPosix. Also EntryPosix extends Entry, should
> > probably be PosixEntry. Not important as these are internal but noticed
> > them when scanning the changes.
> 
> I changed the class names, hopefully to your liking.
> 
> > Also in passing, there are several places using
> > AccessController.doPrivileged that are bit ugly due to the casting. The
> > doPrivileged methods are awkward to use with lambda expressions (not
> > your doing) but one approach that I find readable is to separate the
> > creation of the action, e.g. file the zip file owner it could be:
> >
> > PrivilegedExceptionAction<UserPrincipal> pa = () ->
> Files.getOwner(zfpath);
> > return AccessController.doPrivileged(pa).
> 
> I updated these places. Seems more readable indeed.
> 
> > In the same area, the code in initOwner catches UOE but that will always
> > be wrapped in PAE.
> 
> Fixed.
> 
> > > I have updated module-info a little bit to reflect the latest changes. Is it
> > now time to focus on the CSR?
> > >
> > The "For extended POSIX support ..." paragraph has the property name
> > from a previous iteration so I assume you'll update that. I think it
> > would be using to put quotes around the names too. It also specifies the
> > fallback group owner to be the file owner but I think we converged on
> > use the zip file's group owner where possible.
> >
> > A small bit of word smiting required but I think this is really close to
> > writing the CSR.
> 
> I have updated the doc in module-info.java quite a bit. Please check.
> 
> Is it time to work on the CSR now? How shall we proceed there?
> 
> Thanks
> Christoph



More information about the nio-dev mailing list