Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG - hash ignores empty list fields in dictionary #33

Closed
NiroHaim opened this issue May 2, 2023 · 1 comment · Fixed by #35
Closed

BUG - hash ignores empty list fields in dictionary #33

NiroHaim opened this issue May 2, 2023 · 1 comment · Fixed by #35
Assignees

Comments

@NiroHaim
Copy link

NiroHaim commented May 2, 2023

Hi!
First off, thank you for the awesome lib! Really helped me out when I couldn't finding anything else that could really hash a JSON properly.

I've encountered a bug in the usage of the lib
When running the code snippet below, The received output is True which clearly shouldn't be the case.

import json
import json_fingerprint

hash1 = json_fingerprint.create(
    input=json.dumps({'field1': 'yes'}),
    hash_function='sha256',
    version=1
)

hash2 = json_fingerprint.create(
    input=json.dumps({'field1': 'yes', 'field2': []}),
    hash_function='sha256',
    version=1
)

print(hash1 == hash2)  # True

After a short debug session, I found that the problem lies in the recursive _flatten_json function. This unexpected behavior happens because the function loops the elements of a dict/list recursivley and hashes its inner elements.
When the dict/list is empty, an empty list is returned and in the logic, the out result is extended which does nothing when handling an empty list.
This means that fields of empty dicts or lists in the desired hashing object are being ignored.
I'd be happy to provide more insight if needed to further a solution to the bug :)

Thanks!

@cobaltine
Copy link
Owner

cobaltine commented May 9, 2023

Thank you @NiroHaim for reporting the issue and providing a thorough explanation -- as well as the code snippet!

I'll soon have a look at the issue in more detail, build a unit test for it, and deploy a fix. This project has been dormant for quite some time so fixing this issue means some long overdue refactoring of the ci/cd pipeline just needs to be done prior to that.

@cobaltine cobaltine self-assigned this May 12, 2023
cobaltine added a commit that referenced this issue May 12, 2023
Empty lists and empty dicts were not previously considered to be values.
This caused the entire path to an empty list or dict to be ignored.

JSON objects with or without empty list or dict values could cause jfpv1
hash collisions with similar objects prior to version `0.12.2`. Any
release beyond `v0.12.x` should be considered backward-incompatible.

See the following issue for more details:
#33
cobaltine added a commit that referenced this issue May 12, 2023
Empty lists and empty dicts were not previously considered to be values.
This caused the entire path to an empty list or dict to be ignored.

JSON objects with or without empty list or dict values could cause jfpv1
hash collisions with similar objects up to version `0.12.2`. Any release
beyond `v0.12.z` should be considered backward-incompatible for cases
where empty lists or dicts are present.

See the following issue for more details:
#33
@cobaltine cobaltine linked a pull request May 12, 2023 that will close this issue
cobaltine added a commit that referenced this issue May 13, 2023
Empty lists and empty dicts were not previously considered to be values.
This caused the entire path to an empty list or dict to be ignored.

JSON objects with or without empty list or dict values could cause jfpv1
hash collisions with similar objects up to version `0.12.2`. Any release
beyond `v0.12.z` should be considered backward-incompatible for cases
where empty lists or dicts are present.

See the following issue for more details:
#33
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 a pull request may close this issue.

2 participants