Fix encoding/decoding of primitive string objects - #10
Merged
Conversation
TagReader now supports a `unwrap_js_primitive_objects` option
(defaulting to True) which allows primitive values in wrapper objects
to be kept as wrapped after decoding. (e.g. `new String("example")` in
JavaScript or `v8serialize.jstypes.JSPrimitiveObject("example")` in
Python).
Previously wrapped primitives were always unwrapped when decoding, so
they worked transparently like non-wrapped values. However, when doing
round-trip encoding/decoding, wrapped primitives need to be kept as-is
to retain identity. (Our JSPrimitiveObject is not `==` to an unwrapped
version, just as in Javascript `"foo" === new String("foo")` is false.)
The hypothesis strategies that generated values for the V8 round trip tests were mistakenly not including JSPrimitiveObject values, so we weren't testing that wrapped primitive values were being round-tripped from us to V8 and back.
This explicitly reproduces and can remain as a regression test for issue #8.
We were encoding/decoding primitive-object-wrapped string values
according to an old (< 12) version of the V8 serialization format,
instead of the >= 13, <=15 version range we support.
The older format always encoded wrapped strings as a UTF-8 string,
omitting the `SerializationTag.kUtf8String` tag.
The current format instead encodes the wrapped
string as any of the primitive string
representations, preceded by the appropriate type tag.
BREAKING CHANGE:
1. The ReadableTagStream.read_js_primitive_object() function now
requires a ctx: DecodeContext argument.
2. primitive-object-wrapped strings (represented by
`v8serialize.jstypes.JSPrimitiveObject`) that were encoded by
previous versions of v8serialize will not be decodable by this
version.
This should only be observable if a program was explicitly encoding
JSPrimitiveObject objects and persisting them to be loaded by this
version, because:
- v8serialize could not have decoded wrapped strings from a real V8
implementation, because of the incorrect decoding behaviour.
- v8serialize unwraps JSPrimitiveObject to the primitive value when
decoding, so even if it had decoded a wrapped string, it'd be
re-encoded as an unwrapped string if round-tripped.
h4l
force-pushed
the
issue-8-fix-jsprimitivetype-string
branch
from
September 26, 2025 08:47
e1c1b72 to
da10d47
Compare
h4l
force-pushed
the
issue-8-fix-jsprimitivetype-string
branch
from
September 26, 2025 09:05
da10d47 to
d471f9d
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
1 participant
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.
This fixes #8.
This MR fixes the encoding/decoding of wrapped/boxed string values (JavaScript
new String("x")/ PythonJSPrimitiveObject("x")). They were mistakenly using an older representation of the V8 serialization format, which stored them as raw length-prefixed UTF-8 values without the normalSerializationTag.kUtf8String. This was used before version 12 of the format, which we don't support. This resulted in wrapped strings failing to decode when received from a real V8 implementation, and equally ours would fail to be decoded by V8.The automated integration tests which round-trip through several external implementations of the format failed to detect the error, because the property test scenarios generating examples to verify were mistakenly not generating wrapped primitive objects.
We now use the correct, current encoding and include wrapped primitive objects in the round-trip integration tests.
The API also gained a new feature, to allow wrapped primitive objects to be retained in decoded values. This is disabled by default, but necessary to round-trip values containing wrapped primitive objects.