Skip to content

Commit a3ac746

Browse files
authored
Replace deprecated AccessController in :server (#19239)
Use the drop-in replacement for the deprecated AccessController where appropriate. Also remove tests that were assuming a security manager was present as that can never be true post JDK 21. Signed-off-by: Andrew Ross <[email protected]>
1 parent 2c38321 commit a3ac746

File tree

7 files changed

+21
-139
lines changed

7 files changed

+21
-139
lines changed

server/src/main/java/org/opensearch/bootstrap/OpenSearchUncaughtExceptionHandler.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,9 @@
3636
import org.apache.logging.log4j.Logger;
3737
import org.opensearch.cli.Terminal;
3838
import org.opensearch.common.SuppressForbidden;
39+
import org.opensearch.secure_sm.AccessController;
3940

4041
import java.io.IOError;
41-
import java.security.AccessController;
42-
import java.security.PrivilegedAction;
4342

4443
/**
4544
* UncaughtException Handler used during bootstrapping
@@ -98,12 +97,11 @@ void onNonFatalUncaught(final String threadName, final Throwable t) {
9897
Terminal.DEFAULT.flush();
9998
}
10099

101-
@SuppressWarnings("removal")
102100
void halt(int status) {
103101
AccessController.doPrivileged(new PrivilegedHaltAction(status));
104102
}
105103

106-
static class PrivilegedHaltAction implements PrivilegedAction<Void> {
104+
static class PrivilegedHaltAction implements Runnable {
107105

108106
private final int status;
109107

@@ -113,12 +111,9 @@ private PrivilegedHaltAction(final int status) {
113111

114112
@SuppressForbidden(reason = "halt")
115113
@Override
116-
public Void run() {
114+
public void run() {
117115
// we halt to prevent shutdown hooks from running
118116
Runtime.getRuntime().halt(status);
119-
return null;
120117
}
121-
122118
}
123-
124119
}

server/src/main/java/org/opensearch/common/util/concurrent/ThreadContextAccess.java

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,32 +10,28 @@
1010

1111
import org.opensearch.SpecialPermission;
1212
import org.opensearch.common.annotation.InternalApi;
13+
import org.opensearch.secure_sm.AccessController;
1314

14-
import java.security.AccessController;
15-
import java.security.PrivilegedAction;
15+
import java.util.function.Supplier;
1616

1717
/**
1818
* This class wraps the {@link ThreadContext} operations requiring access in
19-
* {@link AccessController#doPrivileged(PrivilegedAction)} blocks.
19+
* {@link AccessController#doPrivileged} blocks.
2020
*
2121
* @opensearch.internal
2222
*/
23-
@SuppressWarnings("removal")
2423
@InternalApi
2524
public final class ThreadContextAccess {
2625

2726
private ThreadContextAccess() {}
2827

29-
public static <T> T doPrivileged(PrivilegedAction<T> operation) {
28+
public static <T> T doPrivileged(Supplier<T> operation) {
3029
SpecialPermission.check();
3130
return AccessController.doPrivileged(operation);
3231
}
3332

3433
public static void doPrivilegedVoid(Runnable action) {
3534
SpecialPermission.check();
36-
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
37-
action.run();
38-
return null;
39-
});
35+
AccessController.doPrivileged(action);
4036
}
4137
}

server/src/main/java/org/opensearch/index/store/remote/utils/TransferManager.java

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.opensearch.index.store.remote.filecache.CachedIndexInput;
1717
import org.opensearch.index.store.remote.filecache.FileCache;
1818
import org.opensearch.index.store.remote.filecache.FileCachedIndexInput;
19+
import org.opensearch.secure_sm.AccessController;
1920
import org.opensearch.threadpool.ThreadPool;
2021

2122
import java.io.BufferedOutputStream;
@@ -25,9 +26,6 @@
2526
import java.io.UncheckedIOException;
2627
import java.nio.file.Files;
2728
import java.nio.file.Path;
28-
import java.security.AccessController;
29-
import java.security.PrivilegedActionException;
30-
import java.security.PrivilegedExceptionAction;
3129
import java.util.concurrent.CompletableFuture;
3230
import java.util.concurrent.CompletionException;
3331
import java.util.concurrent.Executor;
@@ -78,7 +76,7 @@ public IndexInput fetchBlob(BlobFetchRequest blobFetchRequest) throws IOExceptio
7876
logger.trace("fetchBlob called for {}", key.toString());
7977

8078
try {
81-
return AccessController.doPrivileged((PrivilegedExceptionAction<IndexInput>) () -> {
79+
return AccessController.doPrivilegedChecked(() -> {
8280
CachedIndexInput cacheEntry = fileCache.compute(key, (path, cachedIndexInput) -> {
8381
if (cachedIndexInput == null || cachedIndexInput.isClosed()) {
8482
logger.trace("Transfer Manager - IndexInput closed or not in cache");
@@ -100,14 +98,13 @@ public IndexInput fetchBlob(BlobFetchRequest blobFetchRequest) throws IOExceptio
10098
fileCache.decRef(key);
10199
}
102100
});
103-
} catch (PrivilegedActionException e) {
104-
final Exception cause = e.getException();
105-
if (cause instanceof IOException) {
106-
throw (IOException) cause;
107-
} else if (cause instanceof RuntimeException) {
108-
throw (RuntimeException) cause;
101+
} catch (Exception e) {
102+
if (e instanceof IOException) {
103+
throw (IOException) e;
104+
} else if (e instanceof RuntimeException) {
105+
throw (RuntimeException) e;
109106
} else {
110-
throw new IOException(cause);
107+
throw new IOException(e);
111108
}
112109
}
113110
}

server/src/main/java/org/opensearch/plugins/PluginsService.java

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,6 @@
6464
import java.nio.file.DirectoryStream;
6565
import java.nio.file.Files;
6666
import java.nio.file.Path;
67-
import java.security.AccessController;
68-
import java.security.PrivilegedAction;
6967
import java.util.ArrayList;
7068
import java.util.Collection;
7169
import java.util.Collections;
@@ -799,10 +797,7 @@ private Plugin loadBundle(Bundle bundle, Map<String, Plugin> loaded) {
799797
// Set context class loader to plugin's class loader so that plugins
800798
// that have dependencies with their own SPI endpoints have a chance to load
801799
// and initialize them appropriately.
802-
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
803-
Thread.currentThread().setContextClassLoader(loader);
804-
return null;
805-
});
800+
Thread.currentThread().setContextClassLoader(loader);
806801

807802
logger.debug("Loading plugin [" + name + "]...");
808803
Class<? extends Plugin> pluginClass = loadPluginClass(bundle.plugin.getClassname(), loader);
@@ -821,10 +816,7 @@ private Plugin loadBundle(Bundle bundle, Map<String, Plugin> loaded) {
821816
loaded.put(name, plugin);
822817
return plugin;
823818
} finally {
824-
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
825-
Thread.currentThread().setContextClassLoader(cl);
826-
return null;
827-
});
819+
Thread.currentThread().setContextClassLoader(cl);
828820
}
829821
}
830822

server/src/main/java/org/opensearch/search/lookup/LeafDocLookup.java

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,9 @@
3838
import org.opensearch.index.fielddata.ScriptDocValues;
3939
import org.opensearch.index.mapper.MappedFieldType;
4040
import org.opensearch.index.mapper.MapperService;
41+
import org.opensearch.secure_sm.AccessController;
4142

4243
import java.io.IOException;
43-
import java.security.AccessController;
44-
import java.security.PrivilegedAction;
4544
import java.util.Collection;
4645
import java.util.HashMap;
4746
import java.util.Map;
@@ -91,12 +90,7 @@ public ScriptDocValues<?> get(Object key) {
9190
}
9291
// load fielddata on behalf of the script: otherwise it would need additional permissions
9392
// to deal with pagedbytes/ramusagestimator/etc
94-
scriptValues = AccessController.doPrivileged(new PrivilegedAction<ScriptDocValues<?>>() {
95-
@Override
96-
public ScriptDocValues<?> run() {
97-
return fieldDataLookup.apply(fieldType).load(reader).getScriptValues();
98-
}
99-
});
93+
scriptValues = AccessController.doPrivileged(() -> fieldDataLookup.apply(fieldType).load(reader).getScriptValues());
10094
localCacheFieldData.put(fieldName, scriptValues);
10195
}
10296
try {

server/src/test/java/org/opensearch/bootstrap/OpenSearchPolicyTests.java

Lines changed: 0 additions & 77 deletions
This file was deleted.

server/src/test/java/org/opensearch/bootstrap/SecurityTests.java

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@
3939
import java.net.URL;
4040
import java.nio.file.Files;
4141
import java.nio.file.Path;
42-
import java.security.AccessController;
43-
import java.security.PrivilegedAction;
4442
import java.util.Map;
4543

4644
public class SecurityTests extends OpenSearchTestCase {
@@ -76,17 +74,6 @@ public void testEnsureRegularFile() throws IOException {
7674
} catch (IOException expected) {}
7775
}
7876

79-
/** can't execute processes */
80-
@SuppressWarnings("removal")
81-
public void testProcessExecution() throws Exception {
82-
assumeTrue("test requires security manager", System.getSecurityManager() != null);
83-
try {
84-
Runtime.getRuntime().exec(new String[] { "ls" });
85-
fail("didn't get expected exception");
86-
} catch (SecurityException expected) {}
87-
}
88-
89-
@SuppressWarnings("removal")
9077
public void testReadPolicyWithCodebases() throws IOException {
9178
final Map<String, URL> codebases = Map.of(
9279
"test-netty-tcnative-boringssl-static-2.0.61.Final-linux-x86_64.jar",
@@ -101,8 +88,6 @@ public void testReadPolicyWithCodebases() throws IOException {
10188
URI.create("file://test-zstd-jni-1.5.6-1.jar").toURL()
10289
);
10390

104-
AccessController.doPrivileged(
105-
(PrivilegedAction<?>) () -> Security.readPolicy(SecurityTests.class.getResource("test-codebases.policy"), codebases)
106-
);
91+
Security.readPolicy(SecurityTests.class.getResource("test-codebases.policy"), codebases);
10792
}
10893
}

0 commit comments

Comments
 (0)