This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko.git
The following commit(s) were added to refs/heads/main by this push:
new 3c5449c137 fix: don't dereference a null superclass in the protobuf
allow list check (#3509)
3c5449c137 is described below
commit 3c5449c13744bf5b5c85bb38c91b1aacabaa552b
Author: PJ Fanning <[email protected]>
AuthorDate: Mon Sep 7 07:53:33 2026 +0100
fix: don't dereference a null superclass in the protobuf allow list check
(#3509)
Motivation:
ProtobufSerializer.isInAllowListClassName reads clazz.getSuperclass.getName
unconditionally. getSuperclass is null for an interface, for Object and for
a primitive, and the class it checks is the manifest from the wire, so a
manifest naming an interface raised NullPointerException from the allow
list check rather than the IllegalArgumentException that refusing a class
is meant to produce.
isInAllowList also evaluated isBoundToProtobufSerializer first, which calls
serializerFor and raises, filling in a stack trace, for a class that is not
bound - the common case for a class allowed only by name.
Modification:
Skip the superclass when there is none, and test the name list before the
binding, which cannot throw. Both operands are pure predicates so the
decision is unchanged.
Result:
An interface or Object manifest is refused with the allow list error
instead of a NullPointerException.
---
.../remote/serialization/ProtobufSerializer.scala | 9 +++++++--
.../remote/serialization/ProtobufSerializerSpec.scala | 18 ++++++++++++++++++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git
a/remote/src/main/scala/org/apache/pekko/remote/serialization/ProtobufSerializer.scala
b/remote/src/main/scala/org/apache/pekko/remote/serialization/ProtobufSerializer.scala
index e2503d11bc..c45442cbec 100644
---
a/remote/src/main/scala/org/apache/pekko/remote/serialization/ProtobufSerializer.scala
+++
b/remote/src/main/scala/org/apache/pekko/remote/serialization/ProtobufSerializer.scala
@@ -181,7 +181,10 @@ class ProtobufSerializer(val system: ExtendedActorSystem)
extends BaseSerializer
* and still bind with the same class (interface).
*/
private def isInAllowList(clazz: Class[?]): Boolean = {
- isBoundToProtobufSerializer(clazz) || isInAllowListClassName(clazz)
+ // The name check comes first because it cannot throw:
`isBoundToProtobufSerializer` calls
+ // `serializerFor`, which raises (and fills in the stack trace of) a
NotSerializableException
+ // for a class that is not bound.
+ isInAllowListClassName(clazz) || isBoundToProtobufSerializer(clazz)
}
private def isBoundToProtobufSerializer(clazz: Class[?]): Boolean = {
@@ -194,8 +197,10 @@ class ProtobufSerializer(val system: ExtendedActorSystem)
extends BaseSerializer
}
private def isInAllowListClassName(clazz: Class[?]): Boolean = {
+ // getSuperclass is null for an interface, for Object and for a primitive,
and the manifest
+ // class comes off the wire, so it can be any of those
allowedClassNames(clazz.getName) ||
- allowedClassNames(clazz.getSuperclass.getName) ||
+ ((clazz.getSuperclass ne null) &&
allowedClassNames(clazz.getSuperclass.getName)) ||
clazz.getInterfaces.exists(c => allowedClassNames(c.getName))
}
}
diff --git
a/remote/src/test/scala/org/apache/pekko/remote/serialization/ProtobufSerializerSpec.scala
b/remote/src/test/scala/org/apache/pekko/remote/serialization/ProtobufSerializerSpec.scala
index 3b9885398e..9d79c066bc 100644
---
a/remote/src/test/scala/org/apache/pekko/remote/serialization/ProtobufSerializerSpec.scala
+++
b/remote/src/test/scala/org/apache/pekko/remote/serialization/ProtobufSerializerSpec.scala
@@ -123,6 +123,24 @@ class ProtobufSerializerSpec extends PekkoSpec(s"""
}
}
+ "reject an interface manifest rather than failing on its missing
superclass" in {
+ // getSuperclass is null for an interface, and the manifest class comes
off the wire.
+ // This used to raise NullPointerException from the allow list check
instead of the
+ // IllegalArgumentException that refusing the class is supposed to
produce.
+ val originalSerializer = ser.serializerFor(classOf[MyMessage])
+ intercept[IllegalArgumentException] {
+ ser.deserialize(Array[Byte](), originalSerializer.identifier,
classOf[Runnable].getName).get
+ }.getMessage should include("allow list")
+ }
+
+ "reject java.lang.Object as a manifest" in {
+ // Object.getSuperclass is null too
+ val originalSerializer = ser.serializerFor(classOf[MyMessage])
+ intercept[IllegalArgumentException] {
+ ser.deserialize(Array[Byte](), originalSerializer.identifier,
classOf[Object].getName).get
+ }.getMessage should include("allow list")
+ }
+
"allow deserialization of classes in configured allowed classes" in {
val originalSerializer = ser.serializerFor(classOf[MyMessage])
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
