Skip to content

[Entitlements] Fix: consider case sensitiveness differences #126990

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
fb7b6a7
fix: consider case sensitiveness differences
ldematte Apr 17, 2025
89701ef
Update docs/changelog/126990.yaml
ldematte Apr 17, 2025
4c6cc3f
[CI] Auto commit changes from spotless
elasticsearchmachine Apr 17, 2025
d844993
changelog + spotless
ldematte Apr 17, 2025
761b8a3
Merge branch 'entitlements/fileaccesstree-fix-windows-casing' of http…
ldematte Apr 17, 2025
1b0293c
forbidden
ldematte Apr 17, 2025
c7ab3b9
Merge branch 'main' into entitlements/fileaccesstree-fix-windows-casing
ldematte Apr 17, 2025
e199568
Update docs/changelog/126990.yaml
ldematte Apr 18, 2025
6f2eab2
Merge remote-tracking branch 'upstream/main' into entitlements/fileac…
ldematte Apr 18, 2025
3894d03
consider mac case insensitive too
ldematte Apr 18, 2025
ea10eb5
Merge branch 'main' into entitlements/fileaccesstree-fix-windows-casing
ldematte Apr 18, 2025
bd7881a
Merge remote-tracking branch 'upstream/main' into entitlements/fileac…
ldematte Apr 22, 2025
3862969
Refactoring: move path-as-string comparison functions to Comparison c…
ldematte Apr 22, 2025
b252d3d
Merge branch 'main' into entitlements/fileaccesstree-fix-windows-casing
ldematte Apr 22, 2025
38957a2
extract separator
ldematte Apr 22, 2025
65f8bd8
Merge branch 'entitlements/fileaccesstree-fix-windows-casing' of gith…
ldematte Apr 22, 2025
5d3db2e
[CI] Auto commit changes from spotless
elasticsearchmachine Apr 22, 2025
cfc7299
fix separator for windows tests
ldematte Apr 22, 2025
cfaf7e4
iter
ldematte Apr 22, 2025
2438af9
[CI] Auto commit changes from spotless
elasticsearchmachine Apr 22, 2025
61b1219
Merge remote-tracking branch 'upstream/main' into entitlements/fileac…
ldematte Apr 23, 2025
1d15bfb
iter
ldematte Apr 23, 2025
fe22895
iter
ldematte Apr 23, 2025
0db687c
Merge branch 'main' into entitlements/fileaccesstree-fix-windows-casing
ldematte Apr 23, 2025
a1127fd
Merge branch 'main' into entitlements/fileaccesstree-fix-windows-casing
ldematte Apr 23, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
extract separator
  • Loading branch information
ldematte committed Apr 22, 2025
commit 38957a27dbce8c716698f244fea57e38f63ca6db
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@

class CaseInsensitiveComparison extends FileAccessTreeComparison {

CaseInsensitiveComparison() {
super(CaseInsensitiveComparison::caseInsensitiveCharacterComparator);
CaseInsensitiveComparison(char separatorChar) {
super(CaseInsensitiveComparison::caseInsensitiveCharacterComparator, separatorChar);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@

class CaseSensitiveComparison extends FileAccessTreeComparison {

CaseSensitiveComparison() {
super(Character::compare);
CaseSensitiveComparison(char separatorChar) {
super(Character::compare, separatorChar);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@

import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.Strings;
import org.elasticsearch.core.SuppressForbidden;
import org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement;
import org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode;
import org.elasticsearch.logging.LogManager;
import org.elasticsearch.logging.Logger;

import java.io.File;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.file.Files;
Expand Down Expand Up @@ -172,11 +174,16 @@ static void validateExclusivePaths(List<ExclusivePath> exclusivePaths, FileAcces
}
}

@SuppressForbidden(reason = "we need the separator as a char, not a string")
private static char separatorChar() {
return File.separatorChar;
}

private static final Logger logger = LogManager.getLogger(FileAccessTree.class);
private static final String FILE_SEPARATOR = getDefaultFileSystem().getSeparator();
static final FileAccessTreeComparison DEFAULT_COMPARISON = Platform.LINUX.isCurrent()
? new CaseSensitiveComparison()
: new CaseInsensitiveComparison();
? new CaseSensitiveComparison(separatorChar())
: new CaseInsensitiveComparison(separatorChar());

private final FileAccessTreeComparison comparison;
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,9 @@
package org.elasticsearch.entitlement.runtime.policy;

import org.elasticsearch.core.Strings;
import org.elasticsearch.core.SuppressForbidden;
import org.elasticsearch.logging.LogManager;
import org.elasticsearch.logging.Logger;

import java.io.File;
import java.util.Comparator;

abstract class FileAccessTreeComparison {
Expand All @@ -27,12 +25,14 @@ interface CharComparator {

private final Comparator<String> pathComparator;

private final char separatorChar;

/**
* For our lexicographic sort trick to work correctly, we must have path separators sort before
* any other character so that files in a directory appear immediately after that directory.
* For example, we require [/a, /a/b, /a.xml] rather than the natural order [/a, /a.xml, /a/b].
*/
FileAccessTreeComparison(CharComparator charComparator) {
FileAccessTreeComparison(CharComparator charComparator, char separatorChar) {
pathComparator = (s1, s2) -> {
int len1 = s1.length();
int len2 = s2.length();
Expand All @@ -44,8 +44,8 @@ interface CharComparator {
if (comp == 0) {
continue;
}
boolean c1IsSeparator = isPathSeparator(c1);
boolean c2IsSeparator = isPathSeparator(c2);
boolean c1IsSeparator = c1 == separatorChar;
boolean c2IsSeparator = c2 == separatorChar;
if (c1IsSeparator == false || c2IsSeparator == false) {
if (c1IsSeparator) {
return -1;
Expand All @@ -58,17 +58,17 @@ interface CharComparator {
}
return len1 - len2;
};
this.separatorChar = separatorChar;
}

Comparator<String> pathComparator() {
return pathComparator;
}

@SuppressForbidden(reason = "we need the separator as a char, not a string")
boolean isParent(String maybeParent, String path) {
logger.trace(() -> Strings.format("checking isParent [%s] for [%s]", maybeParent, path));
return pathStartsWith(path, maybeParent)
&& (path.length() > maybeParent.length() && path.charAt(maybeParent.length()) == File.separatorChar);
&& (path.length() > maybeParent.length() && path.charAt(maybeParent.length()) == separatorChar);
}

boolean samePath(String currentPath, String nextPath) {
Expand All @@ -78,9 +78,4 @@ boolean samePath(String currentPath, String nextPath) {
protected abstract boolean pathStartsWith(String pathString, String pathPrefix);

protected abstract boolean pathsAreEqual(String path1, String path2);

@SuppressForbidden(reason = "we need the separator as a char, not a string")
private static boolean isPathSeparator(char c) {
return c == File.separatorChar;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
public class FileAccessTreeComparisonTests extends ESTestCase {

public void testPathOrderPosix() {
var pathComparator = new CaseSensitiveComparison().pathComparator();
var pathComparator = new CaseSensitiveComparison('/').pathComparator();

// Unix-style
// Directories come BEFORE files; note that this differs from natural lexicographical order
Expand All @@ -43,8 +43,8 @@ public void testPathOrderPosix() {
assertThat(pathComparator.compare("/a\\b", "/a/b.txt"), greaterThan(0));
}

public void testPathOrderWindowsOrMac() {
var pathComparator = new CaseInsensitiveComparison().pathComparator();
public void testPathOrderWindows() {
var pathComparator = new CaseInsensitiveComparison('\\').pathComparator();

// Directories come BEFORE files; note that this differs from natural lexicographical order
assertThat(pathComparator.compare("C:\\a\\b", "C:\\a.xml"), lessThan(0));
Expand All @@ -56,7 +56,10 @@ public void testPathOrderWindowsOrMac() {
}

public void testPathOrderingSpecialCharacters() {
var pathComparator = (randomBoolean() ? new CaseInsensitiveComparison() : new CaseSensitiveComparison()).pathComparator();
var separatorChar = randomFrom('/', '\\');
var pathComparator = (randomBoolean()
? new CaseInsensitiveComparison(separatorChar)
: new CaseSensitiveComparison(separatorChar)).pathComparator();

assertThat(pathComparator.compare("aa\uD801\uDC28", "aa\uD801\uDC28"), is(0));
assertThat(pathComparator.compare("aa\uD801\uDC28", "aa\uD801\uDC28a"), lessThan(0));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ public void testInvalidExclusiveAccess() {
}

public void testDuplicatePrunedPaths() {
var comparison = new CaseSensitiveComparison();
var comparison = new CaseSensitiveComparison('/');
List<String> inputPaths = List.of("/a", "/a", "/a/b", "/a/b", "/b/c", "b/c/d", "b/c/d", "b/c/d", "e/f", "e/f");
List<String> outputPaths = List.of("/a", "/b/c", "b/c/d", "e/f");
var actual = FileAccessTree.pruneSortedPaths(inputPaths.stream().map(p -> normalizePath(path(p))).toList(), comparison);
Expand All @@ -372,7 +372,7 @@ public void testDuplicatePrunedPaths() {
}

public void testDuplicatePrunedPathsWindows() {
var comparison = new CaseInsensitiveComparison();
var comparison = new CaseInsensitiveComparison('/');
List<String> inputPaths = List.of("/a", "/A", "/a/b", "/a/B", "/b/c", "b/c/d", "B/c/d", "b/c/D", "e/f", "e/f");
List<String> outputPaths = List.of("/a", "/b/c", "b/c/d", "e/f");
var actual = FileAccessTree.pruneSortedPaths(inputPaths.stream().map(p -> normalizePath(path(p))).toList(), comparison);
Expand All @@ -384,7 +384,7 @@ public void testDuplicateExclusivePaths() {
// Bunch o' handy definitions
var pathAB = path("/a/b");
var pathCD = path("/c/d");
var comparison = randomBoolean() ? new CaseSensitiveComparison() : new CaseInsensitiveComparison();
var comparison = randomBoolean() ? new CaseSensitiveComparison('/') : new CaseInsensitiveComparison('/');
var originalFileData = FileData.ofPath(pathAB, READ).withExclusive(true);
var fileDataWithWriteMode = FileData.ofPath(pathAB, READ_WRITE).withExclusive(true);
var original = new ExclusiveFileEntitlement("component1", "module1", new FilesEntitlement(List.of(originalFileData)));
Expand Down