8177969: Faster FilePermission::implies by avoiding the use of Path::relativize
Reviewed-by: rriggs, mullan
--- a/jdk/src/java.base/share/classes/java/io/FilePermission.java Mon Apr 10 13:51:40 2017 -0700
+++ b/jdk/src/java.base/share/classes/java/io/FilePermission.java Tue Apr 11 10:12:27 2017 +0800
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1997, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2017, 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
@@ -209,6 +209,10 @@
private static final Path here = builtInFS.getPath(
GetPropertyAction.privilegedGetProperty("user.dir"));
+ private static final Path EMPTY_PATH = builtInFS.getPath("");
+ private static final Path DASH_PATH = builtInFS.getPath("-");
+ private static final Path DOTDOT_PATH = builtInFS.getPath("..");
+
/**
* A private constructor that clones some and updates some,
* always with a different name.
@@ -341,7 +345,7 @@
.normalize();
// lastName should always be non-null now
Path lastName = npath.getFileName();
- if (lastName != null && lastName.toString().equals("-")) {
+ if (lastName != null && lastName.equals(DASH_PATH)) {
directory = true;
recursive = !rememberStar;
npath = npath.getParent();
@@ -679,23 +683,76 @@
* @return the depth in between
*/
private static int containsPath(Path p1, Path p2) {
- Path p;
- try {
- p = p2.relativize(p1).normalize();
- if (p.getName(0).toString().isEmpty()) {
- return 0;
- } else {
- for (Path item: p) {
- String s = item.toString();
- if (!s.equals("..")) {
- return -1;
- }
- }
- return p.getNameCount();
- }
- } catch (IllegalArgumentException iae) {
+
+ // Two paths must have the same root. For example,
+ // there is no contains relation between any two of
+ // "/x", "x", "C:/x", "C:x", and "//host/share/x".
+ if (!Objects.equals(p1.getRoot(), p2.getRoot())) {
return -1;
}
+
+ // Empty path (i.e. "." or "") is a strange beast,
+ // because its getNameCount()==1 but getName(0) is null.
+ // It's better to deal with it separately.
+ if (p1.equals(EMPTY_PATH)) {
+ if (p2.equals(EMPTY_PATH)) {
+ return 0;
+ } else if (p2.getName(0).equals(DOTDOT_PATH)) {
+ // "." contains p2 iif p2 has no "..". Since a
+ // a normalized path can only have 0 or more
+ // ".." at the beginning. We only need to look
+ // at the head.
+ return -1;
+ } else {
+ // and the distance is p2's name count. i.e.
+ // 3 between "." and "a/b/c".
+ return p2.getNameCount();
+ }
+ } else if (p2.equals(EMPTY_PATH)) {
+ int c1 = p1.getNameCount();
+ if (!p1.getName(c1 - 1).equals(DOTDOT_PATH)) {
+ // "." is inside p1 iif p1 is 1 or more "..".
+ // For the same reason above, we only need to
+ // look at the tail.
+ return -1;
+ }
+ // and the distance is the count of ".."
+ return c1;
+ }
+
+ // Good. No more empty paths.
+
+ // Common heads are removed
+
+ int c1 = p1.getNameCount();
+ int c2 = p2.getNameCount();
+
+ int n = Math.min(c1, c2);
+ int i = 0;
+ while (i < n) {
+ if (!p1.getName(i).equals(p2.getName(i)))
+ break;
+ i++;
+ }
+
+ // for p1 containing p2, p1 must be 0-or-more "..",
+ // and p2 cannot have "..". For the same reason, we only
+ // check tail of p1 and head of p2.
+ if (i < c1 && !p1.getName(c1 - 1).equals(DOTDOT_PATH)) {
+ return -1;
+ }
+
+ if (i < c2 && p2.getName(i).equals(DOTDOT_PATH)) {
+ return -1;
+ }
+
+ // and the distance is the name counts added (after removing
+ // the common heads).
+
+ // For example: p1 = "../../..", p2 = "../a".
+ // After removing the common heads, they become "../.." and "a",
+ // and the distance is (3-1)+(2-1) = 3.
+ return c1 - i + c2 - i;
}
/**
--- a/jdk/test/java/io/FilePermission/Correctness.java Mon Apr 10 13:51:40 2017 -0700
+++ b/jdk/test/java/io/FilePermission/Correctness.java Tue Apr 11 10:12:27 2017 +0800
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2017, 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
@@ -89,12 +89,14 @@
//check("/-", "-");
try {
- // containsPath is broken on Windows.
containsMethod = FilePermission.class.getDeclaredMethod(
"containsPath", Path.class, Path.class);
containsMethod.setAccessible(true);
System.out.println();
+ // The 1st 2 args of contains() must be normalized paths.
+ // When FilePermission::containsPath is called by implies,
+ // paths have already been normalized.
contains("x", "x", 0);
contains("x", "x/y", 1);
contains("x", "x/y/z", 2);
@@ -160,7 +162,7 @@
}
}
- static void check(String s1, String s2, boolean expected) {
+ static void check0(String s1, String s2, boolean expected) {
FilePermission fp1 = new FilePermission(s1, "read");
FilePermission fp2 = new FilePermission(s2, "read");
boolean b = fp1.implies(fp2);
@@ -173,6 +175,16 @@
}
}
+ static void check(String s1, String s2, boolean expected) {
+ check0(s1, s2, expected);
+ if (isWindows) {
+ check0("C:" + s1, s2, false);
+ check0(s1, "C:" + s2, false);
+ check0("C:" + s1, "D:" + s2, false);
+ check0("C:" + s1, "C:" + s2, expected);
+ }
+ }
+
static void check(String s1, String s2) {
check(s1, s2, true);
}