Re: [cp-patches] java.security.Permissions.implies()

[Mark Wielaard]

Hi,

On Tue, 2005-02-01 at 16:39 +0000, Andrew Haley wrote:
> Permissions.imples() is rather broken ATM.  
> 
> Simply checking if the collection includes a particular permission
> doesn't do the job -- the permission might be a wildcard like
> "/secured/*".  We need to check every permission in the collection.

Oops. Did you write a test case for this?

> The question of synchronization is interesting.  I don't think people
> usually hold the lock on a Permissions instance when they invoke
> implies().

Permissions can (and I assume they normally are) marked readOnly.
That is another bug in this class: add() should check isReadOnly() (see
PermissionCollection which PermissionsHash extends). So you should
probably check if it is read only and if not synchronize the body of the
method.

> 2005-02-01  Andrew Haley  <address@hidden>
> 
>       * java/security/Permissions.java: Iterate over the collection
>       and invoke implies() on each element.

BTW. Please write this as:

        * java/security/Permissions.java (PermissionsHash.implies):
        Iterate over the collection and invoke implies() on each
        element.

That immediately makes clear that you are talking about the inner class
here.

Cheers,

Mark

signature.asc
Description: This is a digitally signed message part