Personally, I dislike String-based factory methods. To
me, that feels like old-think Java. I'd rather instantiate a specific type, with
the type-safety and self-documenting benefits that provides. It makes for less
involuted code, in my opinion. Faced with a decision between:
Hash hasher = new Sha256Hash(object);
and
Hash hasher =
SomeHashFactory.newHasher("SHA-256", object);
and
Hash hasher = new
SomeMasterClassThatHasEveryAlgorithm("SHA-256", object);
I would choose to use the first every time, if it was
available. That's the reason JDOM was such a successful Java library for
XML--it did away with all the extraneous interfaces and String-based factory
methods of the XML API built into the JDK and gave developers exactly what they
really wanted to work with: a model of concrete, type-safe objects. The way
Shiro's Hash hierarchy is built is exactly what I as
a user of the framework would want. Additionally, the presence of toHex() and
toBase64() on the interface and implementation is also something I want, as a
user. I have an intense dislike for what I view as the standard Java approach of
"Make the hard things easy and the easy things obtuse." I don't want
to have to write code like:
Hash hash = new Sha512Hash(object);
byte[] hashBytes = hash.getBytes();
Encoder encoder = new Base64Encoder();
String base64Encoded = encoder.encode(hashBytes);
To me, that feels just like Java forcing me to do this to
read a line from System.in:
BufferedReader br = new BufferedReader(new
InputStreamReader(System.in));
br.readLine();
versus
Console.ReadLine();
in C#.
As an end user of the framework, I don't want or need all the extra
indirection. As an end user, I don't want or need "perfect" architectural
design. What I want is a straightforward API that gives me access to the
functions I want at the point where I need them that also gives me the ability
to go lower-level if I choose to, or have a specific reason for doing so. With
the current Hash interface, I can immediately get a Base-64 encoded version of
my Hash without having to use another set of objects to get it. But, if I want
to use my own custom encoding technique, I can do that too because I also have
access to the raw hashed bytes.
There's certainly a limit to how much of this convenience
you want to build into APIs. It can lead to bloat, making it hard to find what
you're looking for in documentation and in code, and it can also lead to
extensibility and maintainability issues. However, if carefully thought out, it
can also result in a much more usable, friendly API. I'm not looking for a
security framework with textbook perfect architecture and the leanest, DRYest
interfaces. I'm
looking for one that makes hard security operations easy (something Shiro does
well) while keeping the easy things easy too.
Additionally, with regards to the use of inheritance in
Shiro, I would say it’s very well-regulated. At this point I have ported roughly
1/3 to 1/2 of shiro-core to C#, so I’m intimately familiar with how the
internals interact. While the inheritance hierarchy is deep, a deep hierarchy
is not, in and of itself, an indicator that there’s a problem. Shiro’s
hierarchy was intentionally constructed to separate concerns (which I think it
does a good job of) and to allow developers who need to have higher control
over the exact behavior of the framework to get it by allowing them to subclass
and take control of the hierarchy at any point. There may be some cleanup that’s
possible (I’ve found a few places in my work on porting Shiro to C# where I
think things could be tightened up a bit, certainly), but that’s to be expected
of any framework this old.
Best regards,Bryan
> Date: Mon, 25 Oct 2010 12:08:49 -0700
> Subject: Re: Crypto hierarchy is a *good thing*
> From: [email protected]
> To: [email protected]
>
> 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
> API.
>
> > 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,
>
> Les
