SHA instances aren't disposed

[andreimilto]

AFAIU, an instance of SHA256, SHA384 or SHA512 should be disposed after use since it implements IDisposable. E.g., in the documentation they wrap the SHA-related code with using statement. I guess the same should be done here:

private static byte[] EnhancedHash(byte[] inputBytes, HashType hashType)
{
    switch (hashType)
    {
        case HashType.SHA256:
            inputBytes = SafeUTF8.GetBytes(Convert.ToBase64String(SHA256.Create().ComputeHash(inputBytes)));
            break;
        case HashType.SHA384:
            inputBytes = SafeUTF8.GetBytes(Convert.ToBase64String(SHA384.Create().ComputeHash(inputBytes)));
            break;
        case HashType.SHA512:
            inputBytes = SafeUTF8.GetBytes(Convert.ToBase64String(SHA512.Create().ComputeHash(inputBytes)));
            break;
        case HashType.Legacy384:
            inputBytes = SHA384.Create().ComputeHash(inputBytes);
            break;
        default:
            throw new ArgumentOutOfRangeException(nameof(hashType), hashType, null);
    }

    return inputBytes;
}

[ChrisMcKee]

IDisposable in .net is a skip fire of legacy (hence the old HTTPClient is disposable, lets make a new one for every call... what do you mean that wasn't how its supposed to be used; endless battle)

The calls in SHAXXX (lets say 512 here) to dispose just call the base hashingalgorithm method as its part of the framework expectations https://github.com/dotnet/runtime/blob/master/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/SHA512.cs#L56-L57
Which ultimately just calls dispose on base which has a nice note.
https://github.com/dotnet/runtime/blob/master/src/libraries/System.Security.Cryptography.Primitives/src/System/Security/Cryptography/HashAlgorithm.cs#L186-L188

                // Although we don't have any resources to dispose at this level,
                // we need to continue to throw ObjectDisposedExceptions from CalculateHash
                // for compatibility with the .NET Framework.

GC.Suppress finalize is called with the current hash algo object which is only done to suppress pointless GC https://docs.microsoft.com/en-us/dotnet/api/system.gc.suppressfinalize?view=netframework-4.8

So the value of making the dispose wrapper / calls appears limited/worthless

[andreimilto]

The calls in SHAXXX (lets say 512 here) to dispose just call the base hashingalgorithm method as its part of the framework expectations

Well, first of, I wouldn't be so sure about that. From the way the value of _hashProvider is set, the _hashProvider.Dispose() seems to be doing some job, and that job is OS-specific (Windows, Unix, OSX).

And second, even if we agree that there's no harm in not disposing a SHA-2 instance currently, there's no guarantee that it's going to be like that in the future. So, I guess it's better to wrap all SHA instances in using statements just to be on the safe side. Since it doesn't cost anything and makes the code more robust, I see no reason not doing it.

[ChrisMcKee]

HashProviderDispenser just creates a hash provider via the systems native provider. I had totally forgotten how god awful it is to have the core clr open on my machine.
And yeah the SafeEvpMdCtxHandle contexts are disposed of in the Windows/Unix ones, so there's that. Bit nightmare fuel tracing if the release is called in the dispose so 🤷‍♂️ may as well go with it

            switch (hashType)
            {
                case HashType.SHA256:
                    using (var sha = SHA256.Create())
                        inputBytes = SafeUTF8.GetBytes(Convert.ToBase64String(sha.ComputeHash(inputBytes)) + (bcryptMinorRevision >= 'a' ? Nul : EmptyString));
                    break;
                case HashType.SHA384:
                    using (var sha = SHA384.Create())
                        inputBytes = SafeUTF8.GetBytes(Convert.ToBase64String(sha.ComputeHash(inputBytes)) + (bcryptMinorRevision >= 'a' ? Nul : EmptyString));
                    break;
                case HashType.SHA512:
                    using (var sha = SHA512.Create())
                        inputBytes = SafeUTF8.GetBytes(Convert.ToBase64String(sha.ComputeHash(inputBytes)) + (bcryptMinorRevision >= 'a' ? Nul : EmptyString));
                    break;
                default:
                    throw new ArgumentOutOfRangeException(nameof(hashType), hashType, null);
            }

[andreimilto]

@ChrisMcKee, 👍

[ChrisMcKee]

Tis in master