RFR: 8264859: Implement Context-Specific Deserialization Filters [v8]

Brent Christian bchristi at openjdk.java.net
Tue May 25 23:43:22 UTC 2021


On Tue, 25 May 2021 15:46:37 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> JEP 415: Context-specific Deserialization Filters extends the deserialization filtering mechanisms with more flexible and customizable protections against malicious deserialization.  See JEP 415: https://openjdk.java.net/jeps/415.
>> The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are extended with additional
>> configuration mechanisms and filter utilities.
>> 
>> javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and `ObjectInputStream`:
>>     http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html
>
> Roger Riggs has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Moved utility filter methods to be static on ObjectInputFilter
>    Rearranged the class javadoc of OIF to describe the parts of
>    deserialization filtering, filters, composite filters, and the filter factory.
>    And other review comment updates...
>  - Refactored tests for utility functions to SerialFilterFunctionTest.java
>    Deleted confused Config.allowMaxLimits() method
>    Updated example to match move of methods to Config
>    Added test of restriction on setting the filterfactory after a OIS has been created
>    Additional Editorial updates

More suggestions for ObjectInputFilter.java.  I hope they're helpful.

src/java.base/share/classes/java/io/ObjectInputFilter.java line 128:

> 126:  *     provides the {@linkplain Config#getSerialFilter static JVM-wide filter} when invoked from the
> 127:  *     {@linkplain ObjectInputStream#ObjectInputStream(InputStream) ObjectInputStream constructors}
> 128:  *     and replaces the static filter and when invoked from

"...replaces the static filter ~~and~~ when invoked..."

src/java.base/share/classes/java/io/ObjectInputFilter.java line 137:

> 135:  * or to {@linkplain #allowFilter(Predicate, Status) allow} or
> 136:  * {@linkplain #rejectFilter(Predicate, Status) reject} classes based on a
> 137:  * {@linkplain Predicate predicate of a class}.

Maybe a new sentence for the Predicate-based filters? e.g. "Filters can be created from a pattern string.  Or they can allow or reject classes based on a Predicate."

src/java.base/share/classes/java/io/ObjectInputFilter.java line 205:

> 203:  *     // Returns a composite filter of the static JVM-wide filter, a thread-specific filter,
> 204:  *     // and the stream-specific filter.
> 205:  *     public ObjectInputFilter apply(ObjectInputFilter curr, ObjectInputFilter next) {

`@Override` on `apply` ?

src/java.base/share/classes/java/io/ObjectInputFilter.java line 211:

> 209:  *             if (filter != null) {
> 210:  *                 // Prepend a filter to assert that all classes have been Allowed or Rejected
> 211:  *                 filter = ObjectInputFilter.Config.rejectUndecidedClass(filter);

~~Config~~ throughout the example

src/java.base/share/classes/java/io/ObjectInputFilter.java line 301:

> 299:      * on the class is {@code true}.
> 300:      * The filter returns {@code ALLOWED} or the {@code otherStatus} based on the predicate
> 301:      * of the {@code non-null} class and {@code UNDECIDED} if the class is {@code null}.

Can this sentence ("_The filter returns...class is null_") be removed? IMO it doesn't add to what's covered in the rest of the method doc.

src/java.base/share/classes/java/io/ObjectInputFilter.java line 334:

> 332:      * on the class is {@code true}.
> 333:      * The filter returns {@code REJECTED} or the {@code otherStatus} based on the predicate
> 334:      * of the {@code non-null} class and {@code UNDECIDED} if the class is {@code null}.

Same - can this sentence ("The filter returns...class is null") be removed?

-------------

Changes requested by bchristi (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3996


More information about the core-libs-dev mailing list