Repository navigation
Conversation
seperman
reviewed
Oct 15, 2026
seperman
left a comment
Member
There was a problem hiding this comment.
Static review: #640 Fix paths for bytes dictionary keys
Summary: stringify_element now writes bytes keys as Python bytes literals (repr) instead of hitting "'" in b'...', which raised a TypeError. The path tokenizer now honours backslash escapes inside b'...'/b"...", so these paths round-trip through parse_path, extract and Delta. This fixes the crash reported in #472, which still happens on dev whenever a path under a bytes key is built, e.g. DeepDiff({b'foo': 1}, {b'foo': 2}).
Overlap with other PRs
- #614 (closed without merging, targeted
master) only addedrepr()forbytes/memoryviewinstringify_element. That alone breaks round-tripping for keys that contain quotes or backslashes. This PR includes that idea and adds the parser side, so it covers more than #614 did. - I found no other open or merged PR touching
path.py/stringify_element, and nothing ondevaddresses #472.
Security
Path strings can come from untrusted Delta files, so I checked whether the new parsing widens what a path can reach:
literal_evalwas already applied to most path elements ondev. It is now also applied to elements that start withb'/b"and contain a backslash (deepdiff/path.py:61). The tokenizer hands an element to_add_to_elementsas soon as its closing quote is seen (path.py:111-123). So such an element is either one complete bytes literal, which evaluates tobytes, or an unterminated one:literal_evalfails and it stays a plainstr, as ondev.literal_evalcannot execute code.- The only new element type is
bytes, and bytes cannot act as attribute names:getattr/setattrwith a bytes name raise TypeError. Thesetattr(prev_obj, str(prev_elem), ...)fallback (path.py:217) turns bytes into"b'...'", which can never be a dunder. So the GETATTR surface is unchanged. - The escape handling only changes state while inside a quoted bytes literal. At worst, a crafted escape merges the rest of the path into a single GET element. It cannot create new GETATTR segments.
check_elemstill runs on every final element in_get_nested_obj,_get_nested_obj_and_forceand Delta's_get_elements_and_details/_get_elem_and_compare_to_old_value. - The added code is a linear character scan with no regex, imports or I/O. The only non-ASCII character in the diff is the existing
𝆺𝅥𝅯marker (U+1D1C0); there are no bidi or zero-width characters.
No security concerns found in static review.
Correctness
path.py:325-326: the bytes short-circuit sits before theinchecks that raised.repralready chooses the quote style and escapes, so no further quoting is needed.path.py:118:inside_bytesis computed from the text collected before the opening quote. DeepDiff always rendersstrdict keys starting with'or", so string keys such as"b'foo'"are not mistaken for bytes.test_string_key_path_round_tripcovers this.stringify_path(path.py:365-368) still formats bytes elements through the f-string, i.e.str(bytes). The output happens to matchrepr, but underpython -b/-bbit emits or raisesBytesWarning. Delta usesstringify_pathfor flat rows and error messages.memoryviewkeys still produce a path that doesn't round-trip (<memory at 0x…>). As the description says, that is out of scope.
Tests
test_bytes_key_paths(tests/test_path.py:94) has 33 cases: added, removed and changed, with empty keys, both quote types, trailing and doubled backslashes, non-ASCII bytes, and]/.inside keys. Each case checks the reported path,parse_path,stringify_path,extractand bidirectional Delta. Readingdev, these should all fail with the TypeError while the path is built.test_string_key_path_round_trip(tests/test_path.py:126) should already pass ondev. It is a useful guard against the new bytes handling catching string keys.
Suggestions
- In
stringify_path, route bytes throughstringify_elementtoo (e.g.isinstance(element, (str, bytes))), so model.py and Delta format bytes the same way and avoidBytesWarningunder-b. - Optionally add a test showing that an unterminated or malformed bytes literal followed by
.attrparses into one GET element, not a GETATTR. That would pin down the security property above.
AI-assisted static review; the code was not executed.
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.
DeepDiff({b"foo": 1}, {b"foo": 2})raisesTypeErrorwhile building the path for the changed value. The original example in #472 can appear to work now because the default dictionary threshold reports the whole dictionary as changed; comparing values under the same bytes key still reaches the broken path handling.This reports the change at
root[b'foo']and keeps the resulting paths usable byparse_path,extract, andDelta. Bytes keys are written as Python bytes literals, and the parser handles their escaped quotes and backslashes. The regression cases cover nested additions, removals, and value changes with empty keys, both quote types, trailing backslashes, and non-ASCII bytes, including forward and reverse Delta application.This builds on the bytes-literal approach proposed by Mukller in #614, with the parsing changes needed to make escaped keys round-trip. Memoryview behavior is unchanged.
Validation on Windows with Python 3.12.14:
dev.test_delta_dump_and_read3. The same Windows text-mode pickle failure reproduces on unchangeddev.Fixes #472.