Skip to content

Commit b962ca3

Browse files
rjernstelasticmachine
rjernst
and
elasticmachine
authored
Remove PrivilegedOperations (#127726) (#127770)
With the SecurityManager gone, the PrivilegedOperations class is no longer needed, these operations can be called directly. Co-authored-by: Elastic Machine <[email protected]>
1 parent 2f66868 commit b962ca3

File tree

11 files changed

+40
-203
lines changed

11 files changed

+40
-203
lines changed

‎libs/core/src/test/java/org/elasticsearch/core/internal/provider/EmbeddedImplClassLoaderTests.java‎

Lines changed: 13 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@
99

1010
package org.elasticsearch.core.internal.provider;
1111

12+
import org.elasticsearch.core.IOUtils;
1213
import org.elasticsearch.core.Strings;
1314
import org.elasticsearch.core.SuppressForbidden;
1415
import org.elasticsearch.core.internal.provider.EmbeddedImplClassLoader.CompoundEnumeration;
1516
import org.elasticsearch.test.ESTestCase;
16-
import org.elasticsearch.test.PrivilegedOperations;
1717
import org.elasticsearch.test.compiler.InMemoryJavaCompiler;
1818
import org.elasticsearch.test.jar.JarUtils;
1919

@@ -195,13 +195,10 @@ private Object newFooBar(boolean enableMulti, int... versions) throws Exception
195195
Path outerJar = topLevelDir.resolve("impl.jar");
196196
JarUtils.createJarWithEntries(outerJar, jarEntries);
197197
URL[] urls = new URL[] { outerJar.toUri().toURL() };
198-
URLClassLoader parent = URLClassLoader.newInstance(urls, EmbeddedImplClassLoaderTests.class.getClassLoader());
199-
try {
198+
try (URLClassLoader parent = loader(urls)) {
200199
EmbeddedImplClassLoader loader = EmbeddedImplClassLoader.getInstance(parent, "x-foo");
201200
Class<?> c = loader.loadClass("p.FooBar");
202201
return c.getConstructor().newInstance();
203-
} finally {
204-
PrivilegedOperations.closeURLClassLoader(parent);
205202
}
206203
}
207204

@@ -245,8 +242,7 @@ public void testResourcesBasic() throws Exception {
245242
Path outerJar = topLevelDir.resolve("impl.jar");
246243
JarUtils.createJarWithEntriesUTF(outerJar, jarEntries);
247244
URL[] urls = new URL[] { outerJar.toUri().toURL() };
248-
URLClassLoader parent = URLClassLoader.newInstance(urls, EmbeddedImplClassLoaderTests.class.getClassLoader());
249-
try {
245+
try (URLClassLoader parent = loader(urls)) {
250246
EmbeddedImplClassLoader loader = EmbeddedImplClassLoader.getInstance(parent, "res");
251247
// resource in a valid java package dir
252248
URL url = loader.findResource("p/res.txt");
@@ -274,8 +270,6 @@ public void testResourcesBasic() throws Exception {
274270
hasToString(endsWith("impl.jar!/IMPL-JARS/res/zoo-impl.jar/A-C/res.txt"))
275271
)
276272
);
277-
} finally {
278-
PrivilegedOperations.closeURLClassLoader(parent);
279273
}
280274
}
281275

@@ -326,9 +320,7 @@ private void testResourcesParent(String resourcePath) throws Exception {
326320
containsInAnyOrder("Parent Resource", "Embedded Resource")
327321
);
328322
} finally {
329-
for (URLClassLoader closeable : closeables) {
330-
PrivilegedOperations.closeURLClassLoader(closeable);
331-
}
323+
IOUtils.close(closeables);
332324
}
333325
}
334326

@@ -463,9 +455,7 @@ private void testResourcesVersioned(String resourcePath, boolean enableMulti, in
463455
assertThat(new String(is.readAllBytes(), UTF_8), is("Hello World" + expectedVersion));
464456
}
465457
} finally {
466-
for (URLClassLoader closeable : closeables) {
467-
PrivilegedOperations.closeURLClassLoader(closeable);
468-
}
458+
IOUtils.close(closeables);
469459
}
470460
}
471461

@@ -493,8 +483,7 @@ public void testIDontHaveIt() throws Exception {
493483
Path outerJar = topLevelDir.resolve("impl.jar");
494484
JarUtils.createJarWithEntriesUTF(outerJar, jarEntries);
495485
URL[] urls = new URL[] { outerJar.toUri().toURL() };
496-
URLClassLoader parent = URLClassLoader.newInstance(urls, EmbeddedImplClassLoaderTests.class.getClassLoader());
497-
try {
486+
try (URLClassLoader parent = loader(urls)) {
498487
embedLoader = EmbeddedImplClassLoader.getInstance(parent, "res");
499488

500489
Class<?> c = embedLoader.loadClass("java.lang.Object");
@@ -514,8 +503,6 @@ public void testIDontHaveIt() throws Exception {
514503
expectThrows(NPE, () -> embedLoader.getResourceAsStream(null));
515504
expectThrows(NPE, () -> embedLoader.resources(null));
516505
expectThrows(NPE, () -> embedLoader.loadClass(null));
517-
} finally {
518-
PrivilegedOperations.closeURLClassLoader(parent);
519506
}
520507
}
521508

@@ -542,8 +529,7 @@ public void testLoadWithJarDependencies() throws Exception {
542529
JarUtils.createJarWithEntries(outerJar, jarEntries);
543530
URL[] urls = new URL[] { outerJar.toUri().toURL() };
544531

545-
URLClassLoader parent = URLClassLoader.newInstance(urls, EmbeddedImplClassLoaderTests.class.getClassLoader());
546-
try {
532+
try (URLClassLoader parent = loader(urls)) {
547533
EmbeddedImplClassLoader loader = EmbeddedImplClassLoader.getInstance(parent, "blah");
548534
Class<?> c = loader.loadClass("p.Foo");
549535
Object obj = c.getConstructor().newInstance();
@@ -555,8 +541,6 @@ public void testLoadWithJarDependencies() throws Exception {
555541
expectThrows(CNFE, () -> loader.loadClass("p.Unknown"));
556542
expectThrows(CNFE, () -> loader.loadClass("q.Unknown"));
557543
expectThrows(CNFE, () -> loader.loadClass("r.Unknown"));
558-
} finally {
559-
PrivilegedOperations.closeURLClassLoader(parent);
560544
}
561545
}
562546

@@ -577,18 +561,20 @@ public void testResourcesWithMultipleJars() throws Exception {
577561
Path outerJar = topLevelDir.resolve("impl.jar");
578562
JarUtils.createJarWithEntriesUTF(outerJar, jarEntries);
579563
URL[] urls = new URL[] { outerJar.toUri().toURL() };
580-
URLClassLoader parent = URLClassLoader.newInstance(urls, EmbeddedImplClassLoaderTests.class.getClassLoader());
581-
try {
564+
565+
try (URLClassLoader parent = loader(urls)) {
582566
EmbeddedImplClassLoader loader = EmbeddedImplClassLoader.getInstance(parent, "blah");
583567
var res = Collections.list(loader.getResources("res.txt"));
584568
assertThat(res, hasSize(3));
585569
List<String> l = res.stream().map(EmbeddedImplClassLoaderTests::urlToString).toList();
586570
assertThat(l, containsInAnyOrder("fooRes", "barRes", "bazRes"));
587-
} finally {
588-
PrivilegedOperations.closeURLClassLoader(parent);
589571
}
590572
}
591573

574+
private static URLClassLoader loader(URL[] urls) {
575+
return URLClassLoader.newInstance(urls, EmbeddedImplClassLoaderTests.class.getClassLoader());
576+
}
577+
592578
@SuppressForbidden(reason = "file urls")
593579
static String urlToString(URL url) {
594580
try {

‎libs/core/src/test/java/org/elasticsearch/core/internal/provider/ProviderLocatorTests.java‎

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@
1010
package org.elasticsearch.core.internal.provider;
1111

1212
import org.elasticsearch.test.ESTestCase;
13-
import org.elasticsearch.test.PrivilegedOperations;
1413
import org.elasticsearch.test.compiler.InMemoryJavaCompiler;
1514
import org.elasticsearch.test.jar.JarUtils;
1615

1716
import java.lang.module.ModuleDescriptor;
17+
import java.net.MalformedURLException;
1818
import java.net.URL;
1919
import java.net.URLClassLoader;
2020
import java.nio.file.Files;
@@ -117,12 +117,8 @@ public class FooIntSupplier implements java.util.function.IntSupplier {
117117
Path topLevelDir = createTempDir(getTestName());
118118
Path outerJar = topLevelDir.resolve("impl.jar");
119119
JarUtils.createJarWithEntries(outerJar, jarEntries);
120-
URLClassLoader parent = URLClassLoader.newInstance(
121-
new URL[] { outerJar.toUri().toURL() },
122-
ProviderLocatorTests.class.getClassLoader()
123-
);
124120

125-
try {
121+
try (URLClassLoader parent = loader(outerJar)) {
126122
// test scenario
127123
ProviderLocator<IntSupplier> locator = new ProviderLocator<>("x-foo", IntSupplier.class, parent, "x.foo.impl", Set.of(), true);
128124
IntSupplier impl = locator.get();
@@ -139,8 +135,6 @@ public class FooIntSupplier implements java.util.function.IntSupplier {
139135
assertThat(md.exports(), containsInAnyOrder(exportsOf("p")));
140136
assertThat(md.opens(), containsInAnyOrder(opensOf("q")));
141137
assertThat(md.packages(), containsInAnyOrder(equalTo("p"), equalTo("q"), equalTo("r")));
142-
} finally {
143-
PrivilegedOperations.closeURLClassLoader(parent);
144138
}
145139
}
146140

@@ -172,21 +166,15 @@ public class FooLongSupplier implements java.util.function.LongSupplier {
172166
Path topLevelDir = createTempDir(getTestName());
173167
Path outerJar = topLevelDir.resolve("impl.jar");
174168
JarUtils.createJarWithEntries(outerJar, jarEntries);
175-
URLClassLoader parent = URLClassLoader.newInstance(
176-
new URL[] { outerJar.toUri().toURL() },
177-
ProviderLocatorTests.class.getClassLoader()
178-
);
179169

180-
try {
170+
try (URLClassLoader parent = loader(outerJar)) {
181171
// test scenario
182172
ProviderLocator<LongSupplier> locator = new ProviderLocator<>("x-foo", LongSupplier.class, parent, "", Set.of(), false);
183173
LongSupplier impl = locator.get();
184174
assertThat(impl.getAsLong(), is(55L));
185175
assertThat(impl.toString(), equalTo("Hello from FooLongSupplier - non-modular!"));
186176
assertThat(impl.getClass().getName(), equalTo("p.FooLongSupplier"));
187177
assertThat(impl.getClass().getModule().isNamed(), is(false));
188-
} finally {
189-
PrivilegedOperations.closeURLClassLoader(parent);
190178
}
191179
}
192180

@@ -215,21 +203,18 @@ public class BarIntSupplier implements java.util.function.IntSupplier {
215203
Path pb = Files.createDirectories(barRoot.resolve("pb"));
216204
Files.write(pb.resolve("BarIntSupplier.class"), classToBytes.get("pb.BarIntSupplier"));
217205

218-
URLClassLoader parent = URLClassLoader.newInstance(
219-
new URL[] { topLevelDir.toUri().toURL() },
220-
ProviderLocatorTests.class.getClassLoader()
221-
);
222-
223-
try {
206+
try (URLClassLoader parent = loader(topLevelDir)) {
224207
// test scenario
225208
ProviderLocator<IntSupplier> locator = new ProviderLocator<>("y-bar", IntSupplier.class, parent, "", Set.of(), false);
226209
IntSupplier impl = locator.get();
227210
assertThat(impl.getAsInt(), is(42));
228211
assertThat(impl.toString(), equalTo("Hello from BarIntSupplier - exploded non-modular!"));
229212
assertThat(impl.getClass().getName(), equalTo("pb.BarIntSupplier"));
230213
assertThat(impl.getClass().getModule().isNamed(), is(false));
231-
} finally {
232-
PrivilegedOperations.closeURLClassLoader(parent);
233214
}
234215
}
216+
217+
private static URLClassLoader loader(Path jar) throws MalformedURLException {
218+
return URLClassLoader.newInstance(new URL[] { jar.toUri().toURL() }, ProviderLocatorTests.class.getClassLoader());
219+
}
235220
}

‎modules/lang-painless/spi/src/test/java/org/elasticsearch/painless/WhitelistLoaderTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ public void testMissingWhitelistResourceInModule() throws Exception {
132132
JarUtils.createJarWithEntries(jar, jarEntries);
133133

134134
try (var loader = JarUtils.loadJar(jar)) {
135-
Controller controller = JarUtils.loadModule(jar, loader.classloader(), "m");
135+
Controller controller = JarUtils.loadModule(jar, loader, "m");
136136
Module module = controller.layer().findModule("m").orElseThrow();
137137

138138
Class<?> ownerClass = module.getClassLoader().loadClass("p.TestOwner");

‎server/src/test/java/org/elasticsearch/plugins/ExtensionLoaderTests.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
package org.elasticsearch.plugins;
1111

1212
import org.elasticsearch.test.ESTestCase;
13-
import org.elasticsearch.test.PrivilegedOperations.ClosableURLClassLoader;
1413
import org.elasticsearch.test.compiler.InMemoryJavaCompiler;
1514
import org.elasticsearch.test.jar.JarUtils;
1615

@@ -35,7 +34,7 @@ public interface TestService {
3534
int getValue();
3635
}
3736

38-
private ClosableURLClassLoader buildProviderJar(Map<String, CharSequence> sources) throws Exception {
37+
private URLClassLoader buildProviderJar(Map<String, CharSequence> sources) throws Exception {
3938
var classToBytes = InMemoryJavaCompiler.compile(sources);
4039

4140
Map<String, byte[]> jarEntries = new HashMap<>();
@@ -55,7 +54,7 @@ private ClosableURLClassLoader buildProviderJar(Map<String, CharSequence> source
5554
JarUtils.createJarWithEntries(jar, jarEntries);
5655
URL[] urls = new URL[] { jar.toUri().toURL() };
5756

58-
return new ClosableURLClassLoader(URLClassLoader.newInstance(urls, this.getClass().getClassLoader()));
57+
return URLClassLoader.newInstance(urls, this.getClass().getClassLoader());
5958
}
6059

6160
private String defineProvider(String name, int value) {
@@ -79,7 +78,7 @@ public void testNoProvider() {
7978
public void testOneProvider() throws Exception {
8079
Map<String, CharSequence> sources = Map.of("p.FooService", defineProvider("FooService", 1));
8180
try (var loader = buildProviderJar(sources)) {
82-
TestService service = ExtensionLoader.loadSingleton(ServiceLoader.load(TestService.class, loader.classloader()))
81+
TestService service = ExtensionLoader.loadSingleton(ServiceLoader.load(TestService.class, loader))
8382
.orElseThrow(AssertionError::new);
8483
assertThat(service, not(nullValue()));
8584
assertThat(service.getValue(), equalTo(1));
@@ -96,7 +95,7 @@ public void testManyProviders() throws Exception {
9695
try (var loader = buildProviderJar(sources)) {
9796
var e = expectThrows(
9897
IllegalStateException.class,
99-
() -> ExtensionLoader.loadSingleton(ServiceLoader.load(TestService.class, loader.classloader()))
98+
() -> ExtensionLoader.loadSingleton(ServiceLoader.load(TestService.class, loader))
10099
);
101100
assertThat(e.getMessage(), containsString("More than one extension found"));
102101
assertThat(e.getMessage(), containsString("TestService"));

‎server/src/test/java/org/elasticsearch/plugins/PluginIntrospectorTests.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.elasticsearch.indices.recovery.plan.ShardSnapshotsService;
2525
import org.elasticsearch.ingest.Processor;
2626
import org.elasticsearch.test.ESTestCase;
27-
import org.elasticsearch.test.PrivilegedOperations;
2827
import org.elasticsearch.test.compiler.InMemoryJavaCompiler;
2928
import org.elasticsearch.test.jar.JarUtils;
3029

@@ -233,11 +232,8 @@ public final class FooPlugin extends q.AbstractFooPlugin { }
233232
JarUtils.createJarWithEntries(jar, jarEntries);
234233
URL[] urls = new URL[] { jar.toUri().toURL() };
235234

236-
URLClassLoader loader = URLClassLoader.newInstance(urls, PluginIntrospectorTests.class.getClassLoader());
237-
try {
235+
try (URLClassLoader loader = URLClassLoader.newInstance(urls, PluginIntrospectorTests.class.getClassLoader())) {
238236
assertThat(pluginIntrospector.interfaces(loader.loadClass("r.FooPlugin")), contains("ActionPlugin"));
239-
} finally {
240-
PrivilegedOperations.closeURLClassLoader(loader);
241237
}
242238
}
243239

‎server/src/test/java/org/elasticsearch/plugins/PluginsLoaderTests.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import org.elasticsearch.nativeaccess.NativeAccessUtil;
2020
import org.elasticsearch.plugin.analysis.CharFilterFactory;
2121
import org.elasticsearch.test.ESTestCase;
22-
import org.elasticsearch.test.PrivilegedOperations;
2322
import org.elasticsearch.test.compiler.InMemoryJavaCompiler;
2423
import org.elasticsearch.test.jar.JarUtils;
2524

@@ -351,13 +350,13 @@ static void closePluginLoaders(PluginsLoader pluginsLoader) {
351350
pluginsLoader.pluginLayers().forEach(lp -> {
352351
if (lp.pluginClassLoader() instanceof URLClassLoader urlClassLoader) {
353352
try {
354-
PrivilegedOperations.closeURLClassLoader(urlClassLoader);
353+
urlClassLoader.close();
355354
} catch (IOException unexpected) {
356355
throw new UncheckedIOException(unexpected);
357356
}
358357
} else if (lp.pluginClassLoader() instanceof UberModuleClassLoader loader) {
359358
try {
360-
PrivilegedOperations.closeURLClassLoader(loader.getInternalLoader());
359+
loader.getInternalLoader().close();
361360
} catch (Exception e) {
362361
throw new RuntimeException(e);
363362
}

‎server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.elasticsearch.plugins.spi.BarTestService;
2525
import org.elasticsearch.plugins.spi.TestService;
2626
import org.elasticsearch.test.ESTestCase;
27-
import org.elasticsearch.test.PrivilegedOperations;
2827
import org.elasticsearch.test.compiler.InMemoryJavaCompiler;
2928
import org.elasticsearch.test.jar.JarUtils;
3029

@@ -671,9 +670,11 @@ public String name() {
671670
}
672671

673672
public void testLoadServiceProviders() throws Exception {
674-
URLClassLoader fakeClassLoader = buildTestProviderPlugin("integer");
675-
URLClassLoader fakeClassLoader1 = buildTestProviderPlugin("string");
676-
try {
673+
674+
try (
675+
URLClassLoader fakeClassLoader = buildTestProviderPlugin("integer");
676+
URLClassLoader fakeClassLoader1 = buildTestProviderPlugin("string")
677+
) {
677678
@SuppressWarnings("unchecked")
678679
Class<? extends Plugin> fakePluginClass = (Class<? extends Plugin>) fakeClassLoader.loadClass("r.FooPlugin");
679680
@SuppressWarnings("unchecked")
@@ -699,9 +700,6 @@ public void testLoadServiceProviders() throws Exception {
699700
providers = service.loadServiceProviders(TestService.class);
700701

701702
assertEquals(0, providers.size());
702-
} finally {
703-
PrivilegedOperations.closeURLClassLoader(fakeClassLoader);
704-
PrivilegedOperations.closeURLClassLoader(fakeClassLoader1);
705703
}
706704
}
707705

@@ -877,13 +875,13 @@ static void closePluginLoaders(PluginsService pluginService) {
877875
for (var lp : pluginService.plugins()) {
878876
if (lp.classLoader() instanceof URLClassLoader urlClassLoader) {
879877
try {
880-
PrivilegedOperations.closeURLClassLoader(urlClassLoader);
878+
urlClassLoader.close();
881879
} catch (IOException unexpected) {
882880
throw new UncheckedIOException(unexpected);
883881
}
884882
} else if (lp.classLoader() instanceof UberModuleClassLoader loader) {
885883
try {
886-
PrivilegedOperations.closeURLClassLoader(loader.getInternalLoader());
884+
loader.getInternalLoader().close();
887885
} catch (Exception e) {
888886
throw new RuntimeException(e);
889887
}

‎test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,9 @@
1616
import org.elasticsearch.core.Booleans;
1717
import org.elasticsearch.core.PathUtils;
1818
import org.elasticsearch.jdk.JarHell;
19-
import org.elasticsearch.test.PrivilegedOperations;
2019
import org.elasticsearch.test.mockito.SecureMockMaker;
2120

2221
import java.io.IOException;
23-
import java.lang.invoke.MethodHandles;
2422
import java.nio.file.Files;
2523
import java.nio.file.Path;
2624
import java.util.Objects;
@@ -75,13 +73,6 @@ public class BootstrapForTesting {
7573
// init mockito
7674
SecureMockMaker.init();
7775

78-
// init the privileged operation
79-
try {
80-
MethodHandles.publicLookup().ensureInitialized(PrivilegedOperations.class);
81-
} catch (IllegalAccessException unexpected) {
82-
throw new AssertionError(unexpected);
83-
}
84-
8576
// Log ifconfig output before SecurityManager is installed
8677
IfConfig.logIfNecessary();
8778
}

0 commit comments

Comments
 (0)