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

Validate salt length? #31

Open
hoffi opened this issue Oct 4, 2022 · 1 comment
Open

Validate salt length? #31

hoffi opened this issue Oct 4, 2022 · 1 comment

Comments

@hoffi
Copy link

hoffi commented Oct 4, 2022

The salt appears to have a maximum length after which it has no longer impacts the generated hash.

Here some examples:

Hashids.new('a' * 41).encode(1)
=> "pn" 
Hashids.new('a' * 42).encode(1)
=> "nd" 
Hashids.new('a' * 43).encode(1)
=> "yQ" 
Hashids.new('a' * 44).encode(1)
=> "yQ" 
Hashids.new('a' * 45).encode(1)
=> "yQ" 

If i change the alphabet it also affects this maximum length:

Hashids.new('a' * 22, 0, 'abcdefghijkmnopqrstuvwxyz023456789').encode(1)
=> "b0" 
Hashids.new('a' * 23, 0, 'abcdefghijkmnopqrstuvwxyz023456789').encode(1)
=> "bd" 
Hashids.new('a' * 24, 0, 'abcdefghijkmnopqrstuvwxyz023456789').encode(1)
=> "bd" 
Hashids.new('a' * 25, 0, 'abcdefghijkmnopqrstuvwxyz023456789').encode(1)
=> "bd" 

Looks like something around (alphabet.length * 0.7).floor is the maximum length?

Should this be validated and raise a SaltError when the salt is too long?

@maslenkov
Copy link

@hoffi
Yes, there is a maximum length after which it has no effect on the generated hash. But still salt is used and if you change the letter in your snippet you will see an effect.

https://github.com/peterhellberg/hashids.rb/blob/master/lib/hashids.rb#L157
Hashids uses idx = (idx + 1) % salt_length to provide looping/cycling through indices in the alphabet shuffling process. At the same time it depends on the initial alphabet length - separators length - guards - lottery. So if salt length is quite larger then alphabet then it would use only first part of salt for alphabet shuffling. And it would be enough to shuffle the whole alphabet. Anyway, shuffling will happen. And yes, the last part of the salt would not be used. I agree that this is not obvious, but this is not an exception. Using a random string would generate uniq values.

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

2 participants