[PATCH] 4851444: Exposing sun.reflect.Reflection#getCallerClass as a public API in Java 8

[David M. Lloyd]

On 09/04/2013 06:23 PM, Mandy Chung wrote:
> On 9/1/2013 1:16 AM, Nick Williams wrote:
>> Class<?> getCallerClass(): Retrieves the class of the caller of the
>> method calling getCallerClass(). This is identical to the new
>> Reflection#getCallerClass() added in Java 7u25/8.
>>
>> Class<?> getCallerClass(int): Retrieves the class of the caller n
>> frames down the stack. This is identical to the old
>> Reflection#getCallerClass(int) that was deprecated in Java 7u25 and
>> removed in Java 8.
>>
>
> The other part of this patch is about the ability to obtain the caller
> information.  The use case includes Groovy 2.x and GUI app to look up
> resource bundles on behalf on the caller and Log4j for logging isolation
> depending on its caller (isolation on ClassLoader for the app).
>
> Such methods are caller-sensitive and must obtain the right caller;
> otherwise it might get surprises.
>
> The proposed getCallerClass() method, no-arg version, looks reasonable
> and it will return the immediate effective caller of the method calling
> the getCallerClass method. Use your example (I add the class for the
> discussion later).  Let's ignore the @CallerSensitive annotation for now.
>
>      getCallerClass();
>      Foo.someMethod1();
>      Bar.someMethod2();
>      Gee.someMethod3();
>      App.actualCallerMethod()
>
> The caller is Bar.someMethod2() and getCallerClass() method will return
> Bar.  It will return the same result even if Foo is called via
> reflection (the VM already handles this and skips the reflection
> machinery).
>
> When security manager is installed, under what condition can Foo class
> access Bar class (any permission check)?  What if Bar is in a restricted
> package?
>
> The standard way to determine if class A has access to class B is based
> on their class loaders and also it is a restricted class (configured in
> the package.access property in JRE/lib/security/java.security).  Code
> has full access to its own class loader and any class loader that is a
> descendent.  There are several methods in java.lang.Class that are
> performing this security check:
>
>       * @throw  SecurityException
>       *         If a security manager, <i>s</i>, is present and the
> caller's
>       *         class loader is not the same as or an ancestor of the class
>       *         loader for the returning Class object and invocation of
> {@link
>       *         SecurityManager#checkPackageAccess s.checkPackageAccess()}
>       *         denies access to the package of returning Class object
>
> I think the above security check should be applicable to the
> getCallerClass method. It means that
> Foo can access Bar if Foo's class loader is the same or an ancestor of
> Bar's class loader; otherwise, it will perform the package access check.
> BTW - including an example in the javadoc would be helpful since the
> term "caller" might be confusing.
>
> For the known use cases, they are interested in getting the caller's
> class loader.  Class.getClassLoader method has the following security
> check:
>
>       * <p> If a security manager is present, and the caller's class
> loader is
>       * not null and the caller's class loader is not the same as or an
> ancestor of
>       * the class loader for the class whose class loader is requested,
> then
>       * this method calls the security manager's {@code checkPermission}
>       * method with a {@code RuntimePermission("getClassLoader")}
>       * permission to ensure it's ok to access the class loader for the
> class.
>
> I wonder if Class instance is really needed while I understand we can't
> anticipate all cases.  Most of the caller-sensitive methods in the JDK
> only need to get caller's ClassLoader.  There are very few cases that
> require the caller class (used in the reflection implementation that I
> have yet to understand deeper).   Perhaps all you need is
> getCallerClassLoader method?  In that case, you will be able to get the
> caller's class loader if you have the access.  This is one thing I'd
> like to explore further (either one or both).
>
> Performance - I believe in common cases Foo's class loader should be the
> same or ancestor of Bar's clas loader.  Bar calls Foo which should be
> visible to Bar's class loader.  It'd be very useful if Groovy and Log4j
> can measure the performance overhead with security manager installed
> when it's available and see if it is a concern.
>
> About the proposed getCallerClass(int) method to find a frame at a
> specific depth is essentially putting back the
> sun.reflect.Reflection.getCallerClass(int depth) method.  This method is
> very flexible but brittle.  I still think it's more reliable that a
> caller-sensitive method should capture the caller and pass it to the
> runtime properly. Groovy 3.x doesn't do the stack walk. Groovy 2.x needs
> a temporary solution to filter out the intermediate frames of groovy
> runtime, would StackTraceFrame.getDeclaringClass be an adequate interim
> solution?
>
> More on @CallerSensitive and s.r.Reflection.getCallerClass(int):
>
> We had found severe bugs in the code to call
> s.r.Reflection.getCallerClass(int) when prototying the fix for JEP 176
> but they were not noticed and no test uncovered them.  e.g. wrong depth
> was used (easy to miscount the depth or the depth is modified due to
> refactoring but difficult to be caught during review).  As a
> caller-sensitive method, it's important to find the right caller;
> otherwise unexpected behavior.
>
> In the first prototype, Chris Thalinger did implement JVM_GetCallerClass
> to walk past all @CS frames and find the immediate caller.  All methods
> in the method chain invoked by the actual caller must be annotated by
> @CS which ends up something like the example you used earlier:
>
> @CallerSensitive getCallerClass(int)
> @CallerSensitive someMethod1()
> @CallerSensitive someMethod2()
> @CallerSensitive someMethod3()
> @CallerSensitive someMethod4()
> actualCallerMethod()
>
> With more analysis, we identified that there was no absolute need (in
> the JDK) to walk the stack but instead a more reliable way is to capture
> the caller at the entry point of the caller-sensitive methods.  In the
> example above, someMethodxx() are internal implementation in the JDK
> case and thus we modified them to take the caller class parameter
> (instead of looking it up at its execution point).  This greatly
> simplifies the VM enforcement to detect if the method calling
> JVM_GetCallerClass is a caller-sensitive method.

This seems reasonable on the surface but falls over once you capture the 
caller for more than one purpose.  For example, say a logging framework 
captures the caller for the purpose of supplementing log information. 
But you call this logging framework from another framework which uses 
caller information for another purpose, for example locating resources. 
  The intent here might be to show information from the second framework 
in the log, however with one universal @CallerSensitive annotation you 
cannot distinguish which "capture" you want to grab - the second 
framework, or the caller who called the second framework.  However by 
traversing the stack to a fixed depth, you can do so very definitively 
(as long as you always know that your internal code does *not* directly 
call the sensitive method - an easy thing to design for in most frameworks).

In fact you can usually traverse the stack to a fixed depth for this 
kind of thing, with one key exception that comes up in log frameworks. 
When you have one log API which forwards to another, you want to capture 
the "first" caller of any log API.  Pursuant to this, most log 
frameworks have log method variants which accept the fully-qualified 
class name of that first logger.  The moral equivalent to this scenario 
would likely be to provide an API variant which accepts a Class or 
ClassLoader (depending on the usage) and a variant which does not and 
uses a fixed-depth "reach" into the stack instead.

This IMO blows a hole in the whole idea of a single *public* @CS 
annotation, and in fact in public framework code, a depth indicator 
seems to be adequate and more or less problem-free for any purpose I've 
run across.  In private JDK code it's a different story because every 
caller-sensitive usage is in the same code base as the mechanism itself, 
so there is always a tight control and a guarantee that two different 
sensitive methods won't interfere at cross-purposes in this way.

> My feedback is that the getCallerClass() method seems to be adequate for
> all use cases except Groovy 2.x.  I would suggest traversing the stack
> trace (with the new getDeclaringClass method) be the alternative to
> adding getCallerClass(int) method.  Jochen and others - what do you think?
>
> On the @CS subject, if we keep @CallerSensitive in sun.reflect in JDK8,
> the new getCallerClass() method is a caller-sensitive method and if it
> is called by the system classes, it will enforce that the caller must be
> annotated with @s.r.CS; if it called by non-system classes, the VM will
> return the caller and no check on the annotation.  The implication is in
> the 292 method handles. The current behavior is that if a MethodHandle
> for a caller-sensitive method is requested, it will bind the method
> handle with the lookup class (i.e. as if it were called from an
> instruction contained in the lookup class).  There is no change to the
> current behavior since the current implementation only allows
> JVM_GetCallerClass be called from system classes.  If an app (e.g. Foo.m
> method) calls the new getCallerClass method, a MethodHandle for Foo.m
> will not bind with the lookup class.  This will need to get John Rose
> and other 292 members' feedback on it.
>
> This is my thinking so far.  Feedback and comments?
>
> Mandy


-- 
- DML