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

Quality issue: permutations produce the same hashes? #44

Closed
Dherse opened this issue Dec 18, 2023 · 4 comments · Fixed by #45 or #48
Closed

Quality issue: permutations produce the same hashes? #44

Dherse opened this issue Dec 18, 2023 · 4 comments · Fixed by #45 or #48
Assignees
Labels
bug Something isn't working

Comments

@Dherse
Copy link
Contributor

Dherse commented Dec 18, 2023

I have noticed a strange behavior which I am not sure whether it is intended or not:

The hashes of two u8 a and b are the same regardless of order:

  • (a, b).hash(&mut state)
  • (b, a).hash(&mut state)

This is an issue in my particular use case because it prevents me from distinguishing between two distinct cases only by the hash.

I don't know whether this is actually a bug or no as I am a total noob about cryptography and hashing functions.

Thanks for you help/advice,

Dherse

@ogxd
Copy link
Owner

ogxd commented Dec 18, 2023

Hello @Dherse,

The "standard" hash function gxhash64(input: &[u8], seed: i64) -> u64 is backed by smasher tests which has permutation tests and more. However, the Hasher is a different approach, and while it uses the core of the gxhash algorithm, it is not backed by smasher and is covered by very few tests regarding quality (only a few sanity checks).

Given that the rust's default Hasher seems to generate order-dependent hashes, I'd say that the fact that GxHasher is currently order-independent is a bug (I easily reproduced).

Fixing this will only affect GxHasher so I think it can be addressed as part of a hotfix version, I'll see what can be done. Also, some time must be put into adding a few more quality tests on this Hasher side.

@ogxd ogxd self-assigned this Dec 18, 2023
@ogxd ogxd added the bug Something isn't working label Dec 18, 2023
@Dherse
Copy link
Contributor Author

Dherse commented Dec 18, 2023

Thank you, I am a large contributor to https://github.com/typst/typst and I am trying to see whether gxhash could replace Siphash 1-3 that we're currently using since it has so much higher performance hence #7 and this issue!

Anyway, looking forward to it 🎉

@ogxd ogxd linked a pull request Dec 18, 2023 that will close this issue
4 tasks
@ogxd ogxd closed this as completed in #45 Dec 22, 2023
@ogxd ogxd reopened this Dec 22, 2023
@ogxd
Copy link
Owner

ogxd commented Dec 22, 2023

My bad, issue is still open, I only tackled the quality bench part for now

@ogxd ogxd linked a pull request Dec 22, 2023 that will close this issue
@ogxd ogxd closed this as completed in #48 Dec 23, 2023
@ogxd
Copy link
Owner

ogxd commented Dec 23, 2023

Fixed and published in v2.3.1 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants