[JENKINS-47736] Introduced ClassFilter.setDefault (#208)

* [JENKINS-47736] Introduced ClassFilter.setDefault. * Review comments from @oleg-nenashev. * [JENKINS-47736] - Add some annotations, mostly to kick-off CI
jenkinsci · Jan 9, 2018 · 1fda115 · 1fda115
1 parent 3ce2e98
commit 1fda115
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 13 deletions.
diff --git a/src/main/java/hudson/remoting/ClassFilter.java b/src/main/java/hudson/remoting/ClassFilter.java
@@ -15,38 +15,61 @@
 import java.util.regex.PatternSyntaxException;
 
 import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
 
 /**
  * Restricts what classes can be received through remoting.
- *
+ * The same filter is also applied by Jenkins core for XStream serialization.
  * @author Kohsuke Kawaguchi
  * @since 2.53
  */
 public abstract class ClassFilter {
     /**
-     * Property to set to <b>override</b> the blacklist used by {{@link #DEFAULT} with a different set.
+     * Property to set to <b>override</b> the blacklist used by {@link #STANDARD} with a different set.
      * The location should point to a a file containing regular expressions (one per line) of classes to blacklist.
      * If this property is set but the file can not be read the default blacklist will be used.
      * @since 2.53.2
+     * @deprecated use {@link #setDefault} as needed
      */
+    @Deprecated
     public static final String FILE_OVERRIDE_LOCATION_PROPERTY = "hudson.remoting.ClassFilter.DEFAULTS_OVERRIDE_LOCATION";
 
     private static final Logger LOGGER = Logger.getLogger(ClassFilter.class.getName());
 
-    protected boolean isBlacklisted(String name) {
+    /**
+     * Whether a given class should be blocked, before even attempting to load that class.
+     * @param name {@link Class#getName}
+     * @return false by default; override to return true to blacklist this class
+     */
+    public boolean isBlacklisted(@Nonnull String name) {
         return false;
     }
 
-    protected boolean isBlacklisted(Class c) {
+    /**
+     * Whether a given class should be blocked, after having loaded that class.
+     * @param c a loaded class
+     * @return false by default; override to return true to blacklist this class
+     */
+    public boolean isBlacklisted(@Nonnull Class c) {
         return false;
     }
 
+    /**
+     * API version of {@link #isBlacklisted(String)} SPI.
+     * @return the same {@code name}
+     * @throws SecurityException if it is blacklisted
+     */
 	public final String check(String name) {
 		if (isBlacklisted(name))
 			throw new SecurityException("Rejected: " +name);
 		return name;
 	}
 
+    /**
+     * API version of {@link #isBlacklisted(Class)} SPI.
+     * @return the same {@code c}
+     * @throws SecurityException if it is blacklisted
+     */
 	public final Class check(Class c) {
 		if (isBlacklisted(c))
 			throw new SecurityException("Rejected: " +c.getName());
@@ -62,11 +85,13 @@ public final Class check(Class c) {
         "^com[.]sun[.]javafx[.].*",
         "^com[.]sun[.]org[.]apache[.]regex[.]internal[.].*",
         "^java[.]awt[.].*",
+        "^java[.]lang[.]reflect[.]Method$",
         "^java[.]rmi[.].*",
         "^javax[.]management[.].*",
         "^javax[.]naming[.].*",
         "^javax[.]script[.].*",
         "^javax[.]swing[.].*",
+        "^net[.]sf[.]json[.].*",
         "^org[.]apache[.]commons[.]beanutils[.].*",
         "^org[.]apache[.]commons[.]collections[.]functors[.].*",
         "^org[.]apache[.]myfaces[.].*",
@@ -79,37 +104,66 @@ public final Class check(Class c) {
         "^sun[.]rmi[.].*",
         "^javax[.]imageio[.].*",
         "^java[.]util[.]ServiceLoader$",
-        "^java[.]net[.]URLClassLoader$"
+        "^java[.]net[.]URLClassLoader$",
+        "^java[.]security[.]SignedObject$"
     };
 
     /**
-     * A set of sensible default filtering rules to apply,
-     * unless the context guarantees the trust between two channels.
+     * The currently used default.
+     * Defaults to {@link #STANDARD}.
+     */
+    public static final ClassFilter DEFAULT = new ClassFilter() {
+        @Override
+        public boolean isBlacklisted(Class c) {
+            return CURRENT_DEFAULT.isBlacklisted(c);
+        }
+        @Override
+        public boolean isBlacklisted(String name) {
+            return CURRENT_DEFAULT.isBlacklisted(name);
+        }
+    };
+
+    private static @Nonnull ClassFilter CURRENT_DEFAULT;
+
+    /**
+     * Changes the effective value of {@link #DEFAULT}.
+     * @param filter a new default to set; may or may not delegate to {@link STANDARD}
+     * @since FIXME
+     */
+    public static void setDefault(@Nonnull ClassFilter filter) {
+        CURRENT_DEFAULT = filter;
+    }
+
+    /**
+     * A set of sensible default filtering rules to apply, based on a configurable blacklist.
      */
-    public static final ClassFilter DEFAULT;
+    public static final ClassFilter STANDARD;
 
     static {
         try {
-            DEFAULT = createDefaultInstance();
+            STANDARD = createDefaultInstance();
         } catch (ClassFilterException ex) {
             LOGGER.log(Level.SEVERE, "Default class filter cannot be initialized. Remoting will not start", ex);
             throw new ExceptionInInitializerError(ex);
         }
+        CURRENT_DEFAULT = STANDARD;
     }
 
     /**
-     * Adds an additional exclusion to {@link #DEFAULT}.
+     * Adds an additional exclusion to {@link #STANDARD}.
      * 
      * Does nothing if the default list has already been customized via {@link #FILE_OVERRIDE_LOCATION_PROPERTY}.
      * This API is not supposed to be used anywhere outside Jenkins core, calls for other sources may be rejected later.
      * @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 3.11
+     * @deprecated use {@link #setDefault} as needed
      */
+    @Deprecated
     public static void appendDefaultFilter(Pattern filter) throws ClassFilterException {
         if (System.getProperty(FILE_OVERRIDE_LOCATION_PROPERTY) == null) {
-            ((RegExpClassFilter) DEFAULT).add(filter.toString());
+            ((RegExpClassFilter) STANDARD).add(filter.toString());
         }
     }
 
@@ -228,7 +282,7 @@ void add(String pattern) throws ClassFilterException {
         }
 
         @Override
-        protected boolean isBlacklisted(String name) {
+        public boolean isBlacklisted(String name) {
             for (Object p : blacklistPatterns) {
                 if (p instanceof Pattern && ((Pattern)p).matcher(name).matches()) {
                     return true;
@@ -252,7 +306,9 @@ public String toString() {
     /**
      * Class for propagating exceptions in {@link ClassFilter}.
      * @since 3.11
+     * @deprecated Only used by deprecated {@link #appendDefaultFilter}.
      */
+    @Deprecated
     public static class ClassFilterException extends Exception {
 
         @CheckForNull

diff --git a/src/test/java/hudson/remoting/ClassFilterTest.java b/src/test/java/hudson/remoting/ClassFilterTest.java
@@ -48,7 +48,7 @@ public class ClassFilterTest implements Serializable {
 
     private static class TestFilter extends ClassFilter {
         @Override
-        protected boolean isBlacklisted(String name) {
+        public boolean isBlacklisted(String name) {
             return name.contains("Security218");
         }
     }