Skip to content

Fix unflatten crashing on keys that contain a dollar sign#11

Open
c-tonneslan wants to merge 1 commit into
simonw:mainfrom
c-tonneslan:fix/unflatten-keys-with-dollar-signs
Open

Fix unflatten crashing on keys that contain a dollar sign#11
c-tonneslan wants to merge 1 commit into
simonw:mainfrom
c-tonneslan:fix/unflatten-keys-with-dollar-signs

Conversation

@c-tonneslan

Copy link
Copy Markdown

Found this while reading the code. Round-tripping a dict whose key has a $ in it crashes:

>>> from json_flatten import flatten, unflatten
>>> unflatten(flatten({"foo$bar": 5}))
Traceback (most recent call last):
  ...
ValueError: too many values to unpack (expected 2)

flatten does the right thing and produces {"foo$bar$int": "5"}, but unflatten then calls rsplit("$", 2) and gets back three pieces (["foo", "bar", "int"]) instead of the two the unpack expects. The greedy split was the only issue; rsplit("$", 1) keeps the type suffix at the end and leaves the rest of the key intact, so "foo$bar$int" becomes ("foo$bar", "int") and the round-trip works.

Added two parametrized regression cases. The existing dollar_signs_that_are_not_type_indicators case keeps passing because its value is a plain string, so no $type is ever appended.

Round-tripping a dict like `{"foo$bar": 5}` blows up:

    >>> from json_flatten import flatten, unflatten
    >>> unflatten(flatten({"foo$bar": 5}))
    ValueError: too many values to unpack (expected 2)

flatten produces `{"foo$bar$int": "5"}` correctly, but unflatten then
calls `rsplit("$", 2)` and gets back three pieces instead of two. The
greedy split was the only thing wrong; a `rsplit("$", 1)` keeps the
type suffix and leaves the rest of the key alone, so "foo$bar$int"
becomes ("foo$bar", "int"). Added two regression cases to the
parametrized test.

Signed-off-by: Charlie Tonneslan <cst0520@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant