Guava: Provide non-deprecated md5 and sha1 hasher

[alanyst]

For understandable reasons, com.google.common.hash.Hashing.md5 and sha1 were deprecated because they are cryptographically broken and it's vital that programmers not rely on them for secure uses.

In a project of mine, there are legacy uses of md5 that use it as a convenient non-cryptographic hash function, and these uses cannot be readily eliminated.

My project uses Scala, which doesn't give fine-grained control over compiler warnings. As a result, I either have to silence all compiler warnings, or see lots of deprecation warnings about md5 usage. Both alternatives are undesirable.

I propose that the existing deprecation of md5 and sha1 remain; but that a non-deprecated, equivalent version of them be provided for uses such as mine. They should be named something like unsafeMD5 or brokenSHA1 to convey the risk of using them, but should not cause compiler warnings when used.

[kevinb9n]

Personally, I'm not sure it was ever entirely correct to mark these methods deprecated. The hash function you need to use is often dictated by another system or data that was already persisted and can't be changed. If the format chosen was md5, then Hashing.md5() is the one and only best choice to use.

We want to discourage people from choosing it when they have the actual freedom to select their hash function - but there's no way to express that.

Maybe providing each under two different names, with only one of them marked deprecated, is indeed the right way out of this. I'm not sure yet. I would consider naming the non-deprecated ones with the prefix "legacy", like legacyMd5() -- the md5 you use when it's for legacy reasons.

[cpovirk]

Interesting to know about Scala, thanks.

Is it possible to define your own LegacyHashing class that provides md5 and sha1 methods that delegate to ours and then to suppress warnings for that class only (either by compiling it separately with different flags or by doing something like this)?

We'd considered some other names for a non-deprecated version -- "legacyMd5," etc. -- but we were worried that it it would suggest that the method doesn't really implement "real MD5." (In fact, we have a "brokenFoo" method internally from a case in which the original implementation of "foo" was incorrect :))

[alanyst]

That's a good workaround, thanks @cpovirk. By defining a Java class with its fine-grained warning suppression, I was able to eliminate the compiler warnings. This solution works for me (and I hereby place this code in the public domain):

package mypackage;

import com.google.common.hash.HashFunction;
import com.google.common.hash.Hashing;

/**
 * Provides an md5 and sha1 hash function without producing deprecation warnings.
 */
public class LegacyHashing {
    @SuppressWarnings("deprecation")
    public static HashFunction md5() {
        return Hashing.md5();
    }

    @SuppressWarnings("deprecation")
    public static HashFunction sha1() {
        return Hashing.sha1();
    }
}

[Maaartinus]

@cpovirk What about nesting LegacyHashing inside of Hashing and linking the methods the other way round (i.e., Hashing.md5() calling Hashing.LegacyHashing.md5()) in order to make them easy to discover?

[cpovirk]

I think we're mostly happy with the idea that people can suppress the warnings when necessary. (And I'm glad we have a workaround for Scala. I'm entertained that it didn't occur to me to write the workaround in Java :)) If it's causing people additional trouble, though, we'd like to hear about it.

[findepi]

If it's causing people additional trouble, though, we'd like to hear about it.

@cpovirk
I would like to reopen this thread.

As @kevinb9n wrote:

Personally, I'm not sure it was ever entirely correct to mark these methods deprecated.

For non-cryptographic reasons, the deprecation notice for Hashing#sha1 advices to use Hashing#goodFastHash which is a good advise, unless the result is going to be persisted somewhere for comparison at later
The other suggested option, sha256 produces an unnecessarily long hash (again: non-crypto usage) and i have a use-case where storage space is limited.

For now I can @SuppressWarnings("deprecation"), but -- especially with Guava's general view on managing deprecated features -- I don't feel comfortable with it. Please remove the deprecation.