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

Sun Jun 2 21:35:33 UTC 2019

Hi Alan, Lance,

thanks for the updated wording in module-info, that really looks good. I incorporated it into my change, no we'd be here:
http://cr.openjdk.java.net/~clanger/webrevs/8213031.13/

To be honest, I was hoping it would still make it into JDK13.

I guess now I shall update the CSR to get it reviewed, correct?

Thanks
Christoph

From: Lance Andersen <lance.andersen at oracle.com>
Sent: Freitag, 31. Mai 2019 20:01
To: Alan Bateman <Alan.Bateman at oracle.com>
Cc: Langer, Christoph <christoph.langer at sap.com>; 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

On May 31, 2019, at 12:32 PM, Alan Bateman <Alan.Bateman at oracle.com<mailto:Alan.Bateman at oracle.com>> wrote:

On 29/05/2019 13:16, Langer, Christoph wrote:

Hi Alan,

The table items in L119-150 look fine, we just need to avoid really long
lines One minor comment on L123 is that it might be clearer if you drop
"created" from the sentence.

L48-78 is a "wall of text" and links that I don't think will be easy for
most developers to read. Can I provide suggested wording for this part
of the spec? I'm just thinking that an alternative wording might help
avoid too much iteration on this.
I have created a new webrev to add some linebreaks and pick up your suggestion to drop the word "created" in L123.

http://cr.openjdk.java.net/~clanger/webrevs/8213031.12/

Waiting on your update for the other part.
Attached is alternative wording for the "Support for POSIX file permissions" section. My concern with the proposed in webrev.12 is that it's dense and not easy to read. It also misses a few things - one important one is that access permissions aren't enforced.

I think the wording below is looking good.  A few minor suggestions below.

So overall I think you've got this feature into reasonable shape (I realize it has taken 7 months to get here, this is perhaps a good example of something that needs a lot of up front discussion before going near the code).

Once we finalize the CSR, we should look towards getting the changes in early in the JDK 14 cycle so that we have time to vet/catch potential issues.

-Alan

import java.nio.file.Files;
import java.nio.file.LinkOption;
import java.nio.file.Path;
import java.nio.file.attribute.FileAttributeView;
import java.nio.file.attribute.PosixFileAttributes;
import java.nio.file.attribute.PosixFilePermission;
import java.nio.file.attribute.PosixFileAttributeView;
import java.util.Set;

 * <h2> POSIX file attributes </h2>
 *
 * <p> A Zip file system supports a file attribute {@link FileAttributeView view}
 * named "{@code zip}" that defines the following file attribute:
 *
 * <blockquote>
 * <table class="striped">
 * <caption style="display:none">Supported attributes</caption>
 * <thead>
 *   <tr>
 *     <th scope="col"> Name </th>
 *     <th scope="col"> Type </th>
 *   </tr>
 * </thead>
 * <tbody>
 *  <tr>
 *     <th scope="row"> permissions </th>
 *     <td> {@link Set}<{@link PosixFilePermission}> </td>
 *   </tr>
 * </tbody>
 * </table>
 * </blockquote>
 *
 * The "permissions" attribute is the set of access permissions that are optionally
 * stored for entries in a Zip file. The value of the attribute is {@code null}
 * for entries that do not have access permissions. Zip file systems do not
 * enforce access permissions.
 *
 * <p> The "permissions" attribute can be read and set using the
can-> may

 * {@linkplain Files#getAttribute(Path, String, LinkOption...) Files.getAttribute} and
 * {@linkplain Files#setAttribute(Path, String, Object, LinkOption...) Files.setAttribute}
 * methods. The following example uses these methods to read and set the attribute:
 * <pre> {@code
 *     Set<PosixFilePermission> perms = Files.getAttribute(entry, "zip:permissions");
 *     if (perms == null) {
 *         perms = PosixFilePermissions.fromString("rw-rw-rw-");
 *         Files.setAttribute(entry, "zip:permissions", perms);
 *     }
 * } </pre>
 *
 * <p> In addition to the "{@code zip}" view, a Zip file system optionally supports
 * the {@link PosixFileAttributeView POSIX file attribute view} ("{@code posix}").
 * This view extends the "{@code basic}" view with type safe access to the
 * {@link PosixFileAttributes#owner() owner}, {@link PosixFileAttributes#group() group-owner},
 * and {@link PosixFileAttributes#permissions() permissions} attributes. The
 * "{@code posix}" view is only supported when the Zip file system is created with
 * the provider property "{@code enablePosixFileAttributes}" set to "{@code true}".
 * The following creates a file system with this property and reads the access
 * permissions of a file:
 * <pre> {@code
 *     var env = Map.of("enablePosixFileAttributes", "true");
 *     try (FileSystem fs = FileSystems.newFileSystem(file, env) {
 *         Path entry = fs.getPath("entry");
 *         Set<PosixFilePermission> perms = Files.getPosixFilePermissions(entry);
 *     }
 * } </pre>
 *
 * <p> The file owner and group owner attributes are not persisted, meaning they are
 * not stored in the zip file. The "{@code defaultOwner}" and "{@code defaultGroup}"
 * provider properties (listed below) can be used to configure the default values
 * for these attributes. If these properties are not set then the file owner
 * defaults to the owner of the zip file, and the group owner defaults to the
 * zip file's group owner (or the file owner on platforms that don't support a
 * group owner).
 *
 * <p> The "{@code permissions}" attribute is not optional in the "{@code posix}"
 * view so a default of set of permissions are used for entries that do not have
                                 ^^^
                           of seems out of place

 * access permissions stored in the Zip file. The default set of permissions
 * is {@link PosixFilePermission#OWNER_READ OWNER_READ}, {@link PosixFilePermission#OWNER_WRITE
is -> are
 * OWNER_WRITE} and {@link PosixFilePermission#GROUP_READ GROUP_READ}.
perhaps consider using a <UL>  when listing the permissions?

The default
 * permissions can be configured with the "{@code defaultPermissions}" property
 * described below.

Best
Lance
[cid:image001.gif at 01D5199B.DED73A10]<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>