[CALCITE-6590] Use reflection to handle Java SecurityManager deprecation in Avatica
Also bump ByteBuddy version to 1.15.1
This is the alternate patch using reflection.
Have you been able to check this @julianhyde ?
The approach here is directly lifted from Jetty (pruning some unused functions)
rebased on main
I realized my suggestions for SecurityUtils were getting a little involved so I coded up the result and PRed here: https://github.com/stoty/calcite-avatica/pull/1
I have applied your PR, and made some minor changes. These are in different commits. Please check them @chrisdennis .
I have changed the name back to sneakyThrow().
Since we're going for perfection, we may also want to reconsider callAs().
Subject callAs() throws Exceptions, and the fallback path now also throws exceptions (wrapped in PrivilegedActionException).
We could change it to something like :
public static <T> T callAs(Subject subject, Callable<T> action) throws Exception {
try {
return (T) CALL_AS.invoke(subject, action);
} catch (PrivilegedActionException e) {
e.getException();
} catch (Exception e) {
throw e;
} catch (Throwable t) {
throw sneakyThrow(t);
}
}
Since we're going for perfection, we may also want to reconsider callAs().
Apologies for triggering some (all?) of this bike-shedding.
Subject callAs() throws Exceptions, and the fallback path now also throws exceptions (wrapped in PrivilegedActionException).
I'm confused, Subject.callAs(...) throws CompletionException if the action throws, which is what the current code does, unwrapping any PrivilegedActionException and rewrapping it in case we're backed by the old doAs(...) method. The only tweak I'd make to the current code is that the Exception catch and rethrow is redundant given the sneakyThrow(t) call.
I'm confused, Subject.callAs(...) throws
CompletionExceptionif the action throws, which is what the current code does, unwrapping any PrivilegedActionException and rewrapping it in case we're backed by the olddoAs(...)method. The only tweak I'd make to the current code is that theExceptioncatch and rethrow is redundant given thesneakyThrow(t)call.
You are right.
However, since Subject.callAs() explicitly throws CompletionException, shouldn't SecurityUtils.callAs() also explictly throw CompletionException ?
However, since Subject.callAs() explicitly throws CompletionException, shouldn't SecurityUtils.callAs() also explictly throw CompletionException ?
Yes, that would make sense.
This is as polished as it gets now. Can you do a final review @zabetak ?
Merged manually. (Maybe we could enable squash merges...)
Thanks for getting this in @stoty, @chrisdennis and @zabetak!