Repository navigation
fix: make serializer id 0 reject non-empty bytes and manifests - #3751
Merged
pjfanning merged 2 commits intoNov 6, 2026
Merged
Conversation
pjfanning
changed the title
He-Pin
approved these changes
Nov 6, 2026
He-Pin
pushed a commit
that referenced
this pull request
Nov 6, 2026
Motivation: sbt/setup-sbt v1.5.13 fetches its signature verification policy from the default branch of carabiner-dev/policies. That policy now has a meta.name field the ampel version used by the action can't parse, so any job that misses the sbt download cache fails in Install sbt with "compiling policy: unable to parse a policy, group or policySet from data". Modification: Update all sbt/setup-sbt references to v1.5.14, which pins the policy to a compatible commit. v1.5.14 is on the ASF allow list. Result: Jobs can install sbt on a cache miss again. Tests: - Not run locally - CI runs the updated workflows on this PR References: Refs #3751, Refs #3753
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are reported for the serializer id 0 validation changes.
No open issues 🎉
What changed in this PR
Tightens serializer id 0 validation so malformed serialized records fail instead of silently becoming null, while preserving #3495’s avoidance of manifest class loading.
Changes:
- Rejects non-empty bytes and non-empty string manifests for
NullSerializerwithNotSerializableException. - Adds regression tests for valid null round trips and invalid inputs through byte-array and byte-buffer APIs.
- Documents serializer id 0’s expected representation and rejection behavior.
| File | Description |
|---|---|
| docs/src/main/paradox/serialization.md | Documents id 0 validation and malformed-record behavior. |
| actor/src/main/scala/org/apache/pekko/serialization/Serializer.scala | Rejects non-empty input in NullSerializer. |
| actor/src/main/scala/org/apache/pekko/serialization/Serialization.scala | Rejects non-empty manifests without class loading. |
| actor-tests/src/test/scala/org/apache/pekko/serialization/SerializeSpec.scala | Tests null round trips and rejection through both deserialization APIs. |
Review effort: Balanced
Motivation: Since apache#3495, Serialization.deserialize(bytes, 0, manifest) with a non-empty manifest returns Success(null), whatever the bytes contain. Id 0 is a common default for a missing int value, so a corrupt stored record can load as a null payload instead of failing. Modification: NullSerializer.fromBinary throws NotSerializableException for non-empty input. toBinary only ever produces an empty array, so such input cannot have come from this serializer. Document the behaviour of serializer id 0. Result: Deserializing non-empty bytes with serializer id 0 fails, with an empty or non-empty manifest. Empty bytes still deserialize to null, and apache#3495 stays in place with no class loading added. Tests: - sbt "actor-tests/testOnly org.apache.pekko.serialization.*": 171 passed (the new rejection test failed before the fix) - sbt "remote/testOnly ...PekkoProtocolSpec ...ActorSelectionQueueDistributionSpec": 25 passed - sbt actor/mimaReportBinaryIssues: no issues - docs/paradox: not run locally, left to CI References: Fixes apache#3749
Motivation: Since apache#3495, Serialization.deserialize(empty, 0, "not-a-class") returns Success(null). 2.0.0-M4 failed with NotSerializableException. A real serialized null always has an empty manifest, so this input cannot have come from a conforming sender. Modification: Serialization.deserializeByteArray throws NotSerializableException for NullSerializer with a non-empty manifest. The check only tests whether the manifest is empty, so apache#3495 stays in place and nothing is loaded. Result: An unresolvable manifest with serializer id 0 fails as it did in 2.0.0-M4. A loadable manifest also fails, where 2.0.0-M4 returned Success(null), because matching that would need the class lookup that apache#3495 removed. Tests: - sbt "actor-tests/testOnly org.apache.pekko.serialization.*": 172 passed (the new test failed before the fix) - sbt actor/mimaReportBinaryIssues: no issues - docs/paradox: not run locally, left to CI References: Fixes apache#3752
pjfanning
force-pushed
the
fix-null-serializer-non-empty-input
branch
from
November 6, 2026 18:32
21fb4b2 to
e69cbc0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Since #3495,
Serialization.deserialize(bytes, 0, manifest)with a non-empty manifest returnsSuccess(null), whatever the bytes contain. Id 0 is a common default for a missing int value, so a corrupt stored record can load as anullpayload instead of failing, and callers that rely on a failure to fall back never get one.Modification
NullSerializer.fromBinarythrowsNotSerializableExceptionfor non-empty input.toBinaryonly ever produces an empty array, so such input cannot have come from this serializer.Serialization.deserializeByteArraythrowsNotSerializableExceptionforNullSerializerwith a non-empty manifest.Serializers.manifestForalways gives a serializednullan empty manifest. The serializer never sees the manifest, so this check has to be inSerialization.serialization.md.Result
"not-a-class"manifestFailureSuccess(null)Failure""manifestSuccess(null)Success(null)Failure"not-a-class"manifestFailureSuccess(null)FailureSuccess(null)Success(null)Failure""manifest (a real serializednull)Success(null)Success(null)Success(null)(unchanged)Every
Failureabove is aNotSerializableException, anddeserializeByteBufferthrows the same exception in those cases.#3495 stays in place and no class loading is added: the new check only tests whether the manifest is empty.
Two rows are stricter than 2.0.0-M4, so anything relying on them now gets a failure, which needs a line in the 2.0.1 release notes:
Tests
sbt "actor-tests/testOnly org.apache.pekko.serialization.*": 172 passed. Both new rejection tests failed before their fixes.sbt "remote/testOnly org.apache.pekko.remote.classic.transport.PekkoProtocolSpec org.apache.pekko.remote.artery.ActorSelectionQueueDistributionSpec": 25 passed (these build messages with serializer id 0). Run before the manifest check was added, not re-run after.sbt actor/mimaReportBinaryIssues: no issues.scalafmt --mode diff-ref=upstream/mainandgit diff --check: run.sbt docs/paradox: not run locally, left to CI.References
Fixes #3749
Fixes #3752