8170364: FilePermission path modified during merge

Reviewed-by: alanb

jdk/src/java.base/share/classes/java/io/FilePermission.java

@@ -216,19 +216,28 @@ public final class FilePermission extends Permission implements Serializable {
             GetPropertyAction.privilegedGetProperty("user.dir"));
 
     /**
-     * A private constructor like a clone, only npath2 is not touched.
+     * A private constructor that clones some and updates some,
+     * always with a different name.
      * @param input
      */
-    private FilePermission(String name, FilePermission input) {
+    private FilePermission(String name,
+                           FilePermission input,
+                           Path npath,
+                           Path npath2,
+                           int mask,
+                           String actions) {
         super(name);
-        this.npath = input.npath;
+        // Customizables
-        this.actions = input.actions;
+        this.npath = npath;
+        this.npath2 = npath2;
+        this.actions = actions;
+        this.mask = mask;
+        // Cloneds
         this.allFiles = input.allFiles;
         this.invalid = input.invalid;
         this.recursive = input.recursive;
         this.directory = input.directory;
         this.cpath = input.cpath;
-        this.mask = input.mask;
     }
 
     /**
@@ -261,10 +270,12 @@ public final class FilePermission extends Permission implements Serializable {
                             // different than the original so that when one is
                             // added to a FilePermissionCollection it will not
                             // be merged with the original one.
-                            FilePermission np = new FilePermission(
+                            return new FilePermission(input.getName() + "#plus",
-                                    input.getName()+"#plus", input);
+                                    input,
-                            np.npath2 = npath2;
+                                    input.npath,
-                            return np;
+                                    npath2,
+                                    input.mask,
+                                    input.actions);
                         }
                     }
                     return input;
@@ -274,10 +285,12 @@ public final class FilePermission extends Permission implements Serializable {
                         Path npath2 = altPath(input.npath);
                         if (npath2 != null) {
                             // New name, see above.
-                            FilePermission np = new FilePermission(
+                            return new FilePermission(input.getName() + "#using",
-                                    input.getName()+"#using", input);
+                                    input,
-                            np.npath = npath2;
+                                    npath2,
-                            return np;
+                                    null,
+                                    input.mask,
+                                    input.actions);
                         }
                     }
                     return null;
@@ -981,6 +994,20 @@ public final class FilePermission extends Permission implements Serializable {
         s.defaultReadObject();
         init(getMask(actions));
     }
+
+    /**
+     * Create a cloned FilePermission with a different actions.
+     * @param effective the new actions
+     * @return a new object
+     */
+    FilePermission withNewActions(int effective) {
+        return new FilePermission(this.getName(),
+                this,
+                this.npath,
+                this.npath2,
+                effective,
+                null);
+    }
 }
 
 /**
@@ -1051,10 +1078,7 @@ final class FilePermissionCollection extends PermissionCollection
         // permission if applicable. NOTE: cannot use lambda for
         // remappingFunction parameter until JDK-8076596 is fixed.
         perms.merge(fp.getName(), fp,
-            new java.util.function.BiFunction<>() {
+                (existingVal, newVal) -> {
-                @Override
-                public Permission apply(Permission existingVal,
-                                        Permission newVal) {
                     int oldMask = ((FilePermission)existingVal).getMask();
                     int newMask = ((FilePermission)newVal).getMask();
                     if (oldMask != newMask) {
@@ -1063,12 +1087,12 @@ final class FilePermissionCollection extends PermissionCollection
                             return newVal;
                         }
                         if (effective != oldMask) {
-                            return new FilePermission(fp.getName(), effective);
+                            return ((FilePermission)newVal)
+                                    .withNewActions(effective);
                         }
                     }
                     return existingVal;
                 }
-            }
         );
     }
 

jdk/test/java/io/FilePermission/MergeName.java

@@ -0,0 +1,84 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import jdk.test.lib.process.ProcessTools;
+
+import java.io.File;
+import java.io.FilePermission;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+/**
+ * @test
+ * @bug 8170364
+ * @summary FilePermission path modified during merge
+ * @library /test/lib
+ */
+
+public class MergeName {
+
+    public static final String[] ALL_ACTIONS
+            = {"read", "write", "execute", "delete"};
+
+    public static void main(String[] args) throws Exception {
+        if (args.length == 0) {
+            test("p1", "read", "write", "delete", "execute");
+            test("p2", "read,write", "delete,execute");
+            test("p3", "read,write,delete", "execute");
+            test("p4", "read,write,delete,execute");
+        } else {
+            SecurityManager sm = System.getSecurityManager();
+            for (String arg : args) {
+                // Use bits to create powerset of ALL_ACTIONS
+                IntStream.range(1, 16)
+                        .mapToObj(n -> IntStream.range(0, 4)
+                                .filter(x -> (n & (1 << x)) != 0)
+                                .mapToObj(x -> ALL_ACTIONS[x])
+                                .collect(Collectors.joining(",")))
+                        .forEach(a -> sm.checkPermission(
+                                new FilePermission(arg, a)));
+            }
+        }
+    }
+
+    private static void test(String file, String... actions) throws Exception {
+        List<String> content = new ArrayList<>();
+        content.add("grant {");
+        for (String action : actions) {
+            content.add("   permission java.io.FilePermission " +
+                    "\"x\", \"" +action + "\";");
+        }
+        content.add("};");
+        Files.write(Paths.get(file), content);
+        ProcessTools.executeTestJvm("-Djava.security.manager",
+                "-Djava.security.policy=" + file,
+                "MergeName",
+                "x",
+                new File(System.getProperty("user.dir"), "x").getPath())
+            .shouldHaveExitValue(0);
+    }
+}