8170291: Unpredictable results of j.i.ObjectInputFilter::createFilter
Reviewed-by: dfuchs
diff --git a/jdk/src/java.base/share/classes/java/io/ObjectInputFilter.java b/jdk/src/java.base/share/classes/java/io/ObjectInputFilter.java
index e222f47..a0e998c 100644
--- a/jdk/src/java.base/share/classes/java/io/ObjectInputFilter.java
+++ b/jdk/src/java.base/share/classes/java/io/ObjectInputFilter.java
@@ -350,16 +350,24 @@
* 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
- * if a limit is missing the name, or the long value
- * is not a number or is negative,
- * or the module name is missing if the pattern contains "/"
- * or if the package is missing for ".*" and ".**"
+ * @throws IllegalArgumentException if the pattern string is illegal or
+ * malformed and cannot be parsed.
+ * In particular, if any of the following is true:
+ * <ul>
+ * <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 @@
* 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);
- return filter.isEmpty() ? null : filter;
+ try {
+ return new Global(pattern);
+ } catch (UnsupportedOperationException uoe) {
+ // no non-empty patterns
+ return null;
+ }
}
/**
@@ -417,8 +430,10 @@
*
* @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 @@
}
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 @@
filters.add(c -> moduleName.equals(c.getModule().getName()) ? patternFilter.apply(c) : Status.UNDECIDED);
}
}
- }
-
- /**
- * 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;
+ if (filters.isEmpty() && !hasLimits) {
+ throw new UnsupportedOperationException("no non-empty patterns");
+ }
}
/**
diff --git a/jdk/src/java.base/share/classes/java/io/ObjectInputStream.java b/jdk/src/java.base/share/classes/java/io/ObjectInputStream.java
index eac811d..ed033d6 100644
--- a/jdk/src/java.base/share/classes/java/io/ObjectInputStream.java
+++ b/jdk/src/java.base/share/classes/java/io/ObjectInputStream.java
@@ -1164,6 +1164,13 @@
* 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 @@
*
* @implSpec
* The filter, when not {@code null}, is invoked during {@link #readObject readObject}
- * and {@link #readUnshared readUnshared} for each object
- * (regular or class) in the stream including the following:
+ * and {@link #readUnshared readUnshared} for each object (regular or class) in the stream.
+ * 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),
diff --git a/jdk/test/java/io/Serializable/serialFilter/SerialFilterTest.java b/jdk/test/java/io/Serializable/serialFilter/SerialFilterTest.java
index 9a3bc88..586cc24 100644
--- a/jdk/test/java/io/Serializable/serialFilter/SerialFilterTest.java
+++ b/jdk/test/java/io/Serializable/serialFilter/SerialFilterTest.java
@@ -99,16 +99,6 @@
@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=+"},
{".*"},
{".**"},
{"!"},
@@ -121,11 +111,32 @@
@DataProvider(name="Limits")
static Object[][] limits() {
// The numbers are arbitrary > 1
- return new Object[][]{
+ 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 @@
* @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);
@@ -320,6 +331,21 @@
}
/**
+ * 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.
*/
@Test(expectedExceptions=InvalidClassException.class)
@@ -332,7 +358,7 @@
}
});
} catch (InvalidClassException ice) {
- System.out.printf("Success exception: %s%n", ice);
+ System.out.printf(" success exception: %s%n", ice);
throw ice;
}
}
@@ -606,7 +632,7 @@
}
return array;
} else if (pattern.startsWith("maxarray=")) {
- return allowed ? new int[(int)value] : new int[(int)value+1];
+ return allowed ? new int[(int)value] : new int[(int)value+1];
}
Assert.fail("Object could not be generated for pattern: "
+ pattern
