Fix ByteString1C serialization to use SerializationProxy#2929
Merged
pjfanning merged 2 commits intoapache:mainfrom Apr 28, 2026
Merged
Conversation
ByteString1C lacked a writeReplace() method, causing it to be serialized directly (using @serialversionuid(3956956327691936932L)) rather than via the SerializationProxy used by ByteString1 and ByteStrings. This led to SerialVersionUID mismatches when deserializing ByteString1C (and all CompactByteString instances, which are backed by ByteString1C). Add writeReplace() to ByteString1C so all three concrete ByteString types consistently route through SerializationProxy on serialization. Add three new tests: - Verify ByteString1C serialized bytes do not contain the class name "ByteString1C" (i.e. the proxy is used, not direct serialization) - Verify CompactByteString.apply() instances also serialize via proxy - Verify round-trip serialization for all three concrete types Agent-Logs-Url: https://github.com/pjfanning/incubator-pekko/sessions/a999ad23-eb25-41ef-b082-8e78f5b9353b Co-authored-by: pjfanning <11783444+pjfanning@users.noreply.github.com>
He-Pin
approved these changes
Apr 28, 2026
Member
He-Pin
left a comment
He-Pin
left a comment
There was a problem hiding this comment.
LGTM. Reviewed the ByteString1C serialization proxy hook against the existing ByteString1/ByteStrings serialization path and ran local ByteString serialization tests.
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
ByteString1C— the sole concrete implementation ofCompactByteString— lacked awriteReplace()method. This caused Java serialization to serialize it directly using@SerialVersionUID(3956956327691936932L)rather than delegating to theSerializationProxythat bothByteString1andByteStringsuse. Any change to the class structure (e.g. moving to a different package) would make previously serializedByteString1Cdata unreadable due toSerialVersionUIDmismatches. AllCompactByteStringfactory methods (CompactByteString.apply(...),CompactByteString.fromArray(...), etc.) returnByteString1C, so all of them were affected.Modification
Add
protected def writeReplace(): AnyRef = new SerializationProxy(this)toByteString1C, matching the pattern already present inByteString1andByteStrings. All infrastructure (byteStringCompanion,writeToOutputStream,SerializationIdentity) was already in place onByteString1C; the only missing piece was the hook.Result
All three concrete
ByteStringtypes now consistently serialize viaSerializationProxy. The serialized form no longer embeds theByteString1Cclass descriptor, making the format resilient to class renaming/moving.Tests
ByteString1C serializes via SerializationProxy not directly— asserts the raw serialized bytes do not contain the string"ByteString1C"(i.e. the proxy class descriptor is written, not the raw implementation class).CompactByteString instances serialize via SerializationProxy— same assertion forCompactByteString.apply()andCompactByteString("hello")factories.each ByteString concrete type round-trips via serialization— explicit round-trip forByteString1C,ByteString1, andByteStrings.Tests: run locally not possible (sbt not installed in sandbox). Tests added and reviewed; CI will validate.
References
None - serialization proxy consistency fix for ByteString1C / CompactByteString