Skip to content

Commit ff0abac

Browse files
authored
Remove cap on Java version used by forbidden APIs (opensearch-project#19163)
There was a check in the forbidden APIs plugin to cap the Java version at 14, presumably from ages ago when the plugin did not support Java 15 or newer. That means we have not been enforcing any Java deprecations in the past 5 years. This removes that check, updates the plugin to 3.9, and fixes all the resulting deprecation failures. Signed-off-by: Andrew Ross <andrross@amazon.com>
1 parent 074b9d3 commit ff0abac

File tree

28 files changed

+104
-103
lines changed

28 files changed

+104
-103
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
2121
- Adding ScriptedAvg class to painless spi to allowlist usage from plugins ([#19006](https://github.com/opensearch-project/OpenSearch/pull/19006))
2222
- Replace centos:8 with almalinux:8 since centos docker images are deprecated ([#19154](https://github.com/opensearch-project/OpenSearch/pull/19154))
2323
- Add CompletionStage variants to IndicesAdminClient as an alternative to ActionListener ([#19161](https://github.com/opensearch-project/OpenSearch/pull/19161))
24+
- Remove cap on Java version used by forbidden APIs ([#19163](https://github.com/opensearch-project/OpenSearch/pull/19163))
2425

2526
### Fixed
2627
- Fix unnecessary refreshes on update preparation failures ([#15261](https://github.com/opensearch-project/OpenSearch/issues/15261))

buildSrc/build.gradle

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ dependencies {
114114
api 'com.gradleup.shadow:shadow-gradle-plugin:8.3.5'
115115
api 'org.jdom:jdom2:2.0.6.1'
116116
api "org.jetbrains.kotlin:kotlin-stdlib-jdk8:${props.getProperty('kotlin')}"
117-
api 'de.thetaphi:forbiddenapis:3.8'
117+
api 'de.thetaphi:forbiddenapis:3.9'
118118
api 'com.avast.gradle:gradle-docker-compose-plugin:0.17.12'
119119
api "org.yaml:snakeyaml:${props.getProperty('snakeyaml')}"
120120
api 'org.apache.maven:maven-model:3.9.6'

buildSrc/src/main/java/org/opensearch/gradle/precommit/ForbiddenApisPrecommitPlugin.java

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import org.opensearch.gradle.ExportOpenSearchBuildResourcesTask;
4141
import org.opensearch.gradle.info.BuildParams;
4242
import org.opensearch.gradle.util.GradleUtils;
43-
import org.gradle.api.JavaVersion;
4443
import org.gradle.api.Project;
4544
import org.gradle.api.Task;
4645
import org.gradle.api.plugins.ExtraPropertiesExtension;
@@ -53,6 +52,7 @@
5352
import java.util.Arrays;
5453
import java.util.HashSet;
5554
import java.util.List;
55+
import java.util.Set;
5656

5757
public class ForbiddenApisPrecommitPlugin extends PrecommitPlugin {
5858
@Override
@@ -89,10 +89,6 @@ public TaskProvider<? extends Task> createTask(Project project) {
8989
t.setClasspath(project.files(sourceSet.getRuntimeClasspath()).plus(sourceSet.getCompileClasspath()));
9090

9191
t.setTargetCompatibility(BuildParams.getRuntimeJavaVersion().getMajorVersion());
92-
if (BuildParams.getRuntimeJavaVersion().compareTo(JavaVersion.VERSION_14) > 0) {
93-
// TODO: forbidden apis does not yet support java 15, rethink using runtime version
94-
t.setTargetCompatibility(JavaVersion.VERSION_14.getMajorVersion());
95-
}
9692
t.setBundledSignatures(new HashSet<>(Arrays.asList("jdk-unsafe", "jdk-deprecated", "jdk-non-portable", "jdk-system-out")));
9793
t.setSignaturesFiles(
9894
project.files(
@@ -140,6 +136,18 @@ public Void call(Object... names) {
140136
return null;
141137
}
142138
});
139+
// Use of the deprecated security manager APIs are pervasive so set them to warn
140+
// globally for all projects. Replacements for (most of) these APIs are available
141+
// so usages can move to the non-deprecated variants to avoid the warnings.
142+
t.setSignaturesWithSeverityWarn(
143+
Set.of(
144+
"java.security.AccessController",
145+
"java.security.AccessControlContext",
146+
"java.lang.System#getSecurityManager()",
147+
"java.lang.SecurityManager",
148+
"java.security.Policy"
149+
)
150+
);
143151
});
144152
TaskProvider<Task> forbiddenApis = project.getTasks().named("forbiddenApis");
145153
forbiddenApis.configure(t -> t.setGroup(""));

distribution/tools/plugin-cli/src/main/java/org/opensearch/tools/cli/plugin/InstallPluginCommand.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ private String getMavenUrl(Terminal terminal, String[] coordinates, String platf
399399
@SuppressForbidden(reason = "Make HEAD request using URLConnection.connect()")
400400
boolean urlExists(Terminal terminal, String urlString) throws IOException {
401401
terminal.println(VERBOSE, "Checking if url exists: " + urlString);
402-
URL url = new URL(urlString);
402+
URL url = URI.create(urlString).toURL();
403403
assert "https".equals(url.getProtocol()) : "Use of https protocol is required";
404404
HttpURLConnection urlConnection = (HttpURLConnection) url.openConnection();
405405
urlConnection.addRequestProperty("User-Agent", "opensearch-plugin-installer");
@@ -427,7 +427,7 @@ private List<String> checkMisspelledPlugin(String pluginId) {
427427
@SuppressForbidden(reason = "We use getInputStream to download plugins")
428428
Path downloadZip(Terminal terminal, String urlString, Path tmpDir, boolean isBatch) throws IOException {
429429
terminal.println(VERBOSE, "Retrieving zip from " + urlString);
430-
URL url = new URL(urlString);
430+
URL url = URI.create(urlString).toURL();
431431
Path zip = Files.createTempFile(tmpDir, null, ".zip");
432432
URLConnection urlConnection = url.openConnection();
433433
urlConnection.addRequestProperty("User-Agent", "opensearch-plugin-installer");
@@ -684,7 +684,7 @@ InputStream getPublicKey() {
684684
*/
685685
// pkg private for tests
686686
URL openUrl(String urlString) throws IOException {
687-
URL checksumUrl = new URL(urlString);
687+
URL checksumUrl = URI.create(urlString).toURL();
688688
HttpURLConnection connection = (HttpURLConnection) checksumUrl.openConnection();
689689
if (connection.getResponseCode() == 404) {
690690
return null;

distribution/tools/plugin-cli/src/test/java/org/opensearch/tools/cli/plugin/InstallPluginCommandTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,7 @@ public void testSpaceInUrl() throws Exception {
526526
Path pluginDir = createPluginDir(temp);
527527
String pluginZip = createPluginUrl("fake", pluginDir);
528528
Path pluginZipWithSpaces = createTempFile("foo bar", ".zip");
529-
try (InputStream in = FileSystemUtils.openFileURLStream(new URL(pluginZip))) {
529+
try (InputStream in = FileSystemUtils.openFileURLStream(URI.create(pluginZip).toURL())) {
530530
Files.copy(in, pluginZipWithSpaces, StandardCopyOption.REPLACE_EXISTING);
531531
}
532532
installPlugin(pluginZipWithSpaces.toUri().toURL().toString(), env.v1());
@@ -536,8 +536,8 @@ public void testSpaceInUrl() throws Exception {
536536
public void testMalformedUrlNotMaven() throws Exception {
537537
Tuple<Path, Environment> env = createEnv(fs, temp);
538538
// has two colons, so it appears similar to maven coordinates
539-
MalformedURLException e = expectThrows(MalformedURLException.class, () -> installPlugin("://host:1234", env.v1()));
540-
assertTrue(e.getMessage(), e.getMessage().contains("no protocol"));
539+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> installPlugin("://host:1234", env.v1()));
540+
assertThat(e.getMessage(), startsWith("Expected scheme name"));
541541
}
542542

543543
public void testFileNotMaven() throws Exception {

libs/core/src/test/java/org/opensearch/core/util/FileSystemUtilsTests.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040

4141
import java.io.IOException;
4242
import java.io.InputStream;
43+
import java.net.URI;
4344
import java.net.URISyntaxException;
4445
import java.net.URL;
4546
import java.nio.ByteBuffer;
@@ -132,21 +133,21 @@ public void testIsHidden() {
132133
}
133134

134135
public void testOpenFileURLStream() throws IOException {
135-
URL urlWithWrongProtocol = new URL("http://www.google.com");
136+
URL urlWithWrongProtocol = URI.create("http://www.google.com").toURL();
136137
try (InputStream is = FileSystemUtils.openFileURLStream(urlWithWrongProtocol)) {
137138
fail("Should throw IllegalArgumentException due to invalid protocol");
138139
} catch (IllegalArgumentException e) {
139140
assertEquals("Invalid protocol [http], must be [file] or [jar]", e.getMessage());
140141
}
141142

142-
URL urlWithHost = new URL("file", "localhost", txtFile.toString());
143+
URL urlWithHost = URI.create("file://localhost/" + txtFile.toString()).toURL();
143144
try (InputStream is = FileSystemUtils.openFileURLStream(urlWithHost)) {
144145
fail("Should throw IllegalArgumentException due to host");
145146
} catch (IllegalArgumentException e) {
146147
assertEquals("URL cannot have host. Found: [localhost]", e.getMessage());
147148
}
148149

149-
URL urlWithPort = new URL("file", "", 80, txtFile.toString());
150+
URL urlWithPort = URI.create("file://:80/" + txtFile.toString()).toURL();
150151
try (InputStream is = FileSystemUtils.openFileURLStream(urlWithPort)) {
151152
fail("Should throw IllegalArgumentException due to port");
152153
} catch (IllegalArgumentException e) {

libs/ssl-config/src/test/java/org/opensearch/common/ssl/DefaultJdkTrustConfigTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,12 @@ private void assertStandardIssuers(X509ExtendedTrustManager trustManager) {
7777
private void assertHasTrustedIssuer(X509ExtendedTrustManager trustManager, String name) {
7878
final String lowerName = name.toLowerCase(Locale.ROOT);
7979
final Optional<X509Certificate> ca = Stream.of(trustManager.getAcceptedIssuers())
80-
.filter(cert -> cert.getSubjectDN().getName().toLowerCase(Locale.ROOT).contains(lowerName))
80+
.filter(cert -> cert.getSubjectX500Principal().getName().toLowerCase(Locale.ROOT).contains(lowerName))
8181
.findAny();
8282
if (ca.isPresent() == false) {
8383
logger.info("Failed to find issuer [{}] in trust manager, but did find ...", lowerName);
8484
for (X509Certificate cert : trustManager.getAcceptedIssuers()) {
85-
logger.info(" - {}", cert.getSubjectDN().getName().replaceFirst("^\\w+=([^,]+),.*", "$1"));
85+
logger.info(" - {}", cert.getSubjectX500Principal().getName().replaceFirst("^\\w+=([^,]+),.*", "$1"));
8686
}
8787
Assert.fail("Cannot find trusted issuer with name [" + name + "].");
8888
}

libs/ssl-config/src/test/java/org/opensearch/common/ssl/PemKeyConfigTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,8 @@ private void assertCertificateAndKey(PemKeyConfig keyConfig, String expectedDN)
154154
assertThat(chain, notNullValue());
155155
assertThat(chain, arrayWithSize(1));
156156
final X509Certificate certificate = chain[0];
157-
assertThat(certificate.getIssuerDN().getName(), is("CN=Test CA 1"));
158-
assertThat(certificate.getSubjectDN().getName(), is(expectedDN));
157+
assertThat(certificate.getIssuerX500Principal().getName(), is("CN=Test CA 1"));
158+
assertThat(certificate.getSubjectX500Principal().getName(), is(expectedDN));
159159
assertThat(certificate.getSubjectAlternativeNames(), iterableWithSize(2));
160160
assertThat(
161161
certificate.getSubjectAlternativeNames(),

libs/ssl-config/src/test/java/org/opensearch/common/ssl/PemTrustConfigTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ private void assertCertificateChain(PemTrustConfig trustConfig, String... caName
146146
final X509ExtendedTrustManager trustManager = trustConfig.createTrustManager();
147147
final X509Certificate[] issuers = trustManager.getAcceptedIssuers();
148148
final Set<String> issuerNames = Stream.of(issuers)
149-
.map(X509Certificate::getSubjectDN)
149+
.map(X509Certificate::getSubjectX500Principal)
150150
.map(Principal::getName)
151151
.collect(Collectors.toSet());
152152

libs/ssl-config/src/test/java/org/opensearch/common/ssl/StoreKeyConfigTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,8 @@ private void assertKeysLoaded(StoreKeyConfig keyConfig, String... names) throws
183183
assertThat(chain, notNullValue());
184184
assertThat(chain, arrayWithSize(1));
185185
final X509Certificate certificate = chain[0];
186-
assertThat(certificate.getIssuerDN().getName(), is("CN=Test CA 1"));
187-
assertThat(certificate.getSubjectDN().getName(), is("CN=" + name));
186+
assertThat(certificate.getIssuerX500Principal().getName(), is("CN=Test CA 1"));
187+
assertThat(certificate.getSubjectX500Principal().getName(), is("CN=" + name));
188188
assertThat(certificate.getSubjectAlternativeNames(), iterableWithSize(2));
189189
assertThat(
190190
certificate.getSubjectAlternativeNames(),

0 commit comments

Comments
 (0)