h2non / jsonpath-ng

Finally, a JSONPath implementation for Python that aims to be standard compliant. That's all. Enjoy!
Apache License 2.0
572 stars 85 forks source link

Fix issue with assignment in case root element is a list. #109

Closed rolling-robot closed 11 months ago

rolling-robot commented 2 years ago

This is a fix for https://github.com/h2non/jsonpath-ng/issues/108

michaelmior commented 11 months ago

@rolling-robot Would you be able to add a test for this new case?

rolling-robot commented 11 months ago

Here you go. An error message with older clean algorithm

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ jsonpath_ng/jsonpath.py:295: in update_or_create
    return _clean_list_keys(data)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
dict_ = [{'foo': 1}, {'bar': 2, 'foo': 42}]

    def _clean_list_keys(dict_):
        """
        Replace {LIST_KEY: ['foo', 'bar']} with ['foo', 'bar'].

        >>> _clean_list_keys({LIST_KEY: ['foo', 'bar']})
        ['foo', 'bar']

        """
>       for key, value in dict_.items():
E       AttributeError: 'list' object has no attribute 'items'

jsonpath_ng/jsonpath.py:809: AttributeError
rolling-robot commented 11 months ago

Older version implicitly suggests that root element is a dict, but it may not.

rolling-robot commented 11 months ago

BTW, upon looking at original code now, a year later, I started to doubt: although everything works, this may be a wrong fix for this bug. It may be intended by original author of this code that library-internal representation for list is dictionary with special structure {LIST_KEY: [.......]}, so zero-th level in the tree MUST be a dict type, but somehow _create_list_key() is not run for 0-th level in the first place, so bug manifests as an exception in _clean_list_keys . I did not dig into implementation and philosophy of structure representation of jsonpath-ng that deep.

rolling-robot commented 11 months ago

No, it seems like the bugfix is safe and does not break conventions. _create_list_key() is used twice in the whole repo, and both times its argument is an empty dictionary. So IMHO LIST_KEY hack looks like a generic way to allow assignment or access to uninitialized or empty element be it either a list, or a dict in more or less uniform way.

michaelmior commented 11 months ago

@rolling-robot Thanks for the fix!