pjfanning opened a new issue, #3749:
URL: https://github.com/apache/pekko/issues/3749
### Summary
Since #3495, `Serialization.deserialize(bytes, 0, manifest)` with a
non-empty manifest returns `Success(null)`, whatever the bytes contain. Before
#3495 it failed with `NotSerializableException` unless the manifest happened to
name a loadable class.
A corrupt persisted record that resolves to serializer id 0 can therefore
load as a `null` payload instead of failing. Callers that rely on a failure to
fall back to something else never get one.
**#3495 should not be reverted.** It removes a class-loading oracle and
unbounded metaspace growth driven by wire-supplied manifests. This issue is
about the narrower `NullSerializer` consequence.
### Details
`NullSerializer` has id 0, declares `includeManifest = false`, and is always
registered. Its `fromBinary` ignores its input and returns `null`.
For a manifest that isn't a loadable class:
| Pekko version | Call | Result |
|---|---|---|
| 2.0.0-M4 | `deserialize(garbage, 0, "not-a-class")` |
`Failure(NotSerializableException("Cannot find manifest class ..."))` |
| 2.0.0 | `deserialize(garbage, 0, "not-a-class")` | `Success(null)` |
The same change applies to other serializers with `includeManifest = false`,
such as `ByteArraySerializer` and custom ones. They now ignore an unresolvable
manifest, which is the stated intent of #3495. `NullSerializer` stands out
because the result is `null` and the input is discarded.
The empty-manifest path is not new: `deserialize(garbage, 0, "")` already
returned `Success(null)` before #3495. #3495 widened that existing behaviour to
non-empty manifests.
Id 0 is easy to reach by accident. It's the default for a missing int: for
example, the Cassandra driver's `Row.getInt` returns 0 for a null or empty
column.
### How this was found
Updating pekko-persistence-cassandra to Pekko 2.0.0 made two
`CassandraSnapshotStoreSpec` tests fail: apache/pekko-persistence-cassandra#521.
The root cause there is a bug in the spec, which binds `ser_id` and
`ser_manifest` in the wrong order. The stored row therefore had an empty
`ser_id` and a garbage manifest, and the tests only passed because of the old
manifest lookup failure. On 2.0.0 the snapshot store received
`SelectedSnapshot(metadata, null)` rather than a failure, so it did not fall
back to an older snapshot.
The spec is being fixed in that PR, so this does not block
pekko-persistence-cassandra. The plugin always writes a real `ser_id` itself,
so I'm not aware of this affecting production data. Other plugins or user code
reading stored serializer ids could be exposed in the same way.
### Reproducer
Derived from reading the source and the CI failure above. I have not run
this snippet in isolation.
```scala
val serialization = SerializationExtension(system)
serialization.deserialize("garbage".getBytes(StandardCharsets.UTF_8), 0,
"not-a-class")
// 2.0.0-M4: Failure(NotSerializableException)
// 2.0.0: Success(null)
```
### Possible ways forward
These all keep #3495 in place:
1. **Make `NullSerializer.fromBinary` reject non-empty input (suggested).**
`toBinary` only ever produces an empty array, so non-empty bytes can't have
come from a conforming sender. Throwing `NotSerializableException` restores a
failure for this case without any class loading, and also closes the existing
empty-manifest path. The behaviour change for anything relying on the current
leniency would need a note.
2. **Document it only.** Add a release/migration note that deserializing
with an `includeManifest = false` serializer no longer fails on an unresolvable
manifest, and that id 0 yields `null`.
### References
- #3495 (`cffd027b89`), first released in 2.0.0-RC1 and 2.0.0
- #3478
- apache/pekko-persistence-cassandra#521
--
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]
