Rémy, On 11/20/15 8:07 AM, Rémy Maucherat wrote: > 2015-11-20 13:47 GMT+01:00 Christopher Schultz <ch...@christopherschultz.net >> : > >> All, >> >> I thought there was a BZ issue for this, but I didn't find one. It's >> been suggested (and I agree completely) that an application ought to be >> able to fetch the CredentialHandler for the context's realm so that it >> could mutate user credentials in the same way that the Realm expects to >> do. That allows applications to change user's passwords, for instance. >> >> So I think I've identified where to put this: around >> StandardContext.java:5131 (in Tomcat 9/trunk). But I have a few >> questions about it. >> >> It occurs to me that we won't want applications to change the >> CredentialHandler in any way -- such as changing the salt length for a >> DigestCredentialHandler, etc., so I think it makes sense to wrap the >> CredentialHandler in a "safe" class that doesn't allow access to the >> real CredentialHandler. That's fairly easy to do. >> >> Also, we probably want the CredentialHandler in the application scope to >> always be in sync with the CredentialHandler actually being used by the >> Realm. Realm has a setCredentialHandler(CredentialHandler) method, so >> theoretically, the CH could change at any time. >> >> Also, the Realm could theoretically change at any time as well. >> >> So, this "safe" wrapper could just call >> getRealmInternal().getCredentialHandler().underlyingMethod and always >> use that. But, ContainerBase.getRealmInternal uses a lock, and I'm not >> sure how expensive that is. Is there any concern over performance, here? >> >> Another option would be to override setRealm and capture the Realm, >> there... except that StandardContext never calls setRealm directly. >> (Presumably, it's called by another component when constructing the >> StandardContext, but then why does StandardContext.startInternal call >> getRealmInternal().start() when ContainerBase.setRealm already calls >> Realm.start()?) >> >> I'd appreciate some thoughts on the above; I don't want to have to >> commit 12 patches for this due to misunderstandings about what's going >> on with StandardContext. > > +1 for removing those pointless locks.
Okay. I'm not going to remove those myself, though. If I don't hear from anyone in a while, I'll commit my patch which always calls delegates to getRealmInternal.getCredentialHandler.call, as its the simplest patch that will cover all the cases I can think of. -chris --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
