You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2012/11/13 15:17:43 UTC
svn commit: r1408739 - in /tomcat/trunk:
java/org/apache/catalina/startup/RealmRuleSet.java
webapps/docs/config/systemprops.xml
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");
+ }
+ }
+ }
}
Modified: tomcat/trunk/webapps/docs/config/systemprops.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/systemprops.xml?rev=1408739&r1=1408738&r2=1408739&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/config/systemprops.xml (original)
+++ tomcat/trunk/webapps/docs/config/systemprops.xml Tue Nov 13 14:17:42 2012
@@ -643,6 +643,12 @@
<p>If not specified, the default value of <code>false</code> will be used.</p>
</property>
+ <property name="org.apache.catalina.startup. RealmRuleSet.MAX_NESTED_REALM_LEVELS">
+ <p>The CombinedRealm allows nested Realms. This property controls the
+ maximum permitted number of levels of nesting.</p>
+ <p>If not specified, the default value of <code>3</code> will be used.</p>
+ </property>
+
</properties>
</section>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1408739 - in /tomcat/trunk: java/org/apache/catalina/startup/RealmRuleSet.java webapps/docs/config/systemprops.xml
Posted by Mark Thomas <ma...@apache.org>.
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, markt@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1408739 - in /tomcat/trunk: java/org/apache/catalina/startup/RealmRuleSet.java
webapps/docs/config/systemprops.xml
Posted by Christopher Schultz <ch...@christopherschultz.net>.
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, markt@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
Re: svn commit: r1408739 - in /tomcat/trunk: java/org/apache/catalina/startup/RealmRuleSet.java
webapps/docs/config/systemprops.xml
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,
Apologies for the late reply. Please see comments inline.
On 11/13/12 9:17 AM, markt@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.
Thanks,
-chris