diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 442aefdbae5..a9c2648e4fc 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -62,6 +62,9 @@ Improvements previously treated as beginning a line-ending comment (Satoshi Kato and Masaru Hasegawa via Michael Sokolov) +* LUCENE-9109: Use StackWalker to implement TestSecurityManager's detection + of JVM exit (Uwe Schindler) + Bug fixes * LUCENE-8663: NRTCachingDirectory.slowFileExists may open a file while @@ -90,6 +93,9 @@ Improvements * LUCENE-9091: UnifiedHighlighter HTML escaping should only escape essentials (Nándor Mátravölgyi) +* LUCENE-9109: Backport some changes from master (except StackWalker) to improve + TestSecurityManager (Uwe Schindler) + Optimizations --------------------- (No changes) diff --git a/lucene/test-framework/src/java/org/apache/lucene/util/TestSecurityManager.java b/lucene/test-framework/src/java/org/apache/lucene/util/TestSecurityManager.java index a164a9cf5c0..2eaccbe6d91 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/util/TestSecurityManager.java +++ b/lucene/test-framework/src/java/org/apache/lucene/util/TestSecurityManager.java @@ -16,8 +16,9 @@ */ package org.apache.lucene.util; -import java.security.AccessController; -import java.security.PrivilegedAction; +import java.lang.StackWalker.StackFrame; +import java.util.Locale; +import java.util.function.Predicate; /** * A {@link SecurityManager} that prevents tests calling {@link System#exit(int)}. @@ -28,11 +29,14 @@ import java.security.PrivilegedAction; */ public final class TestSecurityManager extends SecurityManager { - static final String JUNIT4_TEST_RUNNER_PACKAGE = "com.carrotsearch.ant.tasks.junit4."; - static final String ECLIPSE_TEST_RUNNER_PACKAGE = "org.eclipse.jdt.internal.junit.runner."; - static final String IDEA_TEST_RUNNER_PACKAGE = "com.intellij.rt.execution.junit."; - static final String GRADLE_TEST_RUNNER_PACKAGE = "worker.org.gradle.process.internal.worker"; + private static final String JUNIT4_TEST_RUNNER_PACKAGE = "com.carrotsearch.ant.tasks.junit4."; + private static final String ECLIPSE_TEST_RUNNER_PACKAGE = "org.eclipse.jdt.internal.junit.runner."; + private static final String IDEA_TEST_RUNNER_PACKAGE = "com.intellij.rt.execution.junit."; + private static final String GRADLE_TEST_RUNNER_PACKAGE = "worker.org.gradle.process.internal.worker."; + private static final String SYSTEM_CLASS_NAME = System.class.getName(); + private static final String RUNTIME_CLASS_NAME = Runtime.class.getName(); + /** * Creates a new TestSecurityManager. This ctor is called on JVM startup, * when {@code -Djava.security.manager=org.apache.lucene.util.TestSecurityManager} @@ -41,7 +45,7 @@ public final class TestSecurityManager extends SecurityManager { public TestSecurityManager() { super(); } - + /** * {@inheritDoc} *

This method inspects the stack trace and checks who is calling @@ -50,46 +54,27 @@ public final class TestSecurityManager extends SecurityManager { */ @Override public void checkExit(final int status) { - AccessController.doPrivileged(new PrivilegedAction() { - @Override - public Void run() { - final String systemClassName = System.class.getName(), - runtimeClassName = Runtime.class.getName(); - String exitMethodHit = null; - for (final StackTraceElement se : Thread.currentThread().getStackTrace()) { - final String className = se.getClassName(), methodName = se.getMethodName(); - if ( - ("exit".equals(methodName) || "halt".equals(methodName)) && - (systemClassName.equals(className) || runtimeClassName.equals(className)) - ) { - exitMethodHit = className + '#' + methodName + '(' + status + ')'; - continue; - } - - if (exitMethodHit != null) { - if (className.startsWith(JUNIT4_TEST_RUNNER_PACKAGE) || - className.startsWith(ECLIPSE_TEST_RUNNER_PACKAGE) || - className.startsWith(IDEA_TEST_RUNNER_PACKAGE) || - className.startsWith(GRADLE_TEST_RUNNER_PACKAGE)) { - // this exit point is allowed, we return normally from closure: - return /*void*/ null; - } else { - // anything else in stack trace is not allowed, break and throw SecurityException below: - break; - } - } - } - - if (exitMethodHit == null) { - // should never happen, only if JVM hides stack trace - replace by generic: - exitMethodHit = "JVM exit method"; - } - throw new SecurityException(exitMethodHit + " calls are not allowed because they terminate the test runner's JVM."); - } - }); - + if (StackWalker.getInstance().walk(s -> s + .dropWhile(Predicate.not(TestSecurityManager::isExitStackFrame)) // skip all internal stack frames + .dropWhile(TestSecurityManager::isExitStackFrame) // skip all exit()/halt() stack frames + .limit(1) // only look at one more frame (caller of exit) + .map(StackFrame::getClassName) + .noneMatch(c -> c.startsWith(JUNIT4_TEST_RUNNER_PACKAGE) || + c.startsWith(ECLIPSE_TEST_RUNNER_PACKAGE) || + c.startsWith(IDEA_TEST_RUNNER_PACKAGE) || + c.startsWith(GRADLE_TEST_RUNNER_PACKAGE)))) { + throw new SecurityException(String.format(Locale.ENGLISH, + "System/Runtime.exit(%1$d) or halt(%1$d) calls are not allowed because they terminate the test runner's JVM.", + status)); + } // we passed the stack check, delegate to super, so default policy can still deny permission: super.checkExit(status); } + private static boolean isExitStackFrame(StackFrame f) { + final String methodName = f.getMethodName(), className = f.getClassName(); + return ("exit".equals(methodName) || "halt".equals(methodName)) && + (SYSTEM_CLASS_NAME.equals(className) || RUNTIME_CLASS_NAME.equals(className)); + } + }