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

[Mark Wielaard <mark at klomp dot org>]

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  <aph@redhat.com>
> 
> 	* 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

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