On Jun 16, 2013, at 1:25 AM, Jeroen Frijters wrote:
> Nick Williams wrote:
>> I'm going to stick by my original assessment that I'm not convinced
>> there's a security issue. It's possible that getClassContext() filters
>> out classes the caller can't access, but nothing in the documentation
>> indicates that's the case, so I'm operating under the assumption that it
>> doesn't.
>
> SecurityManager.getClassContext() is not available to unpriviliged callers,
> so I don't think this a valid argument.
Can you define what "is not available to unprivileged callers" means a little
better? We use it in Log4j 2 when getCallerClass() isn't available, and it
works just fine. I took a look at the Java code, and it's native, so there's no
check there. (You can't instantiate a SecurityManager unless there's not
currently a SecurityManager installed _OR_ the installed SecurityManager allows
the creation (installation privilege isn't checked) of another SecurityManager.
Is that what you're talking about?) I took a look at the native code, and I
don't see anything there that would represent a privilege check that I
recognized, but if I understood what you meant a little better I might see
where I'm wrong.
> Given the security implications, the serialization issue and the need for
> weak references, it seems to me that adding this to Throwable would be way
> too expensive.
The only expense I'm seeing here is the security implications, and I'm still
not convinced that they exist. We have all agreed now that making this
transient is okay, and adding the transient keyword to a field only takes 5
seconds. And I certainly don't believe that declaring the field as
"WeakReference<Class<?>> elementClass" instead of "Class<?> elementClass" takes
more than 30 seconds longer, but maybe I'm missing something, so that's not
expensive either.
Also, to be clear, I'm not suggesting adding it to Throwable specifically, I'm
suggesting adding it to StackTraceElement (which would apply anywhere that a
stack trace is filled in, including Throwable). A stack trace is filled in with
one native code method, so updating that method will make this work in
Throwable, Thread, and anywhere else that you can get a stack trace. I know
this is a nitpicky distinction, but it could be important.
>
> Adding a new API to collect a stack trace seems like a far better approach.
>
> Here's my off the cuff proposal:
>
> public final class StackFrame {
> public Executable method();
> public String getFileName();
> public int getLineNumber();
>
> public static StackFrame[] capture(int skipFrames, int maxLength, boolean
> includeSourceInfo);
> }
>
I'm not opposed to the StackFrame class as suggested, it just doesn't meet my
needs. At all. I can already successfully get this information using the stack
trace and getClassContext(), so I wouldn't be compelled to switch. I need to
efficiently get the Class from a stack trace generated by a Throwable, which
this does not solve. Right now I'm having to resort to getting that Class using
inefficient methods.
Nick
