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/08/31 00:55:20 UTC

svn commit: r1379206 - /tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java

Author: markt
Date: Thu Aug 30 22:55:20 2012
New Revision: 1379206

URL: http://svn.apache.org/viewvc?rev=1379206&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53801
Overlapping URL patterns were sometimes merged incorrectly in security constraints leading to incorrect 401 responses. Note: it was possible for access to be denied when it should have been granted but it was not possible for access to be granted when it should have been denied.

Modified:
    tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java

Modified: tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java?rev=1379206&r1=1379205&r2=1379206&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java (original)
+++ tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java Thu Aug 30 22:55:20 2012
@@ -629,14 +629,15 @@ public abstract class RealmBase extends 
                     }
                 }
                 if(matched) {
-                    found = true;
                     if(length > longest) {
+                        found = false;
                         if(results != null) {
                             results.clear();
                         }
                         longest = length;
                     }
                     if(collection[j].findMethod(method)) {
+                        found = true;
                         if(results == null) {
                             results = new ArrayList<>();
                         }
@@ -760,7 +761,7 @@ public abstract class RealmBase extends 
      */
     private SecurityConstraint [] resultsToArray(
             ArrayList<SecurityConstraint> results) {
-        if(results == null) {
+        if(results == null || results.size() == 0) {
             return null;
         }
         SecurityConstraint [] array = new SecurityConstraint[results.size()];



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1379206 - /tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java

Posted by Mark Thomas <ma...@apache.org>.
On 31/08/2012 12:05, Konstantin Kolinko wrote:
> 2012/8/31  <ma...@apache.org>:
>> Author: markt
>> Date: Thu Aug 30 22:55:20 2012
>> New Revision: 1379206
>>
>> URL: http://svn.apache.org/viewvc?rev=1379206&view=rev
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53801
>> Overlapping URL patterns were sometimes merged incorrectly in security constraints leading to incorrect 401 responses. Note: it was possible for access to be denied when it should have been granted but it was not possible for access to be granted when it should have been denied.
>>
>> Modified:
>>     tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java
>>
>> Modified: tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java?rev=1379206&r1=1379205&r2=1379206&view=diff
>> ==============================================================================
>> --- tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java (original)
>> +++ tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java Thu Aug 30 22:55:20 2012
>> @@ -629,14 +629,15 @@ public abstract class RealmBase extends
>>                      }
>>                  }
>>                  if(matched) {
>> -                    found = true;
>>                      if(length > longest) {
>> +                        found = false;
>>                          if(results != null) {
>>                              results.clear();
>>                          }
>>                          longest = length;
>>                      }
>>                      if(collection[j].findMethod(method)) {
>> +                        found = true;
>>                          if(results == null) {
>>                              results = new ArrayList<>();
>>                          }
> 
> There are several loops over constraints, with
> [[[
>  if(found) {
>  return resultsToArray(results);
> }
> ]]]
> between them, and only one of such loops is fixed by this commit.

Correct. Only in one loop is there the possibility that a previously
found constraint will be removed. The bug was that the found flag was
not reset in this case and an empty set of constraints returned rather
than null.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1379206 - /tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java

Posted by Konstantin Kolinko <kn...@gmail.com>.
2012/8/31  <ma...@apache.org>:
> Author: markt
> Date: Thu Aug 30 22:55:20 2012
> New Revision: 1379206
>
> URL: http://svn.apache.org/viewvc?rev=1379206&view=rev
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53801
> Overlapping URL patterns were sometimes merged incorrectly in security constraints leading to incorrect 401 responses. Note: it was possible for access to be denied when it should have been granted but it was not possible for access to be granted when it should have been denied.
>
> Modified:
>     tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java
>
> Modified: tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java?rev=1379206&r1=1379205&r2=1379206&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java Thu Aug 30 22:55:20 2012
> @@ -629,14 +629,15 @@ public abstract class RealmBase extends
>                      }
>                  }
>                  if(matched) {
> -                    found = true;
>                      if(length > longest) {
> +                        found = false;
>                          if(results != null) {
>                              results.clear();
>                          }
>                          longest = length;
>                      }
>                      if(collection[j].findMethod(method)) {
> +                        found = true;
>                          if(results == null) {
>                              results = new ArrayList<>();
>                          }

There are several loops over constraints, with
[[[
 if(found) {
 return resultsToArray(results);
}
]]]
between them, and only one of such loops is fixed by this commit.

It seems inconsistent. (Though with lack of comments there, I have to
investigate more to be certain).

> @@ -760,7 +761,7 @@ public abstract class RealmBase extends
>       */
>      private SecurityConstraint [] resultsToArray(
>              ArrayList<SecurityConstraint> results) {
> -        if(results == null) {
> +        if(results == null || results.size() == 0) {
>              return null;
>          }
>          SecurityConstraint [] array = new SecurityConstraint[results.size()];
>

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org