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. I still think that the code would be more straightforward without a special-case when i=0. -chris
signature.asc
Description: OpenPGP digital signature
