On Fri, 28 May 2021 15:50:29 GMT, Daniel Fuchs <dfu...@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