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])