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. Rémy