Christopher Schultz <ch...@christopherschultz.net> wrote:
>Mark, > >On 11/17/12 8:04 PM, Christopher Schultz wrote: >> Apologies for the late reply. Please see comments inline. >> >> On 11/13/12 9:17 AM, ma...@apache.org wrote: >>> Author: markt >>> Date: Tue Nov 13 14:17:42 2012 >>> New Revision: 1408739 >>> >>> URL: http://svn.apache.org/viewvc?rev=1408739&view=rev >>> Log: >>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=54141 >>> Increase the maximum number of supported nested realm levels from 2 >to 3 and make the maximum configurable via a system property. >>> >>> Modified: >>> tomcat/trunk/java/org/apache/catalina/startup/RealmRuleSet.java >>> tomcat/trunk/webapps/docs/config/systemprops.xml >>> >>> Modified: >tomcat/trunk/java/org/apache/catalina/startup/RealmRuleSet.java >>> URL: >http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/startup/RealmRuleSet.java?rev=1408739&r1=1408738&r2=1408739&view=diff >>> >============================================================================== >>> --- tomcat/trunk/java/org/apache/catalina/startup/RealmRuleSet.java >(original) >>> +++ tomcat/trunk/java/org/apache/catalina/startup/RealmRuleSet.java >Tue Nov 13 14:17:42 2012 >>> @@ -34,6 +34,10 @@ import org.apache.tomcat.util.digester.R >>> public class RealmRuleSet extends RuleSetBase { >>> >>> >>> + private static final int MAX_NESTED_REALM_LEVELS = >Integer.getInteger( >>> + >"org.apache.catalina.startup.RealmRuleSet.MAX_NESTED_REALM_LEVELS", >>> + 3).intValue(); >>> + >>> // ----------------------------------------------------- >Instance Variables >>> >>> >>> @@ -83,23 +87,28 @@ public class RealmRuleSet extends RuleSe >>> @Override >>> public void addRuleInstances(Digester digester) { >>> >>> - digester.addObjectCreate(prefix + "Realm", >>> - null, // MUST be specified in the >element, >>> - "className"); >>> - digester.addSetProperties(prefix + "Realm"); >>> - digester.addSetNext(prefix + "Realm", >>> - "setRealm", >>> - "org.apache.catalina.Realm"); >>> - >>> - digester.addObjectCreate(prefix + "Realm/Realm", >>> - null, // MUST be specified in the >element >>> - "className"); >>> - digester.addSetProperties(prefix + "Realm/Realm"); >>> - digester.addSetNext(prefix + "Realm/Realm", >>> - "addRealm", >>> - "org.apache.catalina.Realm"); >>> - >>> - } >>> + String pattern = prefix; >>> >>> + for (int i = 0; i < MAX_NESTED_REALM_LEVELS; i++) { >>> >>> + if (i > 0) { >>> + pattern += "/"; >>> + } >>> + pattern += "Realm"; >>> + >>> + digester.addObjectCreate(pattern, >>> + null, // MUST be specified in >the element, >>> + "className"); >>> + digester.addSetProperties(pattern); >>> + if (i == 0) { >>> + digester.addSetNext(pattern, >>> + "setRealm", >>> + "org.apache.catalina.Realm"); >>> + } else { >>> + digester.addSetNext(pattern, >>> + "addRealm", >>> + "org.apache.catalina.Realm"); >>> + } >>> + } >>> + } >> >> The code above might be a bit more straightforward if you did "step >1" >> (prefix/Realm + setRealm) above the loop and then loop for each >"nested" >> realm. >> >> Also, I think most users would expect setting MAX_NESTED_REALM_LEVELS >> to, say, 3, would allow Realm/Realm/Realm while I believe the code >above >> will only allow Realm/Realm. If you want Realm/Realm/Realm, you have >to >> set MAX_NESTED_REALM_LEVELS to 4 because of the strictly-less-than >that >> you have in the loop control statement. > >That last part is, of course, nonsense. That's what I get for reading >code while quite exhausted. Easily done. Don't worry about it. >I still think that the code would be more straightforward without a >special-case when i=0. Given that you need one less '/' than you do 'Realm' there is going to have to be some form of special case or duplicate code or ... Personally, I don't really like any of the constructs you can use to achieve this. Of all of them, I prefer the above approach as you don't end up duplicating the code that happens on every loop. The larger that block of code, the less I like the duplication. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
