Use a specific matcher implementation for REST test blacklists

With this commit we replace the previously used PathMatcher
from the JDK with a specific matcher that is implemented for
this purpose and supports only simple globbing patterns
(i.e. *).

Closes #11391

test-framework/src/main/java/org/elasticsearch/test/rest/BlacklistedPathPatternMatcher.java

@@ -0,0 +1,68 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.elasticsearch.test.rest;
+
+import java.util.regex.Pattern;
+
+/**
+ * Matches blacklist patterns.
+ *
+ * Currently the following syntax is supported:
+ *
+ * <ul>
+ *  <li>Exact matches, as in <code>cat.aliases/10_basic/Empty cluster</code></li>
+ *  <li>Wildcard matches within the same segment of a path , as in <code>indices.get/10_basic/*allow_no_indices*</code>. This will
+ *  match <code>indices.get/10_basic/allow_no_indices</code>, <code>indices.get/10_basic/allow_no_indices_at_all</code> but not
+ *  <code>indices.get/10_basic/advanced/allow_no_indices</code> (contains an additional segment)</li>
+ * </ul>
+ *
+ * Each blacklist pattern is a suffix match on the path. Empty patterns are not allowed.
+ */
+final class BlacklistedPathPatternMatcher {
+    private final Pattern pattern;
+
+    /**
+     * Constructs a new <code>BlacklistedPathPatternMatcher</code> instance from the provided suffix pattern.
+     *
+     * @param p The suffix pattern. Must be a non-empty string.
+     */
+    BlacklistedPathPatternMatcher(String p) {
+        // guard against accidentally matching everything as an empty string lead to the pattern ".*" which matches everything
+        if (p == null || p.trim().isEmpty()) {
+            throw new IllegalArgumentException("Empty blacklist patterns are not supported");
+        }
+        // very simple transformation from wildcard to a proper regex
+        String finalPattern = p
+                .replaceAll("\\*", "[^/]*") // support wildcard matches (within a single path segment)
+                .replaceAll("\\\\,", ",");  // restore previously escaped ',' in paths.
+
+        // suffix match
+        pattern = Pattern.compile(".*" + finalPattern);
+    }
+
+    /**
+     * Checks whether the provided path matches the suffix pattern, i.e. "/foo/bar" will match the pattern "bar".
+     *
+     * @param path The path to match. Must not be null.
+     * @return true iff this path is a suffix match.
+     */
+    public boolean isSuffixMatch(String path) {
+        return pattern.matcher(path).matches();
+    }
+}

test-framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java

@@ -29,7 +29,6 @@ import org.apache.lucene.util.LuceneTestCase.SuppressFsync;
 import org.apache.lucene.util.TimeUnits;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.SuppressForbidden;
-import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.node.Node;
@@ -65,7 +64,6 @@ import java.nio.file.FileSystem;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.PathMatcher;
 import java.nio.file.StandardCopyOption;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -123,9 +121,18 @@ public abstract class ESRestTestCase extends ESIntegTestCase {
     private static final String DEFAULT_TESTS_PATH = "/rest-api-spec/test";
     private static final String DEFAULT_SPEC_PATH = "/rest-api-spec/api";
 
-    private static final String PATHS_SEPARATOR = ",";
+    /**
+     * This separator pattern matches ',' except it is preceded by a '\'. This allows us to support ',' within paths when it is escaped with
+     * a slash.
+     *
+     * For example, the path string "/a/b/c\,d/e/f,/foo/bar,/baz" is separated to "/a/b/c\,d/e/f", "/foo/bar" and "/baz".
+     *
+     * For reference, this regular expression feature is known as zero-width negative look-behind.
+     *
+     */
+    private static final String PATHS_SEPARATOR = "(?<!\\\\),";
 
-    private final PathMatcher[] blacklistPathMatchers;
+    private final List<BlacklistedPathPatternMatcher> blacklistPathMatchers = new ArrayList<>();
     private static RestTestExecutionContext restTestExecutionContext;
 
     private final RestTestCandidate testCandidate;
@@ -133,14 +140,8 @@ public abstract class ESRestTestCase extends ESIntegTestCase {
     public ESRestTestCase(RestTestCandidate testCandidate) {
         this.testCandidate = testCandidate;
         String[] blacklist = resolvePathsProperty(REST_TESTS_BLACKLIST, null);
-        if (blacklist != null) {
-            blacklistPathMatchers = new PathMatcher[blacklist.length];
-            int i = 0;
-            for (String glob : blacklist) {
-                blacklistPathMatchers[i++] = PathUtils.getDefaultFileSystem().getPathMatcher("glob:" + glob);
-            }
-        } else {
-            blacklistPathMatchers = new PathMatcher[0];
+        for (String entry : blacklist) {
+            this.blacklistPathMatchers.add(new BlacklistedPathPatternMatcher(entry));
         }
     }
 
@@ -226,7 +227,7 @@ public abstract class ESRestTestCase extends ESIntegTestCase {
     private static String[] resolvePathsProperty(String propertyName, String defaultValue) {
         String property = System.getProperty(propertyName);
         if (!Strings.hasLength(property)) {
-            return defaultValue == null ? null : new String[]{defaultValue};
+            return defaultValue == null ? Strings.EMPTY_ARRAY : new String[]{defaultValue};
         } else {
             return property.split(PATHS_SEPARATOR);
         }
@@ -324,11 +325,9 @@ public abstract class ESRestTestCase extends ESIntegTestCase {
     @Before
     public void reset() throws IOException, RestException {
         //skip test if it matches one of the blacklist globs
-        for (PathMatcher blacklistedPathMatcher : blacklistPathMatchers) {
-            //we need to replace a few characters otherwise the test section name can't be parsed as a path on windows
-            String testSection = testCandidate.getTestSection().getName().replace("*", "").replace("\\", "/").replaceAll("\\s+/", "/").replace(":", "").trim();
-            String testPath = testCandidate.getSuitePath() + "/" + testSection;
-            assumeFalse("[" + testCandidate.getTestPath() + "] skipped, reason: blacklisted", blacklistedPathMatcher.matches(PathUtils.get(testPath)));
+        for (BlacklistedPathPatternMatcher blacklistedPathMatcher : blacklistPathMatchers) {
+            String testPath = testCandidate.getSuitePath() + "/" + testCandidate.getTestSection().getName();
+            assumeFalse("[" + testCandidate.getTestPath() + "] skipped, reason: blacklisted", blacklistedPathMatcher.isSuffixMatch(testPath));
         }
         //The client needs non static info to get initialized, therefore it can't be initialized in the before class
         restTestExecutionContext.initClient(cluster().httpAddresses(), restClientSettings());

test-framework/src/test/java/org/elasticsearch/test/rest/BlacklistedPathPatternMatcherTests.java

@@ -0,0 +1,72 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.elasticsearch.test.rest;
+
+
+import org.elasticsearch.test.ESTestCase;
+
+public class BlacklistedPathPatternMatcherTests extends ESTestCase {
+
+    public void testMatchesExact() {
+        // suffix match
+        assertMatch("cat.aliases/10_basic/Empty cluster", "/some/suite_path/cat.aliases/10_basic/Empty cluster");
+        // exact match
+        assertMatch("cat.aliases/10_basic/Empty cluster", "cat.aliases/10_basic/Empty cluster");
+        // additional text at the end should not match
+        assertNoMatch("cat.aliases/10_basic/Empty cluster", "cat.aliases/10_basic/Empty clusters in here");
+    }
+
+    public void testMatchesSimpleWildcardPatterns() {
+        assertMatch("termvector/20_issue7121/*", "/suite/termvector/20_issue7121/test_first");
+        assertMatch("termvector/20_issue7121/*", "/suite/termvector/20_issue7121/");
+        // do not cross segment boundaries
+        assertNoMatch("termvector/20_issue7121/*", "/suite/termvector/20_issue7121/test/first");
+    }
+
+    public void testMatchesMultiWildcardPatterns() {
+        assertMatch("indices.get/10_basic/*allow_no_indices*", "/suite/indices.get/10_basic/we_allow_no_indices");
+        assertMatch("indices.get/10_basic/*allow_no_indices*", "/suite/indices.get/10_basic/we_allow_no_indices_at_all");
+        assertNoMatch("indices.get/10_basic/*allow_no_indices*", "/suite/indices.get/10_basic/we_allow_no_indices_at_all/here");
+        assertMatch("indices.get/*/*allow_no_indices*", "/suite/indices.get/10_basic/we_allow_no_indices_at_all");
+        assertMatch("indices.get/*/*allow_no_indices*", "/suite/indices.get/20_basic/we_allow_no_indices_at_all");
+        assertMatch("*/*/*allow_no_indices*", "/suite/path/to/test/indices.get/20_basic/we_allow_no_indices_at_all");
+    }
+
+    public void testMatchesPatternsWithEscapedCommas() {
+        assertMatch("indices.get/10_basic\\,20_advanced/foo", "/suite/indices.get/10_basic,20_advanced/foo");
+    }
+
+    public void testMatchesMixedPatterns() {
+        assertMatch("indices.get/*/10_basic\\,20_advanced/*foo*", "/suite/indices.get/all/10_basic,20_advanced/foo");
+        assertMatch("indices.get/*/10_basic\\,20_advanced/*foo*", "/suite/indices.get/all/10_basic,20_advanced/my_foo");
+        assertMatch("indices.get/*/10_basic\\,20_advanced/*foo*", "/suite/indices.get/all/10_basic,20_advanced/foo_bar");
+    }
+
+
+
+    private void assertMatch(String pattern, String path) {
+        BlacklistedPathPatternMatcher matcher = new BlacklistedPathPatternMatcher(pattern);
+        assertTrue("Pattern [" + pattern + "] should have matched path [" + path + "]", matcher.isSuffixMatch(path));
+    }
+
+    private void assertNoMatch(String pattern, String path) {
+        BlacklistedPathPatternMatcher matcher = new BlacklistedPathPatternMatcher(pattern);
+        assertFalse("Pattern [" + pattern + "] should not have matched path [" + path + "]", matcher.isSuffixMatch(path));
+    }
+}