8170291: Unpredictable results of j.i.ObjectInputFilter::createFilter

Reviewed-by: dfuchs

jdk/src/java.base/share/classes/java/io/ObjectInputFilter.java

@@ -350,16 +350,24 @@ public interface ObjectInputFilter {
          * The first pattern that matches, working from left to right, determines
          * the {@link Status#ALLOWED Status.ALLOWED}
          * or {@link Status#REJECTED Status.REJECTED} result.
-         * If nothing matches, the result is {@link Status#UNDECIDED Status.UNDECIDED}.
+         * If the limits are not exceeded and no pattern matches the class,
+         * the result is {@link Status#UNDECIDED Status.UNDECIDED}.
          *
          * @param pattern the pattern string to parse; not null
-         * @return a filter to check a class being deserialized; may be null;
+         * @return a filter to check a class being deserialized;
          *          {@code null} if no patterns
-         * @throws IllegalArgumentException
+         * @throws IllegalArgumentException if the pattern string is illegal or
-         *                if a limit is missing the name, or the long value
+         *         malformed and cannot be parsed.
-         *                is not a number or is negative,
+         *         In particular, if any of the following is true:
-         *                or the module name is missing if the pattern contains "/"
+         * <ul>
-         *                or if the package is missing for ".*" and ".**"
+         * <li>   if a limit is missing the name or the name is not one of
+         *        "maxdepth", "maxrefs", "maxbytes", or "maxarray"
+         * <li>   if the value of the limit can not be parsed by
+         *        {@link Long#parseLong Long.parseLong} or is negative
+         * <li>   if the pattern contains "/" and the module name is missing
+         *        or the remaining pattern is empty
+         * <li>   if the package is missing for ".*" and ".**"
+         * </ul>
          */
         public static ObjectInputFilter createFilter(String pattern) {
             Objects.requireNonNull(pattern, "pattern");
@@ -402,14 +410,19 @@ public interface ObjectInputFilter {
              * Returns an ObjectInputFilter from a string of patterns.
              *
              * @param pattern the pattern string to parse
-             * @return a filter to check a class being deserialized; not null
+             * @return a filter to check a class being deserialized;
+             *          {@code null} if no patterns
              * @throws IllegalArgumentException if the parameter is malformed
              *                if the pattern is missing the name, the long value
              *                is not a number or is negative.
              */
             static ObjectInputFilter createFilter(String pattern) {
-                Global filter = new Global(pattern);
+                try {
-                return filter.isEmpty() ? null : filter;
+                    return new Global(pattern);
+                } catch (UnsupportedOperationException uoe) {
+                    // no non-empty patterns
+                    return null;
+                }
             }
 
             /**
@@ -417,8 +430,10 @@ public interface ObjectInputFilter {
              *
              * @param pattern a pattern string of filters
              * @throws IllegalArgumentException if the pattern is malformed
+             * @throws UnsupportedOperationException if there are no non-empty patterns
              */
             private Global(String pattern) {
+                boolean hasLimits = false;
                 this.pattern = pattern;
 
                 maxArrayLength = Long.MAX_VALUE; // Default values are unlimited
@@ -436,6 +451,7 @@ public interface ObjectInputFilter {
                     }
                     if (parseLimit(p)) {
                         // If the pattern contained a limit setting, i.e. type=value
+                        hasLimits = true;
                         continue;
                     }
                     boolean negate = p.charAt(0) == '!';
@@ -510,18 +526,9 @@ public interface ObjectInputFilter {
                         filters.add(c -> moduleName.equals(c.getModule().getName()) ? patternFilter.apply(c) : Status.UNDECIDED);
                     }
                 }
+                if (filters.isEmpty() && !hasLimits) {
+                    throw new UnsupportedOperationException("no non-empty patterns");
                 }
-
-            /**
-             * Returns if this filter has any checks.
-             * @return {@code true} if the filter has any checks, {@code false} otherwise
-             */
-            private boolean isEmpty() {
-                return filters.isEmpty() &&
-                        maxArrayLength == Long.MAX_VALUE &&
-                        maxDepth == Long.MAX_VALUE &&
-                        maxReferences == Long.MAX_VALUE &&
-                        maxStreamBytes == Long.MAX_VALUE;
             }
 
             /**

jdk/src/java.base/share/classes/java/io/ObjectInputStream.java

@@ -1164,6 +1164,13 @@ public class ObjectInputStream
      * for each class and reference in the stream.
      * The filter can check any or all of the class, the array length, the number
      * of references, the depth of the graph, and the size of the input stream.
+     * The depth is the number of nested {@linkplain #readObject readObject}
+     * calls starting with the reading of the root of the graph being deserialized
+     * and the current object being deserialized.
+     * The number of references is the cumulative number of objects and references
+     * to objects already read from the stream including the current object being read.
+     * The filter is invoked only when reading objects from the stream and for
+     * not primitives.
      * <p>
      * If the filter returns {@link ObjectInputFilter.Status#REJECTED Status.REJECTED},
      * {@code null} or throws a {@link RuntimeException},
@@ -1178,8 +1185,9 @@ public class ObjectInputStream
      *
      * @implSpec
      * The filter, when not {@code null}, is invoked during {@link #readObject readObject}
-     * and {@link #readUnshared readUnshared} for each object
+     * and {@link #readUnshared readUnshared} for each object (regular or class) in the stream.
-     * (regular or class) in the stream including the following:
+     * Strings are treated as primitives and do not invoke the filter.
+     * The filter is called for:
      * <ul>
      *     <li>each object reference previously deserialized from the stream
      *     (class is {@code null}, arrayLength is -1),

jdk/test/java/io/Serializable/serialFilter/SerialFilterTest.java

@@ -99,16 +99,6 @@ public class SerialFilterTest implements Serializable {
     @DataProvider(name="InvalidPatterns")
     static Object[][] invalidPatterns() {
         return new Object [][] {
-                {"maxrefs=-1"},
-                {"maxdepth=-1"},
-                {"maxbytes=-1"},
-                {"maxarray=-1"},
-                {"xyz=0"},
-                {"xyz=-1"},
-                {"maxrefs=0xabc"},
-                {"maxrefs=abc"},
-                {"maxrefs="},
-                {"maxrefs=+"},
                 {".*"},
                 {".**"},
                 {"!"},
@@ -122,10 +112,31 @@ public class SerialFilterTest implements Serializable {
     static Object[][] limits() {
         // The numbers are arbitrary > 1
         return new Object[][] {
+                {"maxrefs", 1},     // 0 is tested as n-1
                 {"maxrefs", 10},
                 {"maxdepth", 5},
                 {"maxbytes", 100},
                 {"maxarray", 16},
+                {"maxbytes", Long.MAX_VALUE},
+        };
+    }
+
+    @DataProvider(name="InvalidLimits")
+    static Object[][] invalidLimits() {
+        return new Object[][] {
+                {"maxrefs=-1"},
+                {"maxdepth=-1"},
+                {"maxbytes=-1"},
+                {"maxarray=-1"},
+                {"xyz=0"},
+                {"xyz=-1"},
+                {"maxrefs=0xabc"},
+                {"maxrefs=abc"},
+                {"maxrefs="},
+                {"maxrefs=+"},
+                {"maxbytes=-1"},
+                {"maxbytes=9223372036854775808"},
+                {"maxbytes=-9223372036854775807"},
         };
     }
 
@@ -305,7 +316,7 @@ public class SerialFilterTest implements Serializable {
      * @param value a test value
      */
     @Test(dataProvider="Limits")
-    static void testLimits(String name, int value) {
+    static void testLimits(String name, long value) {
         Class<?> arrayClass = new int[0].getClass();
         String pattern = String.format("%s=%d;%s=%d", name, value, name, value - 1);
         ObjectInputFilter filter = ObjectInputFilter.Config.createFilter(pattern);
@@ -319,6 +330,21 @@ public class SerialFilterTest implements Serializable {
                 "last limit value not used: " + filter);
     }
 
+    /**
+     * Test invalid limits.
+     * Construct a filter with the limit, it should throw IllegalArgumentException.
+     * @param pattern a pattern to test
+     */
+    @Test(dataProvider="InvalidLimits", expectedExceptions=java.lang.IllegalArgumentException.class)
+    static void testInvalidLimits(String pattern) {
+        try {
+            ObjectInputFilter filter = ObjectInputFilter.Config.createFilter(pattern);
+        } catch (IllegalArgumentException iae) {
+            System.out.printf("    success exception: %s%n", iae);
+            throw iae;
+        }
+    }
+
     /**
      * Test that returning null from a filter causes deserialization to fail.
      */
@@ -332,7 +358,7 @@ public class SerialFilterTest implements Serializable {
                 }
             });
         } catch (InvalidClassException ice) {
-            System.out.printf("Success exception: %s%n", ice);
+            System.out.printf("    success exception: %s%n", ice);
             throw ice;
         }
     }