[JENKINS-45522] - Modify API in order to propagate error in ClassFilter

jenkinsci · Aug 7, 2017 · 62f3213 · 62f3213
1 parent 7e0ffad
commit 62f3213
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 6 deletions.
diff --git a/src/main/java/hudson/remoting/ClassFilter.java b/src/main/java/hudson/remoting/ClassFilter.java
@@ -86,14 +86,26 @@ public final Class check(Class c) {
      * A set of sensible default filtering rules to apply,
      * unless the context guarantees the trust between two channels.
      */
-    public static final ClassFilter DEFAULT = createDefaultInstance();
+    public static final ClassFilter DEFAULT;
+
+    static {
+        try {
+            DEFAULT = createDefaultInstance();
+        } catch (ClassFilterException ex) {
+            LOGGER.log(Level.SEVERE, "Default class filter cannot be initialized. Remoting will not start", ex);
+            throw new IllegalStateException("Default class filter cannot be initialized", ex);
+        }
+    }
 
     /**
      * Adds an additional exclusion to {@link #DEFAULT}.
      * Does nothing if the default list has already been customized via {@link #FILE_OVERRIDE_LOCATION_PROPERTY}.
      * @param filter a regular expression for {@link Class#getName} which, if matched according to {@link Matcher#matches}, will blacklist the class
+     * @throws ClassFilterException Filter pattern cannot be applied.
+     *                              It means either unexpected processing error or rejection by the internal logic.
+     * @since TODO
      */
-    public static void appendDefaultFilter(Pattern filter) {
+    public static void appendDefaultFilter(Pattern filter) throws ClassFilterException {
         if (System.getProperty(FILE_OVERRIDE_LOCATION_PROPERTY) == null) {
             ((RegExpClassFilter) DEFAULT).add(filter.toString());
         }
@@ -109,7 +121,7 @@ public static void appendDefaultFilter(Pattern filter) {
      * The default filtering rules to apply, unless the context guarantees the trust between two channels. The defaults
      * values provide for user specified overrides - see {@link #FILE_OVERRIDE_LOCATION_PROPERTY}.
      */
-    /*package*/ static ClassFilter createDefaultInstance() {
+    /*package*/ static ClassFilter createDefaultInstance() throws ClassFilterException {
         try {
             List<String> patternOverride = loadPatternOverride();
             if (patternOverride != null) {
@@ -187,21 +199,27 @@ private static final class RegExpClassFilter extends ClassFilter {
 
         private final List<Object> blacklistPatterns;
 
-        RegExpClassFilter(String[] patterns) throws PatternSyntaxException {
+        RegExpClassFilter(String[] patterns) throws ClassFilterException {
             blacklistPatterns = new ArrayList<>(patterns.length);
             for (String pattern : patterns) {
                 add(pattern);
             }
         }
 
-        void add(String pattern) throws PatternSyntaxException {
+        void add(String pattern) throws ClassFilterException {
+
             if (OPTIMIZE1.matcher(pattern).matches()) {
                 // this is a simple startsWith test, no need to slow things down with a regex
                 blacklistPatterns.add(pattern.substring(1, pattern.length() - 2).replace("[.]", "."));
             } else if (OPTIMIZE2.matcher(pattern).matches()) {
                 // this is a simple startsWith test, no need to slow things down with a regex
                 blacklistPatterns.add(pattern.substring(3, pattern.length() - 4));
             } else {
+                try {
+                    Pattern regex = Pattern.compile(pattern);
+                } catch (PatternSyntaxException ex) {
+                    throw new ClassFilterException("Cannot add RegExp class filter", ex);
+                }
                 blacklistPatterns.add(Pattern.compile(pattern));
             }
         }
@@ -227,4 +245,37 @@ public String toString() {
             return blacklistPatterns.toString();
         }
     }
+
+    /**
+     * Class for propagating exceptions in {@link ClassFilter}.
+     * @since TODO
+     */
+    public static class ClassFilterException extends Exception {
+
+        @CheckForNull
+        final String pattern;
+
+        public ClassFilterException(String message, PatternSyntaxException ex) {
+            this(message, ex, ex.getPattern());
+        }
+
+        public ClassFilterException(String message, @CheckForNull String pattern) {
+            this(message, new IllegalStateException(message), pattern);
+        }
+
+        public ClassFilterException(String message, Throwable cause, @CheckForNull String pattern) {
+            super(message, cause);
+            this.pattern = pattern;
+        }
+
+        @CheckForNull
+        public String getPattern() {
+            return pattern;
+        }
+
+        @Override
+        public String getMessage() {
+            return super.getMessage() + ". Pattern: " + pattern;
+        }
+    }
 }
diff --git a/src/test/java/hudson/remoting/DefaultClassFilterTest.java b/src/test/java/hudson/remoting/DefaultClassFilterTest.java
@@ -146,10 +146,12 @@ public static Matcher<String> blacklisted() {
             return new BlackListMatcher();
         }
 
-        public boolean matches(Object item) {
+        public boolean matches(Object item)  {
             try {
                 ClassFilter.createDefaultInstance().check(item.toString());
                 return Boolean.FALSE;
+            } catch (ClassFilter.ClassFilterException ex) {
+                throw new IllegalStateException("Failed to initialize the default class filter", ex);
             } catch (SecurityException sex) {
                 return Boolean.TRUE;
             }