speps/go-hashids

DecodingWithError does not produce error

matthewvalimaki opened this issue · 5 comments

Using TestDecodeWithError on https://github.com/speps/go-hashids/blob/master/hashids_test.go#L163 I noticed that it fails and no error is produced if I do the following:

hdata.Alphabet = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890"
dec, err := hid.DecodeWithError("")

However if I use hdata.Alphabet = "PleasAkMEFoThStx" that is currently in the test it works.
Depending on the hash dec is either [] or [some number].

My expectation here was to see an error and [].

Workaround is to rehash the decoded integer value and see if hashes match. Something that is recommended on http://hashids.org: If you give Hashids the wrong salt and it just so happens that it decodes back to some random integers, Hashids does a quick check by encoding those integers to be sure the initial id matches. If it does not, the integers are discarded.

I have gone ahead and added such a check in my code but I do believe it should be done as part of the library itself. For example in comparison the JS implementation produces expected [] result with the case I mentioned earlier.

speps commented

Thanks for the investigation! I'll see if I have some time to implement that in the library soon, otherwise you could send a PR and I'll gladly integre it.

@speps unfortunately I do not have time to research where the problem lies. Hopefully you get the time soon.

Probably it would be a good idea to convert tests from https://github.com/ivanakimov/hashids.js/tree/master/tests. I might have time to assist with that.

PR #31 now adds TestDecodeWithError2 to hashids_test.go that is specific to this issue.
A side note is that hashid.js does not return errors but empty arrays (in Go's case []int{}, not nil) and that's why the other tests are tripping. I should've created own branch for just this specific test :( Let me know if I need to clean up.