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



Reply via email to