shiro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Les Hazlewood <>
Subject Re: Crypto hierarchy is a *good thing*
Date Mon, 25 Oct 2010 19:08:49 GMT
Just to clarify, the 'crypto hierarchy' that Kalle and I were talking
about at the time was regarding the CipherService implementations, not
the Hash implementations.

> My response here is that we should have.
> try {
>  Hasher hasher = new MessageDigestHasher("SHA-512");
>  HashedCredentialsMatcher matcher = new HashedCredentialsMatcher(hasher);
> } catch(NoSuchAlgorithmException nsae)
> {
> }

There is no reason for this IMO - this is essentially no different to
the end-user than using the MessageDigest class directly.  Shiro uses
unchecked exceptions for everything as a design tenet.  The end-user
has the choice to catch a CryptoException if they want to react to a
potentially erroneous algorithm name.

> Strings specification for algorithms are there for a reason, pluggable algorithms into
the JVM.  If it's known, a priori, that the algorithm is in every JVM, e.g. SHA-512, then
one can use a helper method.

They're there for a reason due to a procedurally-oriented JDK
implementation that foregos type-safety and interface-based design
goals (in a type-safe language!).  The CipherService and Hash
interfaces in Shiro exist to circumvent these design issues and to
provide end-users a checked-exception-free, type-safe and intuitive

> Now, with that said.  Look at all the classes we can remove.   Quite a lot.

We can remove the concrete Hash implementations, but I don't agree
that we should.   Without question, the number one design goal of
Shiro is ease-of-use, even if it means maintaining a little more code
by the dev team. A lot of people are using code like this:

new Sha1Hash(blah).toBase64()
new Md5Hash(foo).toHex()

which is a just a joy to use as an end-user and more readable compared
to anything the JDK provides.

So, even if we can remove the Hash implementations and ask everyone to
move to using SimpleHash("SHA-1", blah).toBase64() instead, I don't
know that we should.  The additional feature of type-safety (which
also eliminates erroneous input: "is it SHA1? or SHA-1?  or sha-1?")
can be used for reflection and other configuration mechanisms is a
_nice_ thing.  Keeping 6 or 7 or so Hash implementations that never
change for this level of convenience is a suitable concession to me.
And we retain backwards compatibility, which is ideal if we can do it,
even when moving to 2.0.

>> new Sha512Hash(aFile).toHex()
>> is _much_ nicer for end users than using the JDK's crappy
>> MessageDigest class.  It is also type safe.  Using a string to

> What I have an issue with here is that you've also bundled in the encoding.  That should
be decoupled.

I agree with you on principle and try to do this almost always -
separate orthogonal concerns and not combine them.  However, in this
particular case, I think practicality wins out.  Shiro is not a
framework 'on a hill' that mandates perfect architectural design.  We
strive for it when possible, but when in conflict with practicality,
practicality should 'win out'.

When working with hashes and output from Ciphers, the need to
hex-encode or base64-encode the resulting data is so strong, they
should be immediately accessible for the end-user.  This is a
real-world use-case where I think architectural philosophy takes a
back seat.

Without the toBase64() and toHex() methods, now and end-user has to go
find out how to do this themselves.  They could search for a while in
Shiro's APIs or go read the documentation, or resort unnecessarily to
commons-codec because they didn't know they didn't have to - or they
could just use their IDE's auto-complete feature and have a working
solution in seconds.

The same philosophy exists in Java's Object class's toString() method.
 Surely creating a string representation of an object (XML? JSON?
Simple string name?) is architecturally orthogonal to the object's
reason for existence.  It is included (and forced upon) Java
developers - something not to be taken lightly in such a core language
feature - because it is used so darn often, that real-world use
justifies its inclusion.

I think this same principle applies to the ByteSource interface and
its implementations given their usage patterns in an application.

My .02,


View raw message