search

yomorun / hashids-java

Hashids algorithm v1.0.0 implementation in Java
http://hashids.org
MIT License
1.02k stars 156 forks source link

Data lost when using decodeHex #58

Closed sharebear closed 5 years ago

sharebear commented 5 years ago

In our project we wanted to identify if a given string was a valid HashId. The first version of this utilised decodeHex and then tested to see if the resulting string was empty or not. This validation did not work for values under 16 thus leading me in the direction of this strange behaviour of decodeHex.

Looking at the implementation the problem seems to be this call to substring https://github.com/sharebear/hashids-java/blob/master/src/main/java/org/hashids/Hashids.java#L209

As far as I can tell this has been this way since it was introduced in 43b258a7b9121046b18f2fe27fbfddd12bbb43cb what is it's intended usage?

PR with failing test incoming in case my description wasn't clear.

I'm happy to follow up with a docs-pr / impl. pr to change or document the behaviour as necessary.

sharebear commented 5 years ago

After digging around in the code a little more I see that the hashes created for encode/decode and encodeHex/decodeHex are not interchangable (although I'm still not sure why the groups are prefixed with "1"), so my original error was due to IDE based development (I chose the easiest method that autocompleted).

Would it make sense to split the Hashids class in two to reflect this? i.e. one class to convert to and from an array of longs and another class to convert to and from hex numbers (that can still delegate to an instance of the first).

0x3333 commented 5 years ago

Well, I don't know why the encodeHex is prefixed with "1", but this makes it non-interoperable between non-Hex` functions. It was added as "MongoDB support" but I could not understand why it should be prefixed with "1".

sharebear commented 5 years ago

This issue can now be closed. I've submitted a PR to improve the documentation around this functionality.