Skip to content

Commit b0ed006

Browse files
committed
Custom comparator for paths in FileAccessTree
1 parent b2bd8d2 commit b0ed006

File tree

2 files changed

+45
-6
lines changed

2 files changed

+45
-6
lines changed

libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import java.nio.file.Path;
1515
import java.util.ArrayList;
1616
import java.util.Arrays;
17+
import java.util.Comparator;
1718
import java.util.List;
1819
import java.util.Objects;
1920

@@ -46,8 +47,8 @@ private FileAccessTree(FilesEntitlement filesEntitlement, PathLookup pathLookup)
4647
readPaths.add(tempDir);
4748
writePaths.add(tempDir);
4849

49-
readPaths.sort(String::compareTo);
50-
writePaths.sort(String::compareTo);
50+
readPaths.sort(PATH_ORDER);
51+
writePaths.sort(PATH_ORDER);
5152

5253
this.readPaths = pruneSortedPaths(readPaths).toArray(new String[0]);
5354
this.writePaths = pruneSortedPaths(writePaths).toArray(new String[0]);
@@ -60,7 +61,7 @@ private static List<String> pruneSortedPaths(List<String> paths) {
6061
prunedReadPaths.add(currentPath);
6162
for (int i = 1; i < paths.size(); ++i) {
6263
String nextPath = paths.get(i);
63-
if (nextPath.startsWith(currentPath) == false) {
64+
if (isParent(currentPath, nextPath) == false) {
6465
prunedReadPaths.add(nextPath);
6566
currentPath = nextPath;
6667
}
@@ -95,14 +96,17 @@ private static boolean checkPath(String path, String[] paths) {
9596
if (paths.length == 0) {
9697
return false;
9798
}
98-
int ndx = Arrays.binarySearch(paths, path);
99+
int ndx = Arrays.binarySearch(paths, path, PATH_ORDER);
99100
if (ndx < -1) {
100-
String maybeParent = paths[-ndx - 2];
101-
return path.startsWith(maybeParent) && path.startsWith(FILE_SEPARATOR, maybeParent.length());
101+
return isParent(paths[-ndx - 2], path);
102102
}
103103
return ndx >= 0;
104104
}
105105

106+
private static boolean isParent(String maybeParent, String path) {
107+
return path.startsWith(maybeParent) && path.startsWith(FILE_SEPARATOR, maybeParent.length());
108+
}
109+
106110
@Override
107111
public boolean equals(Object o) {
108112
if (o == null || getClass() != o.getClass()) return false;
@@ -114,4 +118,30 @@ public boolean equals(Object o) {
114118
public int hashCode() {
115119
return Objects.hash(Arrays.hashCode(readPaths), Arrays.hashCode(writePaths));
116120
}
121+
122+
/**
123+
* For our lexicographic sort trick to work correctly, we must have path separators sort before
124+
* any other character so that files in a directory appear immediately after that directory.
125+
* For example, we require [/a, /a/b, /a.xml] rather than the natural order [/a, /a.xml, /a/b].
126+
*/
127+
private static final Comparator<String> PATH_ORDER = (s1, s2) -> {
128+
Path p1 = Path.of(s1);
129+
Path p2 = Path.of(s2);
130+
var i1 = p1.iterator();
131+
var i2 = p2.iterator();
132+
while (i1.hasNext() && i2.hasNext()) {
133+
int cmp = i1.next().compareTo(i2.next());
134+
if (cmp != 0) {
135+
return cmp;
136+
}
137+
}
138+
if (i1.hasNext()) {
139+
return 1;
140+
} else if (i2.hasNext()) {
141+
return -1;
142+
} else {
143+
assert p1.equals(p2);
144+
return 0;
145+
}
146+
};
117147
}

libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,15 @@ public void testPrunedPaths() {
117117
assertThat(tree.canWrite(path("foo/baz")), is(false));
118118
}
119119

120+
public void testPathAndFileWithSamePrefix() {
121+
var tree = accessTree(entitlement("foo/bar/", "read", "foo/bar.xml", "read"));
122+
assertThat(tree.canRead(path("foo")), is(false));
123+
assertThat(tree.canRead(path("foo/bar")), is(true));
124+
assertThat(tree.canRead(path("foo/bar/baz")), is(true));
125+
assertThat(tree.canRead(path("foo/bar.xml")), is(true));
126+
assertThat(tree.canRead(path("foo/bar.txt")), is(false));
127+
}
128+
120129
public void testReadWithRelativePath() {
121130
for (var dir : List.of("config", "home")) {
122131
var tree = accessTree(entitlement(Map.of("relative_path", "foo", "mode", "read", "relative_to", dir)));

0 commit comments

Comments
 (0)