extract separator

Author: ldematte

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

@@ -11,8 +11,8 @@
 
 class CaseInsensitiveComparison extends FileAccessTreeComparison {
 
-    CaseInsensitiveComparison() {
-        super(CaseInsensitiveComparison::caseInsensitiveCharacterComparator);
+    CaseInsensitiveComparison(char separatorChar) {
+        super(CaseInsensitiveComparison::caseInsensitiveCharacterComparator, separatorChar);
     }
 
     /**

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

@@ -11,8 +11,8 @@
 
 class CaseSensitiveComparison extends FileAccessTreeComparison {
 
-    CaseSensitiveComparison() {
-        super(Character::compare);
+    CaseSensitiveComparison(char separatorChar) {
+        super(Character::compare, separatorChar);
     }
 
     @Override

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

@@ -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;
@@ -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;
     /**

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

@@ -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 {
@@ -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();
@@ -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;
@@ -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) {
@@ -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;
-    }
 }

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

@@ -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
@@ -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));
@@ -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));

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

@@ -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);
@@ -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);
@@ -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)));