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

Error on TryComputeHash #12

Open
Zetanova opened this issue Mar 23, 2023 · 1 comment
Open

Error on TryComputeHash #12

Zetanova opened this issue Mar 23, 2023 · 1 comment

Comments

@Zetanova
Copy link

The generic HashAlgorithm.TryComputeHash(ReadOnlySpan<byte> source, Span<byte> destination, out int bytesWritten) implementation of this function gives an error on the FinalBlock call.

It would be nice to overload this method

@daiplusplus
Copy link

daiplusplus commented Feb 17, 2024

I experienced the same problem today when I was reimplementing MurmurHash myself and was running tests that compared my implementation to this library's implementations as a reference.

  • The root cause is (what I consider) a poor API design in .NET's HashAlgorithm class: when you subclass HashAlgorithm you need to set the hash length; this can be done in two ways:
    • By either setting the protected Int32 HashSizeValue field in your constructor - or anywhere in your subclass (because the field is fully mutable and not readonly).
    • Or by overriding the public virtual Int32 HashSize { get; } property.
    • However, if you only override HashSize and never set HashSizeValue then you'll get this error.
  • When HashAlgorithm added support for Span<Byte> in the new virtual methods TryComputeHash and TryHashFinal, the default implementations only check the protected Int32 HashSizeValue field and not the property.
  • ...but this MurmurHashXX library only overrides the properties, it never sets the HashSizeValue field.
  • So the default implementations throw the InvalidOperationException "The algorithm's implementation is incorrect." because it thinks the hash length is 0 bytes, instead of 4 or 16.
  • Subclassing MurmurHash32 and MurmurHash128 isn't really a feasible option, but the good news is that you can use reflection to correctly set the field right-after you get an instance of the objects.

Like so:

private static readonly FieldInfo _hashAlgorithm_HashSizeValue_Field = typeof(HashAlgorithm).GetField( "HashSizeValue", BindingFlags.NonPublic | BindingFlags.Instance ) ?? throw new InvalidOperationException( "Couldn't find HashAlgorithm.HashSizeValue field." );

private static void FixHashSizeValueField( this HashAlgorithm hashAlgo )
{
	_hashAlgorithm_HashSizeValue_Field.SetValue( obj: hashAlgo, value: hashAlgo.HashSize );
}

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