pjfanning opened a new issue, #3752:
URL: https://github.com/apache/pekko/issues/3752
### Summary
`Serialization.deserialize(Array.empty[Byte], 0, "not-a-class")` returns
`Success(null)`, even though a conforming sender never pairs serializer id 0
with a manifest.
This is the case left open by #3751, the fix for #3749. That PR makes
`NullSerializer.fromBinary` reject non-empty bytes. It can't cover this case,
because the serializer never sees the manifest.
### Details
`NullSerializer` declares `includeManifest = false`, so since #3495
`Serialization.deserializeByteArray` ignores any manifest and calls
`fromBinary(bytes, None)`. With empty bytes, that returns `null`.
A real serialized `null` always has an empty manifest:
`Serializers.manifestFor` returns `""` for a serializer with `includeManifest =
false`. So id 0 with a non-empty manifest can't have come from a conforming
sender.
| Pekko version | Call | Result |
|---|---|---|
| 2.0.0-M4 | `deserialize(empty, 0, "not-a-class")` |
`Failure(NotSerializableException("Cannot find manifest class ..."))` |
| 2.0.0, and with #3751 | `deserialize(empty, 0, "not-a-class")` |
`Success(null)` |
A stored record could end up in this state if its serializer id is missing
and reads as 0 while its payload is also empty. This is narrower than #3749,
because it needs an empty payload as well as a missing id. I'm not aware of it
affecting production data.
### Reproducer
Derived from reading the source. I have not run this snippet in isolation.
```scala
val serialization = SerializationExtension(system)
serialization.deserialize(Array.empty[Byte], 0, "not-a-class")
// 2.0.0-M4: Failure(NotSerializableException)
// 2.0.0 and #3751: Success(null)
```
### Possible ways forward
Both of these keep #3495 in place:
1. **Reject a non-empty manifest for `NullSerializer` in `Serialization`.**
No class loading is needed, because the check only looks at whether the
manifest is empty. `deserializeByteArray` would need a special case for this
serializer, and the `deserialize` and `deserializeByteBuffer` paths should both
be covered.
2. **Leave it as it is and document it.** The payload is empty, so no data
is discarded, and the extra special case may not be worth it.
Other serializers with `includeManifest = false` are out of scope. Ignoring
an unresolvable manifest is the stated intent of #3495.
### References
- #3749
- #3751
- #3495
--
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]
