RFR 9: JEP 290: Filter Incoming Serialization Data

Wed Jul 20 08:19:29 UTC 2016

Hi Roger,

On first reading, I have the following thoughts:

- The name "ObjectInputFilter" makes me think that it is a function that 
"filters" the input stream (like a Predicate in 
Stream::filter(Predicate)), but it is in fact a validator that 
terminates deserialization on 1st rejection. So perhaps a different name 
is in order - ObjectInputValidator ?

- I haven't found in the public javadocs, an explanation of what happens 
when the filter returns ALLOWED, REJECTED or UNDECIDED. Docs just say 
that the deserialization is terminated (on UNDECIDED too?) but not with 
what exception (there is some explanation on OIS::filterCheck, but this 
is a private method).

- The crux of behavioral docs is on the OIS::setObjectInputFilter 
method. I would expect it to be on the ObjectInputFilter class, but I 
understand that OIS subclasses might have a different behavior. How do 
they behave indeed? For example IIOPInputStream does not use the filter, 
right?

- I had some trouble to precisely understand the behavior from the docs 
alone. The following in OIS::setObjectInputFilter:

1174      * @implSpec
1175      * The filter, when {@code non-null}, is invoked during 
{@linkplain #readObject()}
1176      * for each object (regular or class) in the stream including 
the following:
1177      * <ul>
1178      *     <li>each object reference previously deserialized from 
the stream,
1179      *     <li>each regular class,
1180      *     <li>each interface of a dynamic proxy and the dynamic 
proxy class itself,
1181      *     <li>each array is filtered using the array size and the 
type of the array,
1182      *     <li>each object replaced by its class' {@code 
readResolve} method
1183      *         is filtered using the replacement object's class and 
if it is an array, the length,
1184      *     <li>and each object replaced by {@linkplain 
#resolveObject resolveObject}
1185      *         is filtered using the replacement object's class and 
if it is an array, the length.
1186      * </ul>
1187      *
1188      * When  the {@link ObjectInputFilter#checkInput checkInput} 
method is invoked
1189      * it is passed the current class, (null if no class),

...does not specify when the passed-in class might be "null". Reading 
the implementation, I see it is null when a back reference to previously 
deserialized object is read from stream, but javadocs are not clear 
about that.

- I wonder if invoking the filter for each interface of a dynamic proxy 
is necessary (other properties passed to the filter don't change during 
iteration through the interfaces and each interface call-back is not an 
indicator that an object is about to be read-in next). This is not 
uniform with other objects where the filter is invoked only once. Why is 
a dynamic proxy so special? If one wants to check the proxy interfaces 
in the filter, she can obtain them manually:

if (Proxy.isProxyClass(clazz)) {
     for (Class<?> intf : class.getInterfaces()) {
         ...
     }
}

- The docs might be more clear about when precisely the filter is 
invoked (i.e. after the type of the object and possible length of array 
or the back reference has already been read from the stream, but the 
object state has not been read yet). This is important to correctly 
interpret the streamBytes parameter. The docs might also be more clear 
about when the nRefs is incremented (it says: "for each call". Is it 
before or after the call?).

- What is the purpose of the UNDECIDED return? I suspect it is meant to 
be used in some filter implementation that delegates the validation to 
some "parent" filter and respects its decision unless it is UNDECIDED in 
which case it decides (or does not) on its own. Should such strategy be 
mentioned in the docs to encourage inter-operable filter implementations?

- The call-back is invoked after the type of the object and possible 
array length is read from stream but before the object's state is read. 
Suppose that the object that is about to be read is either 
Externalizable object or an object with a readObject() method(s) that 
consume block data from the stream. This block data can be large. Should 
there be a call-back to "announce" the block data too? (for example, 
when the 'clazz' is null and the 'size' is 0, the call-back reports a 
back-reference to a previously read object, but when the 'clazz' is null 
and the 'size' > 0, it announces the 'size' bytes of block data. Does 
this make sense?)

That's it for the start. If I notice something else, I'll post again.

Regards, Peter

On 07/19/2016 04:02 PM, Roger Riggs wrote:
> Please review the design, implementation, and tests of JEP 290: Filter 
> Incoming Serialization Data[1]
>
> It allows incoming streams of object-serialization data to be filtered 
> in order to improve both security and robustness.
> The JEP[1] has more detail on the background and scope.
>
> The core mechanism is a filter interface implemented by serialization 
> clients and set on an |ObjectInputStream|. The filter is called during 
> the deserialization process to validate the classes being 
> deserialized, the sizes of arrays being created, and metrics 
> describing stream length, stream depth, and number of references as 
> the stream is being decoded.
>
> A process-wide filter can be configured that is applied to every 
> ObjectInputStream.
> The API of ObjectInputStream can be used to set a custom filter to 
> supersede or augment the process-wide filter.
>
> Webrev:
> http://cr.openjdk.java.net/~rriggs/webrev-serial-filter-jdk9-8155760/
>
> 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 
>
>
> Comments appreciated, Roger
>
> [1] JEP 290:   https://bugs.openjdk.java.net/browse/JDK-8154961
>