On Sep 2, 2013, at 9:59 AM, Alan Bateman wrote: > It was a non goal of JEP 176 to provide @CallerSensitive as a public API. So > the suggestion to start out small was to leave that out and focus on some of > the use-cases initially. I don't think this suggestion is unreasonable as it > allows the less controversial part to move forward.
To be blunt, this goal is incompatible with the need to make getCallerClass public. Either we need to remove CallerSensitive entirely (which I have nothing against), or we need to make it public. But we can't keep CallerSensitive private and make getCallerClass public. Now if you just have a general objection to making getCallerClass public, then please, state it. In fact, if anything, I'm actually in favor of removing CallerSensitive entirely. It prevents code compiled on Java 6 from using those two methods, since they can't be annotated with @CallerSensitive. On Sep 2, 2013, at 3:33 AM, Alan Bateman wrote: > From a quick scan then it appears that you've decided to ignore the security > concerns so I don't think anyone can (or should) sponsor this patch until > there is further discussion on the API and the security implications. > > To help things then one suggestion is start small and just focus on the > no-arg getCallerClass and the method to return the array of stack traces (be > it an extended StackTraceElement or the new StackTraceFrame that you are > proposing). The getCallerClass will need a permission check, as will the > method to get the extended stack trace. I think it would also be useful to > lay out the points on extending StackTraceEment vs. introducing a few > StackTraceFrame type. I didn't decide to ignore the security concerns, I just don't see any security concerns. As has been /clearly/ established in the last three months, frameworks have been using getCallerClass for years, if not a decade. It has never caused a security problem. Ever. If I'm wrong, please point out to me a specific vulnerability that this has led to. I have worked on this API for a MONTH. We discussed it for TWO months before that. I am a month behind on the deadline for my Java 8 / Java EE 7 book because I essentially dropped it to work on this. I don't have the ability to go back and start from the beginning on something that I had the understanding was generally agreed upon before I started. It's simply not an option. We /clearly/ outlined these use cases in multiple emails over the last three months: 1. The need to get the immediate caller class. 2. The need to get the caller class N levels back. 3. The need to get the entire current stack trace, except with Classes instead of class names. 4. The need to get the stack trace for a Throwable, except with Classes instead of class names. This API meets all of these needs. The one you suggest us starting over on does not. It also adds the ability to get single frames from the stack trace, because that's a natural extension of the above four use cases, and it was trivial to include. There is /no/ need to lay out the points of extending StackTraceElement versus adding StackTraceFrame. It was made /very/ clear to me by several people in June that changing/extending StackTraceElement was /not/ an option due to serialization issues. The very clear intent was for this change to have minimal/no impact on the existing APIs, and that's what I did. The exact interface I used for StackTraceFrame was present in many previous emails (and evolved over about a month of communications), and when I said that I was starting on a patch there were no specific objections to the API that I ever saw. Jörn's email links to several of these discussions. I'm not voicing any objection to any kind of security/permissions checks in these methods. Before I can pass judgement one way or another, I'd want to know 1) specifically what type of permissions check you are looking for, and 2) what you're looking to achieve with said permissions check. In general, doing things like instantiating a class or calling methods on a class using the Class involves security checks already. I don't see how an additional security check before retrieving a class would be helpful here. Additionally, nothing ever stops me from calling Object#getClass if I have an instance of an object; I may not be able to do anything with that Class (newInstance, invoke methods, etc.), but I can call getClass. On Sep 2, 2013, at 9:08 AM, Jörn Huxhorn wrote: > Nick also explained a performance issue in Thread#getStackTrace() and > Throwable#getStackTrace(). Keeping that in mind, I'm not sure if going for > "start small" (i.e. leaving that issue alone) would be a wise decision. > > The thing is that we really need a proper solution before API freeze on > 2013-10-10. This is a proper solution with regards to the API, with as little > side effects as possible (due to not changing StackTraceElement, Throwable or > Thread regarding declaringClass). I'd suggest to not restart the discussion > about whether adding this to StackTraceElement or putting it into a separate > class. We should instead try to discuss this existing API and clarify what > needs to be changed to get this approved. > > I hope I don't come across rude or something. The prospect of a Java 8 > without a proper replacement API for sun.reflect.Reflection#getCallerClass is > simply giving me serious creeps and time is very pressing if we want to pull > this off. All of that: my sentiments exactly. Nick On Sep 2, 2013, at 10:07 AM, Remi Forax wrote: > On 09/02/2013 03:56 PM, Tom Hawtin wrote: >> On 02/09/2013 09:33, Alan Bateman wrote: >> >>> From a quick scan then it appears that you've decided to ignore the >>> security concerns so I don't think anyone can (or should) sponsor this >>> patch until there is further discussion on the API and the security >>> implications. >> >> I've barely looked through the patch, let alone audited it. However: >> >> As the method appears to be mostly for tracing, performing a security check >> per invocation is likely to hurt performance. Unless it was caller sensitive >> (which seems to be equated with Bad Practice these days) it would also >> require a doPrivileged block. Much better to perform a single security check >> when acquiring a capability object, in a similar(ish) way to sun.misc.Unsafe >> or SharedSecrets. >> >> In order to work with less privileged code, it may also be worthwhile to >> provide a version of the capability object parameterised with a class >> loader. If code has access to a ClassLoader object, then it is consistent >> for it to be given access to Classes loaded by it or child loaders, and in >> practice from parent loaders (but not children of parents). >> >> Security relating to Class objects is "interesting". The genuine security >> issue with Class objects is that they have are an attenuation of the >> capability of their ClassLoader. There are many ways to acquire Class >> objects. Some have confusing defence-in-depth security; some, such as >> Object.getClass used for quick null checks, do not. >> >> Tom > > I wonder if a java.lang.invoke.MethodHandles.Lookup is not a better object > than a ClassLoader for a capability object. > > Rémi
