RFR: 8275338: Add JFR events for notable serialization situations [v4]

Roger Riggs rriggs at openjdk.org
Tue Dec 19 19:44:44 UTC 2023


On Tue, 19 Dec 2023 12:21:05 GMT, Raffaello Giulietti <rgiulietti at openjdk.org> wrote:

>> Adds serialization misdeclaration events to JFR.
>
> Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Better name for a label, corrected name of removed field.

src/java.base/share/classes/java/io/SerializationMisdeclarationChecker.java line 108:

> 106:                     SUID_NAME + " should be declared of type long");
> 107:         }
> 108:         if (!isStatic(f)) {

The two calls to isStatic could be reordered closer together to be a single if (isstatic()) { ... } else {... }.

src/java.base/share/classes/jdk/internal/event/SerializationMisdeclarationEvent.java line 126:

> 124:     public static int ACC_METH_PARAM_TYPES            = 404;
> 125:     public static int ACC_METH_NON_ACCESSIBLE         = 405;
> 126: 

I'd rather see few (fewer, just one) kinds of events, with so many different kinds of events there needs to be a convenient method to look for any kind of serialization mis-declaration.
Perhaps a single event with flags for the different kinds of errors.
The event classes could be responsible for turning the flags back into useful messages on display.

test/jdk/jdk/jfr/event/io/TestSerializationMisdeclarationEvent.java line 157:

> 155:     }
> 156: 
> 157:     private static class A implements Serializable {

The test classes should document the good or badness of the class either in the class/field/method names or in comments.  What's wrong with this XXX.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17129#discussion_r1431846034
PR Review Comment: https://git.openjdk.org/jdk/pull/17129#discussion_r1431850091
PR Review Comment: https://git.openjdk.org/jdk/pull/17129#discussion_r1431856800



More information about the security-dev mailing list