Re: Patch: embedded window extension

[Bryce McKinlay <mckinlay at redhat dot com>]

Michael Koch wrote:

> Am Freitag, 23. Juli 2004 14:26 schrieb Eric Blake:
>  
>
> > Your approach looks okay to me; you solved my initial concern about
> > correctness.  But I still have 2 comments.
> >
> > First, I am not sure whether setAccessible(true) is visible by
> > anyone else using reflection; should you be caching the
> > pre-existing accessible value on entry and restoring it before
> > existing the priveleged action?  This is more of a general question
> > I have about reflection - is getDeclaredField() allowed to return
> > the same Field instance to more than one caller, and if so, is
> > there a race condition where when one thread has set the Field as
> > accessible inside a privileged action, another thread without
> > security privileges can use that same Field with the temporary
> > elevation in accessibility even though that thread can't change the
> > accessibility?  Or is getDeclaredField() required to return a
> > different instance to every caller?  On the other hand, there are
> > also security checks for getDeclaredField, so a security manager
> > can enforce that only privileged actions can access the Field
> > instance in the first place, allowing getDeclaredField to return
> > the same instance to all allowed callers.
> >    
>
> I'm no expert in reflection. In fact I'm using it first time directly. 
> Perhaps someone other can help ?
>  
In libgcj, getDeclaredField(), along with the other reflection methods 
in java.lang.Class, will return a new Field object every time. I don't 
know whether this behaviour is specified by the spec or not, but it does 
seem like a good idea given the potential for conflicts between code 
with different privileges getting back the same object.

Conclusion: You do not need to call setAccessible(false) to put things 
back when you're done.

> > Second, how often is this method called?  If it is called
> > frequently, I would consider creating a cached version of the
> > anonymous PrivilegedAction, rather than creating and destroying one
> > for every invocation.  If it is rarely called, then caching is
> > probably not worth the effort, and your code is fine as is.
> >    
>
> This method should be called only once. I think there is no need for 
> caching.
>  

Note that you can now use gnu.java.security.action.SetAccessibleAction 
to do this.

Regards

Bryce