Could anyone verify the correctness of getting a md5 hash using this method?

Question

        MessageDigest m=MessageDigest.getInstance("MD5");
        StringBuffer sb = new StringBuffer();
        if(nodeName!=null) sb.append(nodeName);
        if(nodeParentName!=null) sb.append(nodeParentName);
        if(nodeParentFieldName!=null) sb.append(nodeParentFieldName);
        if(nodeRelationName!=null) sb.append(nodeRelationName);
        if(nodeViewName!=null) sb.append(nodeViewName);
        if(treeName!=null) sb.append(treeName);
        if(nodeValue!=null && nodeValue.trim().length()>0) sb.append(nodeValue);
        if(considerParentHash) sb.append(parentHash);
        m.update(sb.toString().getBytes("UTF-8"),0,sb.toString().length());
        BigInteger i = new BigInteger(1,m.digest());
        hash = String.format("%1$032X", i);

The idea behind these lines of code is that we append all the values of a class/model into a StringBuilder and then return the padded hash of that (the Java implementation returns md5 hashes that are lenght 30 or 31, so the last line formats the hash to be padded with 0s).

I can verify that this works, but I have a feeling it fails at one point (our application fails and I believe this to be the probable cause).

Can anyone see a reason why this wouldn't work? Are there any workarounds to make this code less prone to errors (e.g. removing the need for the strings to be UTF-8).

UTF-8 strings can contain multibyte characters, in which (as the name suggests) a character spans more than one byte. So if any of your data fields will possibly ever contain characters which would be multibyte in a UTF-8 representation, then using length() would be incorrect, because length() counts characters, not bytes. But to be honest, I'm not sure if this actually matters - does it really matter if the byte array used to create the hash is slightly truncated? It would matter if the hash were created by different code somewhere else, certainly. But why would you have 2 routines for that? — Robin Green, Apr 12 '11 at 08:28
how does your application fail? please provide a [SSCCE](http://sscce.org/) demonstrating the fail. — Lesmana, Apr 12 '11 at 08:29

score 1 · Accepted Answer · edited May 23 '17 at 11:47

There are a few weird things in your code.

UTF-8 encoding of a character may use more than one byte. So you should not use the string length as final parameter to the update() call, but the length of the array of bytes that getBytes() actually returned. As suggested by Paŭlo, use the update() method which takes a single byte[] as parameter.

The output of MD5 is a sequence of 16 bytes with quite arbitrary values. If you interpret it as an integer (that's what you do with your call to BigInteger()), then you will get a numerical value which will be smaller than 2¹⁶⁰, possibly much smaller. When converted back to hexadecimal digits, you may get 32, 31, 30... or less than 30 characters. Your usage of the the "%032X" format string left-pads with enough zeros, so your code works, but it is kind of indirect (the output of MD5 has never been an integer to begin with).

You assemble the hash input elements with raw concatenation. This may induce issues. For instance, if modeName is "foo" and modeParentName is "barqux", then the MD5 input will begin with (the UTF-8 encoding of) "foobarqux". If modeName is "foobar" and modeParentName is "qux", then the MD5 input will also begin with "foobarqux". You do not tell why you want to use a hash function, but usually, when one uses a hash function, it is to have a unique trace of some piece of data; two distinct data elements should yield distinct hash inputs.

When handling nodeValue, you call trim(), which means that this string could begin and/or end with whitespace, and you do not want to include that whitespace into the hash input -- but you do include it, since you append nodeValue and not nodeValue.trim().

If what you are trying to do has any relation to security then you should not use MD5, which is cryptographically broken. Use SHA-256 instead.

Hashing an XML element is normally done through canonicalization (which handles whitespace, attribute order, text representation, and so on). See this question on the topic of canonicalizing XML data with Java.

score 0 · Answer 2 · answered Apr 12 '11 at 10:40

One possible problem is here:

m.update(sb.toString().getBytes("UTF-8"),0,sb.toString().length());

As said by Robing Green, the UTF-8 encoding can produce a byte[] which is longer than your original string (it will do this exactly when the String contains non-ASCII characters). In this case, you are only hashing the start of your String.

Better write it like this:

m.update(sb.toString().getBytes("UTF-8"));

Of course, this would not cause an exception, simply another hash than would be produced otherwise, if you have non-ASCII-characters in your string. You should try to brew your failure down to an SSCCE, like lesmana recommended.

Could anyone verify the correctness of getting a md5 hash using this method?

2 Answers2