pjfanning opened a new pull request, #3509: URL: https://github.com/apache/pekko/pull/3509
### Motivation This is the `pekko-remote` half of the allow list ordering noted in #3503, and looking at it turned up something more than a reorder. `ProtobufSerializer.isInAllowListClassName` (`:196-200`) reads `clazz.getSuperclass.getName` unconditionally. `getSuperclass` is `null` for an interface, for `java.lang.Object` and for a primitive, and the class being checked is the manifest that arrived on the wire — so a manifest naming an interface raises `NullPointerException` out of the allow list check instead of the `IllegalArgumentException` that refusing a class is supposed to produce. It is reachable whenever the named class is not bound to a protobuf serializer, which is exactly the case the allow list exists to refuse. Separately, `isInAllowList` (`:183-185`) evaluated `isBoundToProtobufSerializer` first. That calls `serializerFor`, which raises `NotSerializableException` — stack trace and all — for a class that is not bound, which is the common case for a class allowed only by `pekko.serialization.protobuf.allowed-classes`. ### Modification Skip the superclass when there is none, and test the name list before the binding lookup, which cannot throw. Both operands are pure predicates, so which one runs first does not change the decision. The ordering matters less here than in Jackson: `ProtobufSerializer` caches its parsing handle after the check (`:124-133`), so it pays the cost once per class rather than once per message. The null dereference is the part worth fixing. ### Result An interface or `Object` manifest is refused with the allow list error rather than a `NullPointerException`. ### Tests - `sbt "remote/testOnly org.apache.pekko.remote.serialization.*"` — 196 passed, 1 pending Two new tests in `ProtobufSerializerSpec`, both checked to discriminate by reverting the production file and re-running — each then fails with `NullPointerException was thrown`: - `reject an interface manifest rather than failing on its missing superclass` - `reject java.lang.Object as a manifest` - `sbt "remote/mimaReportBinaryIssues"` — no issues - `sbt "remote/scalafmtCheckAll" headerCreateAll` — clean ### References Completes the `isInAllowList` ordering from #3503, which changed the two Jackson serializers and deliberately left this one out of a Jackson-scoped PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
