Add Boolean#parseBoolean to forbidden-apis #129684

kvanerum · 2025-06-19T05:29:19Z

Resolves #59190

Added Boolean#parseBoolean to the list of forbidden-apis.
Resolved the violations against this in tests.
Marked other usages to review in #128993.

cla-checker-service · 2025-06-19T05:29:24Z

💚 CLA has been signed

mosche

Thanks so much for your contribution @kvanerum, this is very much appreciated! I have a couple of small comments.
Could I also ask you to sign the Contributor Agreement, pls?

...llers-finder/src/main/java/org/elasticsearch/entitlement/tools/publiccallersfinder/Main.java

...ility/src/javaRestTest/java/org/elasticsearch/lucene/AbstractIndexCompatibilityTestCase.java

server/src/main/java/org/elasticsearch/gateway/PersistedClusterStateService.java

server/src/main/java/org/elasticsearch/index/IndexService.java

server/src/main/java/org/elasticsearch/index/IndexSettings.java

modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/NettyAllocator.java

server/src/main/java/org/elasticsearch/action/admin/indices/resolve/ResolveIndexAction.java

server/src/main/java/org/elasticsearch/cluster/metadata/LifecycleExecutionState.java

server/src/main/java/org/elasticsearch/common/network/NetworkUtils.java

server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesFormat.java

elasticsearchmachine · 2025-06-24T08:38:23Z

Pinging @elastic/es-core-infra (Team:Core/Infra)

kvanerum · 2025-06-26T18:19:38Z

@mosche Thanks for your comments — I’ve reviewed them and added Boolean.valueOf(String s) to the forbidden APIs list.

mosche · 2025-06-27T08:35:07Z

@elasticmachine test this please

mosche

@kvanerum thanks for addressing my comments. This is looking really good, though I have just a few more ... I also triggered tests, let's see where we're at.

mosche · 2025-06-27T07:51:06Z

modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java

@@ -96,6 +97,9 @@ public Set<ScriptContext<?>> getSupportedContexts() {
        return Set.of(TemplateScript.CONTEXT, TemplateScript.INGEST_CONTEXT);
    }

+    @SuppressForbidden(
+        reason = "TODO Deprecate any lenient usage of Boolean#parseBoolean https://github.com/elastic/elasticsearch/issues/128993"
+    )


please also extract a small helper here to minimize the scope of @SuppressForbidden

mosche · 2025-06-27T07:57:12Z

server/src/main/java/org/elasticsearch/index/IndexService.java

@@ -246,7 +247,7 @@ public IndexService(
                mapperMetrics
            );
            this.indexFieldData = new IndexFieldDataService(indexSettings, indicesFieldDataCache, circuitBreakerService);
-            boolean sourceOnly = Boolean.parseBoolean(indexSettings.getSettings().get("index.source_only"));
+            boolean sourceOnly = isSourceOnly(indexSettings);


I had a closer look at this, this can actually be

Suggested change

boolean sourceOnly = isSourceOnly(indexSettings);

boolean sourceOnly = indexSettings.getAsBoolean("index.source_only", false);

mosche · 2025-06-27T07:58:45Z

server/src/main/java/org/elasticsearch/index/IndexService.java

+    @SuppressForbidden(
+        reason = "TODO Deprecate any lenient usage of Boolean#parseBoolean https://github.com/elastic/elasticsearch/issues/128993"
+    )
+    static boolean isSourceOnly(IndexSettings indexSettings) {
+        return Boolean.parseBoolean(indexSettings.getSettings().get("index.source_only"));
+    }
+


Suggested change

@SuppressForbidden(

reason = "TODO Deprecate any lenient usage of Boolean#parseBoolean https://github.com/elastic/elasticsearch/issues/128993"

)

static boolean isSourceOnly(IndexSettings indexSettings) {

return Boolean.parseBoolean(indexSettings.getSettings().get("index.source_only"));

}

mosche · 2025-06-27T08:00:12Z

modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java

@@ -391,6 +392,9 @@ ScriptScope compile(Compiler compiler, Loader loader, String scriptName, String
        }
    }

+    @SuppressForbidden(
+        reason = "TODO Deprecate any lenient usage of Boolean#parseBoolean https://github.com/elastic/elasticsearch/issues/128993"
+    )


please also extract a small helper here to minimize the scope of @SuppressForbidden

mosche · 2025-06-27T08:05:11Z

server/src/main/java/org/elasticsearch/action/admin/indices/resolve/ResolveIndexAction.java

@@ -629,7 +630,7 @@ private static void enrichIndexAbstraction(
                        if (ia.isSystem()) {
                            attributes.add(Attribute.SYSTEM);
                        }
-                        final boolean isFrozen = Boolean.parseBoolean(writeIndex.getSettings().get("index.frozen"));
+                        final boolean isFrozen = isFrozen(writeIndex);


I had a closer look at this, this can actually be

Suggested change

final boolean isFrozen = isFrozen(writeIndex);

final boolean isFrozen = writeIndex.getSettings().getAsBoolean("index.frozen", false);

mosche · 2025-06-27T08:06:18Z

server/src/main/java/org/elasticsearch/common/network/NetworkUtils.java

+    @SuppressForbidden(
+        reason = "TODO Deprecate any lenient usage of Boolean#parseBoolean https://github.com/elastic/elasticsearch/issues/128993"
+    )
+    static boolean preferIPv6Addresses() {


Suggested change

static boolean preferIPv6Addresses() {

private static boolean preferIPv6Addresses() {

mosche · 2025-06-27T08:07:43Z

server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesFormat.java

+    @SuppressForbidden(
+        reason = "TODO Deprecate any lenient usage of Boolean#parseBoolean https://github.com/elastic/elasticsearch/issues/128993"
+    )
+    static boolean getOptimizedMergeEnabledDefault() {


Suggested change

static boolean getOptimizedMergeEnabledDefault() {

private static boolean getOptimizedMergeEnabledDefault() {

mosche · 2025-06-27T08:15:45Z

server/src/main/java/org/elasticsearch/search/SearchService.java

+    @SuppressForbidden(
+        reason = "TODO Deprecate any lenient usage of Boolean#parseBoolean https://github.com/elastic/elasticsearch/issues/128993"
+    )
+    static boolean getErrorTraceHeader(ThreadPool threadPool) {


Suggested change

static boolean getErrorTraceHeader(ThreadPool threadPool) {

private static boolean getErrorTraceHeader(ThreadPool threadPool) {

mosche · 2025-06-27T08:17:14Z

...rc/main/java/org/elasticsearch/xpack/core/ml/inference/trainedmodel/PredictionFieldType.java

@@ -45,6 +46,7 @@ public String toString() {
        return name().toLowerCase(Locale.ROOT);
    }

+    @SuppressForbidden(reason = "accept lenient boolean field values")


please also extract a small helper here to minimize the scope of @SuppressForbidden

mosche · 2025-06-27T08:25:17Z

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/TypeConverter.java

@@ -310,6 +311,9 @@ private static <T> T failConversion(Object value, EsType columnType, String type
        throw e != null ? new SQLException(message, e) : new SQLException(message);
    }

+    @SuppressForbidden(
+        reason = "TODO Deprecate any lenient usage of Boolean#parseBoolean https://github.com/elastic/elasticsearch/issues/128993"


Suggested change

reason = "TODO Deprecate any lenient usage of Boolean#parseBoolean https://github.com/elastic/elasticsearch/issues/128993"

reason = "allow lenient conversion to boolean"

mosche · 2025-06-27T09:55:33Z

@elasticmachine test this please

...main/java/org/elasticsearch/index/codec/vectors/es818/ES818BinaryQuantizedVectorsFormat.java

mosche · 2025-06-27T10:20:02Z

@elasticmachine test this please

mosche · 2025-06-27T12:53:26Z

...est/java/org/elasticsearch/xpack/migrate/action/ReindexDatastreamIndexTransportActionIT.java

+        assertFalse(Booleans.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_READ_ONLY)));
+        assertFalse(Booleans.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_READ_ONLY_ALLOW_DELETE)));
+        assertFalse(Booleans.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_BLOCKS_WRITE)));


looking at test failures, these need to default to false

Suggested change

assertFalse(Booleans.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_READ_ONLY)));

assertFalse(Booleans.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_READ_ONLY_ALLOW_DELETE)));

assertFalse(Booleans.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_BLOCKS_WRITE)));

assertFalse(Booleans.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_READ_ONLY), false));

assertFalse(Booleans.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_READ_ONLY_ALLOW_DELETE), false));

assertFalse(Booleans.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_BLOCKS_WRITE), false));

Add Boolean#parseBoolean to forbidden-apis

3147645

kvanerum requested review from a team as code owners June 19, 2025 05:29

elasticsearchmachine added v9.1.0 needs:triage Requires assignment of a team area label external-contributor Pull request authored by a developer outside the Elasticsearch team labels Jun 19, 2025

Merge branch 'main' into add-parseboolean-to-forbidden-apis

78a79f3

breskeby requested a review from mosche June 19, 2025 09:28

mosche requested changes Jun 24, 2025

View reviewed changes

mosche self-assigned this Jun 24, 2025

mosche added >non-issue :Core/Infra/Core Core issues without another label and removed needs:triage Requires assignment of a team area label labels Jun 24, 2025

elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Jun 24, 2025

Merge branch 'elastic:main' into add-parseboolean-to-forbidden-apis

288c0b5

elasticsearchmachine added v9.2.0 and removed v9.1.0 labels Jun 26, 2025

kvanerum added 7 commits June 26, 2025 18:36

Remove suppression and replace with Booleans.parseBoolean

5196d60

Remove suppression and replace with Booleans.parseBoolean

945ae4a

Use Booleans.parseBoolean(...., false) in case setting is absent

61ac240

Update reason for suppressions

26ba14b

Suppress usage of Boolean.parseBoolean

db42edf

Use Booleans.parseBoolean(...., false) in case setting is absent

7e5cacb

Add Boolean.valueOf to forbidden-apis

498de1d

Merge branch 'main' into add-parseboolean-to-forbidden-apis

17bf3bb

mosche reviewed Jun 27, 2025

View reviewed changes

[CI] Auto commit changes from spotless

48d29ac

mosche reviewed Jun 27, 2025

View reviewed changes

...main/java/org/elasticsearch/index/codec/vectors/es818/ES818BinaryQuantizedVectorsFormat.java Outdated Show resolved Hide resolved

fix previous merge

457f5cc

mosche reviewed Jun 27, 2025

View reviewed changes

Resolve comments

61bc66b

	boolean sourceOnly = isSourceOnly(indexSettings);
	boolean sourceOnly = indexSettings.getAsBoolean("index.source_only", false);

	final boolean isFrozen = isFrozen(writeIndex);
	final boolean isFrozen = writeIndex.getSettings().getAsBoolean("index.frozen", false);

	static boolean preferIPv6Addresses() {
	private static boolean preferIPv6Addresses() {

	static boolean getOptimizedMergeEnabledDefault() {
	private static boolean getOptimizedMergeEnabledDefault() {

	static boolean getErrorTraceHeader(ThreadPool threadPool) {
	private static boolean getErrorTraceHeader(ThreadPool threadPool) {

	reason = "TODO Deprecate any lenient usage of Boolean#parseBoolean https://github.com/elastic/elasticsearch/issues/128993"
	reason = "allow lenient conversion to boolean"

Add Boolean#parseBoolean to forbidden-apis #129684

Are you sure you want to change the base?

Add Boolean#parseBoolean to forbidden-apis #129684

Uh oh!

Conversation

kvanerum commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cla-checker-service bot commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mosche left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elasticsearchmachine commented Jun 24, 2025

Uh oh!

kvanerum commented Jun 26, 2025

Uh oh!

mosche commented Jun 27, 2025

Uh oh!

mosche left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mosche commented Jun 27, 2025

Uh oh!

Uh oh!

mosche commented Jun 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kvanerum commented Jun 19, 2025 •

edited

Loading

cla-checker-service bot commented Jun 19, 2025 •

edited

Loading