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

[Brent Christian]

On Mon, 24 May 2021 21:57:50 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 one additional commit since the last revision:
> 
>   Move merge and rejectUndecidedClass methods to OIF.Config
>   As default methods on OIF, their implementations were not concrete and not trustable

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

> 175:  *                 // Initially this would be the static JVM-wide filter passed from the OIS constructor
> 176:  *                 // Append the filter to reject all UNDECIDED results
> 177:  *                 filter = next.merge(filter).rejectUndecidedClass();

Update for merge() now being class method

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

> 864:         /**
> 865:          * Returns a filter that merges the status of a filter and another filter.
> 866:          * If the other filter is {@code null}, the filter is returned.

Now that this method is static, this sentence could be further clarified with some markup, IMO:
"If `{@code anotherFilter}` is `{@code null}`, `{@code filter}` is returned."

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

> 872:          *     <li>Invoke {@code filter} on the {@code FilterInfo} to get its {@code status};
> 873:          *     <li>Return {@code REJECTED} if the {@code status} is {@code REJECTED};
> 874:          *     <li>Invoke the {@code otherFilter} to get the {@code otherStatus};

"the `otherFilter`" -> "`anotherFilter`"

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

> 890: 
> 891:         /**
> 892:          * Returns a filter that invokes a filter and maps {@code UNDECIDED} to {@code REJECTED}

"...that invokes _the given_ filter..." ?

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

> 893:          * for classes, with some exceptions, and otherwise returns the status.
> 894:          * The filter returned checks that classes not {@code ALLOWED} and not {@code REJECTED} by the filter
> 895:          * are {@code REJECTED}, if the class is an array and the base component type is not allowed,

Could/should this be simplified to, "...checks that classes not ALLOWED by the filter are REJECTED."?

Also, I would add something like, "...,_including_ if the class is..." or "...,_even_ if the class is..."; otherwise it sounds a bit like this _only_ applies to arrays.

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

> 1420:          * {@linkplain ObjectInputStream#ObjectInputStream(InputStream) ObjectInputStream constructors}.
> 1421:          * When invoked from {@link ObjectInputStream#setObjectInputFilter(ObjectInputFilter)
> 1422:          * to set the stream-specific filter} the requested filter replaces the static serial filter,

"When invoked _from to_ set the..."

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

> 1475: 
> 1476:             /**
> 1477:              * Returns the class name name of this builtin deserialization filter factory.

name name

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

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