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