On Tue, 21 Jul 2026 16:53:30 GMT, Chen Liang <[email protected]> wrote:
> The Deserializer documentation states that it only applies to the boot class > path: > > https://github.com/openjdk/valhalla/blob/1c3301cb96d779d06bb6e2391c35ce729bc28189/src/java.base/share/classes/jdk/internal/value/Deserializer.java#L58-L59 > > > There is an implementation defect right now that this annotation is scanned > from all sources. > > Updated `ValueSerializationTest` to add positive and negative tests for a > value class with `@Deserializer` both on and off the boot class path. > > In the test updates I noticed `SerializedObjectCombo` is broken that it never > generates value classes somehow; tracked in > [JDK-8388597](https://bugs.openjdk.org/browse/JDK-8388597). > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). The change here proposes to ignore `@Deserializer` annotation from classes that aren't loaded by the boot classloader. I was in agreement with that change until I saw the test in this PR. The test illustrates a scenario where, even after the proposed fix in this PR, if some application-provided class with a `@Deserializer` annotation is placed on the bootclasspath, then the JDK's legacy deserializer will continue to honour that annotation. I think we shouldn't allow that. So what I proposed was something like the following (untested) change which very specifically only allows this annotation on the primitive wrapper classes: diff --git a/src/java.base/share/classes/java/io/ObjectStreamClass.java b/src/java.base/share/classes/java/io/ObjectStreamClass.java @@ -1362,6 +1362,21 @@ private ObjectStreamClass getVariantFor(Class<?> cl) */ private static Executable findDeserializer(Class<?> clazz, ObjectStreamField[] fields) { + // The JDK allows the @Deserializer annotation to be present only on + // some specific classes. Currently it's only allowed on the primitive + // wrapper classes. We ignore @Deserializer on everything else. + if (clazz != Integer.class + && clazz != Long.class + && clazz != Short.class + && clazz != Double.class + && clazz != Float.class + && clazz != Byte.class + && clazz != Boolean.class + && clazz != Character.class) { + // not a primitive wrapper class, thus no @Deserializer applicable + return null; + } + // class is eligible for having a @Deserializer, find if it has any return Stream.concat( Arrays.stream(clazz.getDeclaredMethods()).filter(m -> Modifier.isStatic(m.getModifiers())), Arrays.stream(clazz.getDeclaredConstructors())) I think this will allow us to not think of scenarios where someone sets up a class on the bootclasspath with a `@Deserializer` annotation. > if we do apply such restrictions, I would prefer to hardcode in serialization > instead of keeping the annotation and its scanning. That's going to be a much bigger change and I think we should avoid it. Having the current `@Deserializer` infrastructure in place is good. It's just that we want to restrict its usage to a very select few classes. ------------- PR Comment: https://git.openjdk.org/valhalla/pull/2661#issuecomment-5047187785
