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

different salt can give random number or crash #20

Closed
chrisport opened this issue May 14, 2015 · 4 comments
Closed

different salt can give random number or crash #20

chrisport opened this issue May 14, 2015 · 4 comments

Comments

@chrisport
Copy link

Hi,
When using different salt it should return empty array. But I recognized the behavior is quite unpredictable, there is a probability that it decodes to a wrong number instead or crashes with an ArrayIndexOutOfBoundsException.

Here and example that will decode to a wrong number:

    @Test
    public void encodeAndDecodeTokenWithWrongKey() {
        Hashids a = new Hashids("supersecret",4);
        Hashids b = new Hashids("differentsupersecret",4);

        long sampleId = 123l;
        String token = a.encode(sampleId);

        long[] ids = b.decode(token); // []long id = { 81143 }
        assertEquals(0, ids.length);
    }

assertion will fail, id[0] will be 81143

Same test with sampleId = 37l will cause "ArrayIndexOutOfBoundsException"

@chrisport chrisport changed the title combination with different salt gives me random number different salt can give random number or crash May 14, 2015
@fanweixiao
Copy link
Member

We should not expect assertEquals(0, ids.length) will be success when using different salts.

@fanweixiao
Copy link
Member

But crash should be avoid, should return empty array

@fanweixiao fanweixiao reopened this May 14, 2015
@chrisport
Copy link
Author

Okay good.

Also in case somebody needs: duplicating the id will decrease the probability of receiving an id from an invalid token:

String token = hashids.encode(id, id);

long[] ids = hashids.decode(token);
if (ids.length == 2 && ids[0] == ids[1]) {
   return ids[0];
}else{
   // not a valid token
} 

@0x3333
Copy link
Collaborator

0x3333 commented Aug 12, 2016

ArrayIndexOutOfBoundsException crash has been fixed in PR #27 .

The collision behavior is the same as the default javascript implementation, it is just a collision within different salts. Can be less possible increasing the minHashLength.

Collisions is not possible within the same salt, but perfectly possible in a small sample space as 4 lenght hash.

[A-Z,a-z,0-9] 62^4 = almost 14 million unique values, not that much
[A-Z,a-z,0-9] 62^6 = almost 56 billion unique values, much better

As long as I can tell, this issue can be closed, as the crash has been fixed and the collision is not a bug.

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

No branches or pull requests

3 participants