Skip to content

Commit 3862969

Browse files
committed
Refactoring: move path-as-string comparison functions to Comparison classes
1 parent bd7881a commit 3862969

File tree

9 files changed

+281
-185
lines changed

9 files changed

+281
-185
lines changed
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.entitlement.runtime.policy;
11+
12+
class CaseInsensitiveComparison extends FileAccessTreeComparison {
13+
14+
CaseInsensitiveComparison() {
15+
super(CaseInsensitiveComparison::caseInsensitiveCharacterComparator);
16+
}
17+
18+
/**
19+
* Case-insensitive character comparison. Taken from the JDK {@code WindowsPath#compareTo} implementation.
20+
*/
21+
private static int caseInsensitiveCharacterComparator(char c1, char c2) {
22+
if (c1 == c2) {
23+
return 0;
24+
}
25+
c1 = Character.toUpperCase(c1);
26+
c2 = Character.toUpperCase(c2);
27+
if (c1 == c2) {
28+
return 0;
29+
}
30+
return Character.compare(c1, c2);
31+
}
32+
33+
@Override
34+
protected boolean pathStartsWith(String pathString, String pathPrefix) {
35+
return pathString.regionMatches(true, 0, pathPrefix, 0, pathPrefix.length());
36+
}
37+
38+
@Override
39+
protected boolean pathsAreEqual(String path1, String path2) {
40+
return path1.equalsIgnoreCase(path2);
41+
}
42+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.entitlement.runtime.policy;
11+
12+
class CaseSensitiveComparison extends FileAccessTreeComparison {
13+
14+
CaseSensitiveComparison() {
15+
super(Character::compare);
16+
}
17+
18+
@Override
19+
protected boolean pathStartsWith(String pathString, String pathPrefix) {
20+
return pathString.startsWith(pathPrefix);
21+
}
22+
23+
@Override
24+
protected boolean pathsAreEqual(String path1, String path2) {
25+
return path1.equals(path2);
26+
}
27+
}

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

Lines changed: 40 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333

3434
import static java.util.Comparator.comparing;
3535
import static org.elasticsearch.core.PathUtils.getDefaultFileSystem;
36-
import static org.elasticsearch.entitlement.runtime.policy.FileUtils.PATH_ORDER;
3736
import static org.elasticsearch.entitlement.runtime.policy.PathLookup.BaseDir.CONFIG;
3837
import static org.elasticsearch.entitlement.runtime.policy.PathLookup.BaseDir.TEMP;
3938
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ_WRITE;
@@ -70,9 +69,9 @@
7069
* Secondly, the path separator (whether slash or backslash) sorts after the dot character. This means, for example, if the array contains
7170
* {@code ["/a", "/a.xml"]} and we look up {@code "/a/b"} using normal string comparison, the binary search would land on {@code "/a.xml"},
7271
* which is neither an exact match nor a containing directory, and so the lookup would incorrectly report no match, even though
73-
* {@code "/a"} is in the array. To fix this, we define {@link FileUtils#PATH_ORDER} which sorts path separators before any other
74-
* character. In the example, this would cause {@code "/a/b"} to sort between {@code "/a"} and {@code "/a.xml"} so that it correctly
75-
* finds {@code "/a"}.
72+
* {@code "/a"} is in the array. To fix this, we define {@link FileAccessTreeComparison#pathComparator()} which sorts path separators
73+
* before any other character. In the example, this would cause {@code "/a/b"} to sort between {@code "/a"} and {@code "/a.xml"} so that
74+
* it correctly finds {@code "/a"}.
7675
* </p>
7776
* With the paths pruned, sorted, and segregated by permission, each binary search has the following properties:
7877
* <ul>
@@ -118,7 +117,11 @@ public String toString() {
118117
}
119118
}
120119

121-
static List<ExclusivePath> buildExclusivePathList(List<ExclusiveFileEntitlement> exclusiveFileEntitlements, PathLookup pathLookup) {
120+
static List<ExclusivePath> buildExclusivePathList(
121+
List<ExclusiveFileEntitlement> exclusiveFileEntitlements,
122+
PathLookup pathLookup,
123+
FileAccessTreeComparison comparison
124+
) {
122125
Map<String, ExclusivePath> exclusivePaths = new HashMap<>();
123126
for (ExclusiveFileEntitlement efe : exclusiveFileEntitlements) {
124127
for (FilesEntitlement.FileData fd : efe.filesEntitlement().filesData()) {
@@ -150,16 +153,16 @@ static List<ExclusivePath> buildExclusivePathList(List<ExclusiveFileEntitlement>
150153
}
151154
}
152155
}
153-
return exclusivePaths.values().stream().sorted(comparing(ExclusivePath::path, PATH_ORDER)).distinct().toList();
156+
return exclusivePaths.values().stream().sorted(comparing(ExclusivePath::path, comparison.pathComparator())).distinct().toList();
154157
}
155158

156-
static void validateExclusivePaths(List<ExclusivePath> exclusivePaths) {
159+
static void validateExclusivePaths(List<ExclusivePath> exclusivePaths, FileAccessTreeComparison comparison) {
157160
if (exclusivePaths.isEmpty() == false) {
158161
ExclusivePath currentExclusivePath = exclusivePaths.get(0);
159162
for (int i = 1; i < exclusivePaths.size(); ++i) {
160163
ExclusivePath nextPath = exclusivePaths.get(i);
161-
if (FileUtils.samePath(currentExclusivePath.path(), nextPath.path)
162-
|| FileUtils.isParent(currentExclusivePath.path(), nextPath.path())) {
164+
if (comparison.samePath(currentExclusivePath.path(), nextPath.path)
165+
|| comparison.isParent(currentExclusivePath.path(), nextPath.path())) {
163166
throw new IllegalArgumentException(
164167
"duplicate/overlapping exclusive paths found in files entitlements: " + currentExclusivePath + " and " + nextPath
165168
);
@@ -171,7 +174,11 @@ static void validateExclusivePaths(List<ExclusivePath> exclusivePaths) {
171174

172175
private static final Logger logger = LogManager.getLogger(FileAccessTree.class);
173176
private static final String FILE_SEPARATOR = getDefaultFileSystem().getSeparator();
177+
static final FileAccessTreeComparison DEFAULT_COMPARISON = Platform.LINUX.isCurrent()
178+
? new CaseSensitiveComparison()
179+
: new CaseInsensitiveComparison();
174180

181+
private final FileAccessTreeComparison comparison;
175182
/**
176183
* lists paths that are forbidden for this component+module because some other component has granted exclusive access to one of its
177184
* modules
@@ -189,19 +196,27 @@ static void validateExclusivePaths(List<ExclusivePath> exclusivePaths) {
189196
private static String[] buildUpdatedAndSortedExclusivePaths(
190197
String componentName,
191198
String moduleName,
192-
List<ExclusivePath> exclusivePaths
199+
List<ExclusivePath> exclusivePaths,
200+
FileAccessTreeComparison comparison
193201
) {
194202
List<String> updatedExclusivePaths = new ArrayList<>();
195203
for (ExclusivePath exclusivePath : exclusivePaths) {
196204
if (exclusivePath.componentName().equals(componentName) == false || exclusivePath.moduleNames().contains(moduleName) == false) {
197205
updatedExclusivePaths.add(exclusivePath.path());
198206
}
199207
}
200-
updatedExclusivePaths.sort(PATH_ORDER);
208+
updatedExclusivePaths.sort(comparison.pathComparator());
201209
return updatedExclusivePaths.toArray(new String[0]);
202210
}
203211

204-
private FileAccessTree(FilesEntitlement filesEntitlement, PathLookup pathLookup, Path componentPath, String[] sortedExclusivePaths) {
212+
FileAccessTree(
213+
FilesEntitlement filesEntitlement,
214+
PathLookup pathLookup,
215+
Path componentPath,
216+
String[] sortedExclusivePaths,
217+
FileAccessTreeComparison comparison
218+
) {
219+
this.comparison = comparison;
205220
List<String> readPaths = new ArrayList<>();
206221
List<String> writePaths = new ArrayList<>();
207222
BiConsumer<Path, Mode> addPath = (path, mode) -> {
@@ -253,23 +268,23 @@ private FileAccessTree(FilesEntitlement filesEntitlement, PathLookup pathLookup,
253268
Path jdk = Paths.get(System.getProperty("java.home"));
254269
addPathAndMaybeLink.accept(jdk.resolve("conf"), Mode.READ);
255270

256-
readPaths.sort(PATH_ORDER);
257-
writePaths.sort(PATH_ORDER);
271+
readPaths.sort(comparison.pathComparator());
272+
writePaths.sort(comparison.pathComparator());
258273

259274
this.exclusivePaths = sortedExclusivePaths;
260-
this.readPaths = pruneSortedPaths(readPaths).toArray(new String[0]);
261-
this.writePaths = pruneSortedPaths(writePaths).toArray(new String[0]);
275+
this.readPaths = pruneSortedPaths(readPaths, comparison).toArray(new String[0]);
276+
this.writePaths = pruneSortedPaths(writePaths, comparison).toArray(new String[0]);
262277
}
263278

264279
// package private for testing
265-
static List<String> pruneSortedPaths(List<String> paths) {
280+
static List<String> pruneSortedPaths(List<String> paths, FileAccessTreeComparison comparison) {
266281
List<String> prunedReadPaths = new ArrayList<>();
267282
if (paths.isEmpty() == false) {
268283
String currentPath = paths.get(0);
269284
prunedReadPaths.add(currentPath);
270285
for (int i = 1; i < paths.size(); ++i) {
271286
String nextPath = paths.get(i);
272-
if (FileUtils.samePath(currentPath, nextPath) == false && FileUtils.isParent(currentPath, nextPath) == false) {
287+
if (comparison.samePath(currentPath, nextPath) == false && comparison.isParent(currentPath, nextPath) == false) {
273288
prunedReadPaths.add(nextPath);
274289
currentPath = nextPath;
275290
}
@@ -290,7 +305,8 @@ static FileAccessTree of(
290305
filesEntitlement,
291306
pathLookup,
292307
componentPath,
293-
buildUpdatedAndSortedExclusivePaths(componentName, moduleName, exclusivePaths)
308+
buildUpdatedAndSortedExclusivePaths(componentName, moduleName, exclusivePaths, DEFAULT_COMPARISON),
309+
DEFAULT_COMPARISON
294310
);
295311
}
296312

@@ -302,7 +318,7 @@ public static FileAccessTree withoutExclusivePaths(
302318
PathLookup pathLookup,
303319
@Nullable Path componentPath
304320
) {
305-
return new FileAccessTree(filesEntitlement, pathLookup, componentPath, new String[0]);
321+
return new FileAccessTree(filesEntitlement, pathLookup, componentPath, new String[0], DEFAULT_COMPARISON);
306322
}
307323

308324
public boolean canRead(Path path) {
@@ -333,14 +349,14 @@ private boolean checkPath(String path, String[] paths) {
333349
return false;
334350
}
335351

336-
int endx = Arrays.binarySearch(exclusivePaths, path, PATH_ORDER);
337-
if (endx < -1 && FileUtils.isParent(exclusivePaths[-endx - 2], path) || endx >= 0) {
352+
int endx = Arrays.binarySearch(exclusivePaths, path, comparison.pathComparator());
353+
if (endx < -1 && comparison.isParent(exclusivePaths[-endx - 2], path) || endx >= 0) {
338354
return false;
339355
}
340356

341-
int ndx = Arrays.binarySearch(paths, path, PATH_ORDER);
357+
int ndx = Arrays.binarySearch(paths, path, comparison.pathComparator());
342358
if (ndx < -1) {
343-
return FileUtils.isParent(paths[-ndx - 2], path);
359+
return comparison.isParent(paths[-ndx - 2], path);
344360
}
345361
return ndx >= 0;
346362
}
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.entitlement.runtime.policy;
11+
12+
import org.elasticsearch.core.Strings;
13+
import org.elasticsearch.core.SuppressForbidden;
14+
import org.elasticsearch.logging.LogManager;
15+
import org.elasticsearch.logging.Logger;
16+
17+
import java.io.File;
18+
import java.util.Comparator;
19+
20+
abstract class FileAccessTreeComparison {
21+
22+
private static final Logger logger = LogManager.getLogger(FileAccessTreeComparison.class);
23+
24+
interface CharComparator {
25+
int compare(char c1, char c2);
26+
}
27+
28+
private final Comparator<String> pathComparator;
29+
30+
/**
31+
* For our lexicographic sort trick to work correctly, we must have path separators sort before
32+
* any other character so that files in a directory appear immediately after that directory.
33+
* For example, we require [/a, /a/b, /a.xml] rather than the natural order [/a, /a.xml, /a/b].
34+
*/
35+
FileAccessTreeComparison(CharComparator charComparator) {
36+
pathComparator = (s1, s2) -> {
37+
int len1 = s1.length();
38+
int len2 = s2.length();
39+
int lim = Math.min(len1, len2);
40+
for (int k = 0; k < lim; k++) {
41+
char c1 = s1.charAt(k);
42+
char c2 = s2.charAt(k);
43+
var comp = charComparator.compare(c1, c2);
44+
if (comp == 0) {
45+
continue;
46+
}
47+
boolean c1IsSeparator = isPathSeparator(c1);
48+
boolean c2IsSeparator = isPathSeparator(c2);
49+
if (c1IsSeparator == false || c2IsSeparator == false) {
50+
if (c1IsSeparator) {
51+
return -1;
52+
}
53+
if (c2IsSeparator) {
54+
return 1;
55+
}
56+
return comp;
57+
}
58+
}
59+
return len1 - len2;
60+
};
61+
}
62+
63+
Comparator<String> pathComparator() {
64+
return pathComparator;
65+
}
66+
67+
@SuppressForbidden(reason = "we need the separator as a char, not a string")
68+
boolean isParent(String maybeParent, String path) {
69+
logger.trace(() -> Strings.format("checking isParent [%s] for [%s]", maybeParent, path));
70+
return pathStartsWith(path, maybeParent)
71+
&& (path.length() > maybeParent.length() && path.charAt(maybeParent.length()) == File.separatorChar);
72+
}
73+
74+
boolean samePath(String currentPath, String nextPath) {
75+
return pathsAreEqual(currentPath, nextPath);
76+
}
77+
78+
protected abstract boolean pathStartsWith(String pathString, String pathPrefix);
79+
80+
protected abstract boolean pathsAreEqual(String path1, String path2);
81+
82+
@SuppressForbidden(reason = "we need the separator as a char, not a string")
83+
private static boolean isPathSeparator(char c) {
84+
return c == File.separatorChar;
85+
}
86+
}

0 commit comments

Comments
 (0)