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