ullmark/hashids.net

Bug on long encoding with 16 alphabet chars

GihanSoft opened this issue ยท 8 comments

below code throw System.IndexOutOfRangeException

var hashids = new Hashids("salt", alphabet:"0123456789ABCDEF");
var hash = hashids.EncodeLong(long.MaxValue);

full exception text:

System.IndexOutOfRangeException: Index was outside the bounds of the array.
   at HashidsNet.Hashids.BuildReversedHash(Int64 input, Char[] alphabet, Char[] hashBuffer)
   at HashidsNet.Hashids.GenerateHashFrom(Int64[] numbers)
   at HashidsNet.Hashids.EncodeLong(Int64[] numbers)
   at Program.<<Initialize>>d__0.MoveNext() in :line 7

JS implementation produces an empty string for this case, see https://playcode.io/880757/

However, changing this to be an actual long.MaxValue length of 19 fixes the issue (and btw why it is 12? that's incorrect)

private const int MaxNumberHashLength = 12; // Length of long.MaxValue;

@ullmark @manigandham what are your thoughts on this?

The library is a port of a javascript library, which has since been ported in a lot of languages. My initial code was me trying to make a translation to C# that was a map/translation of the javascript code. That way, if the algoritm changes (which it did, quite a bit at one point) it would be easier to update the .NET version.

That's basically one of the reasons for the code not being optimal both in sense of memory usage, and C# coding standards. In javascript Number doesn't fully map to our Int32 & Int64, so the reason for it being 12 is probably a mistake for us with support for long.

Many users map database IDs to C# long type and I'm afraid the library will lose some amount of users if we remove support for long

I've checked the Python implementation and it works fine with long.MaxValue, producing the same result as if we changed MaxNumberHashLength to 19:

C#:
image

Python:
image

yeah, I didn't mean remove it... I was just explaining why it probably was 12. I also think that we've strayed away from keeping a "similar" codebase, so making it more C# is also ๐Ÿ‘

yeah, I didn't mean remove it... I was just explaining why it probably was 12. I also think that we've strayed away from keeping a "similar" codebase, so making it more C# is also ๐Ÿ‘

Oh, I see. I misunderstood that as "long support was a mistake", my bad

Confirming that Go version returns the same:

package main

import (
	"fmt"
	"github.com/speps/go-hashids/v2"
)

func main() {
	hd := hashids.NewData()
	hd.Salt = "salt"
	hd.Alphabet = "0123456789ABCDEF"
	h, _ := hashids.NewWithData(hd)
	e, _ := h.EncodeInt64([]int64{9223372036854775807})
	fmt.Println(e)
	d, _ := h.DecodeWithError(e)
	fmt.Println(d)
}
58E9BDD9A7598254DA4E
[9223372036854775807]

This change came from commit 1e280f1

Javascript can only support integers up to 53 bits because the number type uses floating points. Looks like it was just a bug at the time. I'll merge in the PR that fixes it.