Repository navigation
fix: REDUCE-created instances re-encode with REDUCE (#32) - #56
Merged
Merged
Conversation
…32) `InstanceData` gains `newobj` (true for `NEWOBJ`, the default everywhere). The `BUILD` arm sets it from the popped `Reduce` when the arguments are empty, so `GLOBAL () REDUCE state BUILD` and `GLOBAL () NEWOBJ state BUILD` no longer collapse into the same node; a second `BUILD` keeps the flag. Nothing reads it yet. Assisted-by: Claude Fable 5.1
…JSON (#32) Both JSON writers add `"@newobj": false` next to `@cls`/`@s` when the instance was created by calling the class; the JSON reader and the Python dict reader take an absent key (stored data) or any value but `false` as `NEWOBJ`. Both pickle encoders emit `REDUCE` for the false kind, items before `BUILD` as before; the direct dict encoder routes such dicts through the `PickleValue` path. The 1,692-record sample produces identical PG JSON, dict-path output and re-encoded pickles (no such instance in it). Assisted-by: Claude Fable 5.1
…nts (#32) Fresh review findings: the Python-dict readers took any dict of more than four keys for a plain dict, and `@newobj` makes an instance with `@items` and `@appends` five keys, so such an object came back as a dict of five string keys on the dict path and the record path (silent). Both guards now allow five keys, the test pushes the shape through `dict_to_pickle` and `encode_zodb_record`. Every reader rejects `@newobj`, `@items` or `@appends` on a bare class reference (the JSON reader used to drop them). Minor: stale encoder comments, the docs name the callable and the opposite default of the `@reduce` key, the dict path's non-bool flag handling and a PG writer case are tested, "unpickling" is in the spell-check vocabulary. Assisted-by: Claude Fable 5.1
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.
Fixes #32
An instance that a pickler wrote as
GLOBAL () REDUCE state BUILD(a__reduce__returning(cls, (), state), for exampleOrderedDictsubclasses with a__dict__, or a module-level factory function with()and state) was re-encoded asNEWOBJ + BUILD, socls.__new__(cls)ran instead ofcls()and__init__was skipped; a factory function even failed to load (NEWOBJ class argument must be a type).What changes
InstanceDatagainsnewobj(true forNEWOBJ, the default everywhere; the decoder'sBUILDarm takes it from the poppedREDUCE/NEWOBJ, a secondBUILDkeeps it)."@newobj": falsenext to@cls/@s, written only for theREDUCEkind; absent meansNEWOBJ, so stored data is unaffected. Documented in the format reference, with the note that@reduce'snewobjkey has the opposite default (each absent value is the common case of its form).REDUCEfor the false kind, items beforeBUILDas before; the direct Python-dict encoder routes such dicts through thePickleValuepath.@cls,@s,@newobj,@items,@appends; the oldlen > 4guard would have pickled such an object as a plain dict of five strings), and every reader rejects@newobj,@itemsor@appendson a bare class reference without@s(the JSON reader used to drop them silently).Evidence
__init__calls, check opcode order withpickletoolsand cover the five-key shape, non-bool flag values and the rejected shapes.REDUCE-kind instance in it).__getnewargs__, dict/list/OrderedDictsubclasses with and without__dict__, both-items reduce, persistent refs, shared objects, uuid,Persistent,OOBTree) through every path at protocols 2 and 3 againstpickle.loadsof the original, plus 22 stored-JSON inputs: every shape that must not change is byte-identical tomain; the fourREDUCE-kind shapes are fixed. Its one Important finding (the five-key guard) is the last commit.Assisted-by: Claude Fable 5.1