[CALCITE-6590] Use reflection to handle Java SecurityManager deprecation in Avatica

[stoty]

Also bump ByteBuddy version to 1.15.1

[stoty]

This is the alternate patch using reflection.

[stoty]

Have you been able to check this @julianhyde ?

[stoty]

The approach here is directly lifted from Jetty (pruning some unused functions)

[stoty]

rebased on main

[chrisdennis]

Since community review was requested I figured I'd cast an eye over this. The usage of SecurityUtils looks fine to me... the class itself has some issues for me though. Not sure how you want to approach that though, and obviously feel free to dismiss all of this if desired.

[chrisdennis]

I realized my suggestions for SecurityUtils were getting a little involved so I coded up the result and PRed here: stoty#1

[stoty]

I have applied your PR, and made some minor changes. These are in different commits.
Please check them @chrisdennis .

[chrisdennis]

Looks good to me... you rightly point out that what we have makes some assumptions about the order of any future removals but I think they're reasonable assumptions.

[zabetak]

Few small comments and I think we are ready to merge this.

[stoty]

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);
    }
  }

[chrisdennis]

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.

[stoty]

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.

You are right.

However, since Subject.callAs() explicitly throws CompletionException, shouldn't SecurityUtils.callAs() also explictly throw CompletionException ?

[chrisdennis]

However, since Subject.callAs() explicitly throws CompletionException, shouldn't SecurityUtils.callAs() also explictly throw CompletionException ?

Yes, that would make sense.

[stoty]

This is as polished as it gets now. Can you do a final review @zabetak ?

[zabetak]

The changes in the PR LGTM!

I also run ./gradlew build with JDK23 locally and it passed without problems so I guess we are good to merge this.

[stoty]

Merged manually.
(Maybe we could enable squash merges...)

[F21]

Thanks for getting this in @stoty, @chrisdennis and @zabetak!