RFR 9: 8155760 Implement Serialization Filtering

Daniel Fuchs daniel.fuchs at oracle.com
Tue Sep 13 08:57:03 UTC 2016


Hi Roger,

Looks good!

One nit: In ObjectInputFilter.java, links to Status.XXXX
should probably use @link instead of @linkplain - e.g
{@linkplain Status#REJECTED REJECTED} or {@linkplain Status#REJECTED 
Status.REJECTED} should probably be @link to make the constants
appear in code font.

No need to regenerate a webrev if you decide to change that.

best regards,

-- daniel

On 12/09/16 21:25, Roger Riggs wrote:
> Hi Daniel,
>
> Thanks for the review and suggestions:
>
> Updated in place:
> 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
>
> http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/SerializablePermission.html
>
>
> On 9/12/2016 1:15 PM, Daniel Fuchs wrote:
>> Hi Roger,
>>
>> ObjectInputStream.java: some cosmetic comments:
>>
>> 317      * {@link ObjectInputFilter.Config#getSerialFilter() the
>> process-wide filter}.
>> 352      * {@link ObjectInputFilter.Config#getSerialFilter() the
>> process-wide filter}.
>>
>>  => should be @linkplain
> ok
>>
>> 1185      * The filter, when not {@code null}, is invoked during
>> {@linkplain #readObject()}
>> 1186      * and {@linkplain #readUnshared readUnshared} for each object
>>        (+ also at lines 1207,1208,1211,1212,
>> Should that be @link? I saw that in other places, readObject and
>> readUnshared were not wrapped in {@code } - so for consistency it
>> might make sense to use @linkplain. However the usual idiom would
>> be to use {@link }.
>
> ok, with explicit method references and class references it should be @link.
>
>>
>> 2046                 // Filter the replacement object
>> 2047                 if (rep != null) {
>> 2048                     if (rep.getClass().isArray()) {
>> 2049                         filterCheck(rep.getClass(),
>> Array.getLength(rep));
>> 2050                     } else {
>> 2051                         filterCheck(rep.getClass(), -1);
>> 2052                     }
>> 2053                 }
>>
>> In this case should the filter be also invoked with the
>> class of each element in the substituted array?
>> Or is it OK that only the array type is checked (could be
>> "[Ljava.lang.Object;" containing elements of classes
>> X, Y, Z, but the filter will only see the array type).
> The filter sees the type of the object returned from resolveObject
> whether it
> is an array or another object type.  In the case of arrays, it is
> usually the array
> length that is most interesting to the filter.
>
> Thanks, Roger
>
>



More information about the core-libs-dev mailing list