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

[Roger Riggs]

On Fri, 28 May 2021 15:50:29 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> Roger Riggs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 13 additional commits since the last revision:
>> 
>>  - Merge branch 'master' into 8264859-context-filter-factory
>>  - Added test for rejectUndecidedClass array cases
>>    Added test for preventing disabling filter factory
>>    Test cleanup
>>  - Editorial updates to review comments.
>>    Simplify the builtin filter factory implementation.
>>    Add atomic update to setting the filter factory.
>>    Clarify the description of OIS.setObjectInputFilter.
>>    Cleanup of the example code.
>>  - Editorial updates
>>    Updated java.security properties to include jdk.serialFilterFactory
>>    Added test cases to SerialFilterFactoryTest for java.security properties and
>>    enabling of the SecurityManager with existing policy permission files
>>    Corrected a test that OIS.setObjectInputFilter could not be called twice.
>>    Removed a Factory test that was not intended to be committed
>>  - 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
>>  - Move merge and rejectUndecidedClass methods to OIF.Config
>>    As default methods on OIF, their implementations were not concrete and not trustable
>>  - Review suggestions included;
>>    Added @implSpec for default methods in OIF;
>>    Added restriction that the filter factory cannot be set after an ObjectInputStream has been created and applied the current filter factory
>>  - Editorial javadoc updated based on review comments.
>>    Clarified behavior of rejectUndecidedClass method.
>>    Example test added to check status returned from file.
>>  - Editorial updates to review comments
>>    Add filter tracing support
>>  - ... and 3 more: https://git.openjdk.java.net/jdk/compare/3d56b7b2...0930f0f8
>
> src/java.base/share/classes/java/io/ObjectInputFilter.java line 352:
> 
>> 350:      *
>> 351:      * @param predicate a predicate to test a non-null Class, non-null
>> 352:      * @param otherStatus a Status to use if the predicate is {@code false}
> 
> should it be specified that the `otherStatus` must also be non-null?
> Is there a blanket statement somewhere for NPE, or are `@throws NPE` clauses missing everywhere?
> I'm asking because elsewhere in the JDK we usually specify that "unless otherwise specified, null parameters are not allowed and a NullPointerException will be thrown". But here it seems the opposite direction has been taken (which is fine), but the fact that NPE will be thrown if `null` is passed for a parameter specified as non-null seems to be implicit.

At the end of FIlterInputStream javadoc, there is the blanket statement.

 * Unless otherwise noted, passing a {@code null} argument to a
 * method in this interface and its nested classes will cause a
 * {@link NullPointerException} to be thrown.``` 
including non-null on the @ param line reinforces the point

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

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