geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Genender <jgenen...@savoirtech.com>
Subject Re: Fix for GERONIMO-646
Date Fri, 27 May 2005 06:36:59 GMT
Hi Tom,

I investigated your patches...

For the JAASJettyRealm, I altered your patches slightly.  I changed the 
isUserInRole to test if the user or role is null and return false if so. 
  Tomcat handles it this way.  Thanks for the heads up on this.

The empty String for a role should be caught by the checkPermission() 
call right afterwards.

Relative to the WebRoleRefPermission, the JavaDoc/spec makes no mention 
of an IllegalArgumentException that should be thrown.  Although an 
IllegalArgumentException is a RuntimeException, so its not required to 
be documented as thrown...Sun is pretty good at describing when an 
IllegalArgumentException should be thrown.

What I am getting at here is the WebRoleRefPermission is a Sun spec 
class and I would want to get adc and/or David Jencks' input on this 
before I would consider implementing this change.  In the mean time, the 
above patch for the JAASJettyRealm should prevent this from getting 
called in any case, since a null role would never make it to the 
WebRoleRefPermission.

I will update JIRA to reflect this.  I'll leave it open and assign it to 
adc to decide on the WebRoleRefPermission.

Jeff

Tom McQueeney wrote:
> I submitted a patch for bug GERONIMO-646, 
> http://issues.apache.org/jira/browse/GERONIMO-646, to fix a problem if a 
> Jetty web app calls HttpServletRequest.isUserInRole with a null string. 
> If no one objects to my solution of having the method return false if 
> the role is null or an empty string, would someone kindly commit the 
> patch to JAASJettyRealm.isUserInRole?
> 
> I also submitted a patch to the WebRoleRefPermission constructor to 
> throw an IllegalArgumentException if the role name is null or empty. JSR 
> 115 prohibits a role from being null or empty.
> 
> I also submitted a patch to add a test to WebRoleRefPermissionTest.
> 
> Thanks. Please let me know if you have any questions.
> 
> -Tom

Mime
View raw message