On 20/11/2015 13:07, 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?
For what an application is going to use this for? I don't think there is a performance concern here. I'd go this route. >> 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()?) Not sure right now. I'll take a quick look. >> 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. -1 for removing those entirely necessary locks. See svn log for explanation. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
