RFR 9: 8155760 Implement Serialization Filtering

[Roger Riggs]

Hi Chris,

Thanks for the review and comments...

On 9/14/2016 5:46 AM, Chris Hegarty wrote:
> On 08/09/16 20:09, Roger Riggs wrote:
>> Please review updates to the Serialization filtering API and
>> implementation:
>>   - The ObjectInputFilter pattern based filters support matching on
>> module names as well as package and class names.
>>   - Rename of system property and java.security property for
>> configurable filters.  (jdk.serialFilter)
>>   - ObjectInputFilter clarifications about the values passed to the 
>> filter
>>   - Javadoc editorial improvements
>>   - Clarification of SerializablePermission description of targets
>>
>>   - More tests
>>
>> Webrev:
>> http://cr.openjdk.java.net/~rriggs/webrev-serial-filter-jdk9-8155760/
>
> This looks very good Roger, just a few comments:
>
> 1) The pattern separator in the java.security file should be ';'
>    Right?
>        925 #jdk.serialFilter=pattern,pattern
Good catch, will fix
>                                    ^^^
>
> 2) A question on the expected usage. During the initialization of
>    OIS the process-wide filter is cached in an instance field,
>    'serialFilter'. A subsequent change to the process-wide filter
>    will not affect the OIS instance. I think this is ok, just
>    checking the expected usage, as the example in the OIF class
>    description reads the process-wide filter ever time. Maybe
>    the example should be changed slightly to not promote this type
>    of usage? Maybe just remove the call to getSerialFilter?
The process-wide filter is set-once, so it can't change. 
(ObjectInputFilter.setSerialFilter())
The caching in OIS as a final field allows some optimizations;
Client code as in the example could do the same for performance reasons.

>
> 3) Are third-party OIS implementations required, or expected, to
>    "callback" to the filter? The spec, of course, would appear to
>    allow it, but not require it? Just wondering if this is required,
>    or not, as it is not clear to me.
Practically, I don't think it can be required; but is encouraged.

Thanks, Roger


>
> -Chris.
>
>
>> SpecDiff:
>> http://cr.openjdk.java.net/~rriggs/filter-diffs/overview-summary.html
>>
>> Javadoc (subset)
>> http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/ObjectInputStream.html 
>>
>>
>> http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/ObjectInputFilter.html 
>>
>>
>> http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/SerializablePermission.html 
>>
>>
>>
>> Thanks, Roger
>>
>>
>>