On 16/09/2014 16:20, Christopher Schultz wrote:
> 1. In terms of limiting converting to String values, we could base
> everything on byte[] instead of String. 

Having looked at the current code and the Servlet API I don't believe
that this is practical.

> 2. I don't like CredentialHandler.mutate(String input, byte[] salt, int
> iterations).

Having considered the options this looks like the least bad approach. A
symmetric encryption impl can just ignore the salt and the iterations.

> 3. MessageDigestCredentialHandler is good, but I think we ought to
> push-down the iteration and salt members and methods from
> BaseCredentialHandler into the subclass.

Given my view of 2, I don't see a need for this.

> 4. In MessageDigestCredentialHandler.matches, there are two uses of
> StandardCharsets.ISO_8859_1 directly instead of using the client's
> preferred character encoding. Is this a requirement of {MD5|SHA|SSHA}
> formatting or an oversight?

As per previous e-mail, I'm not sure but it is what the current code
does so I plan to leave it alone for this work.

> 5. matchesSaltIterationsEncoded method does not check for unexpected
> formatting.

Fixed in updated patch.

> 6. Performance. There are places where we know the sizes of things
> before they are allocated but don't use that to our advantage.

Fixed in updated patch.

> 7. Thread safety. Is it safe to check/set the "random" reference in
> multiple threads?

I believe it is since it is only used via command line tool which will
be single threaded.

> 8. RealmBase.main checks all known CredentialHandlers for matching
> algorithms (a bit inefficient, but its offline so who cares), but it
> doesn't know about anyone's custom CH classes. We should allow the
> caller to specify the CH class if they want, so they can use the
> RealmBase driver with their own CredentiaHandler implementation.

Fixed in updated patch.

Updated patch:
http://people.apache.org/~markt/patches/2014-09-16-bug56403-tc8-v2.patch

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to