From e3302da589f30bd54cc618f3acd7578a2416f60d Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Thu, 15 Dec 2016 19:06:59 +0100 Subject: [PATCH 1/3] Make boolean conversion strict This PR removes all leniency in the conversion of Strings to booleans: "true" is converted to the boolean value `true`, "false" is converted to the boolean value `false`. Everything else raises an error. Original commit: elastic/x-pack-elasticsearch@6400f18911a7e322323edb80f9923b839a334182 --- elasticsearch/src/main/resources/security-index-template.json | 2 +- ...AbstractOldXPackIndicesBackwardsCompatibilityTestCase.java | 2 ++ .../test/java/org/elasticsearch/test/SettingsFilterTests.java | 2 +- .../security/authc/esnative/ESNativeMigrateToolTests.java | 4 ++++ .../resources/missing-version-security-index-template.json | 2 +- .../test/resources/wrong-version-security-index-template.json | 2 +- 6 files changed, 10 insertions(+), 4 deletions(-) diff --git a/elasticsearch/src/main/resources/security-index-template.json b/elasticsearch/src/main/resources/security-index-template.json index 35dcd50de3f..c462a5ee2e4 100644 --- a/elasticsearch/src/main/resources/security-index-template.json +++ b/elasticsearch/src/main/resources/security-index-template.json @@ -10,7 +10,7 @@ "filter" : { "email" : { "type" : "pattern_capture", - "preserve_original" : 1, + "preserve_original" : true, "patterns" : [ "([^@]+)", "(\\p{L}+)", diff --git a/elasticsearch/src/test/java/org/elasticsearch/AbstractOldXPackIndicesBackwardsCompatibilityTestCase.java b/elasticsearch/src/test/java/org/elasticsearch/AbstractOldXPackIndicesBackwardsCompatibilityTestCase.java index 33405a4c1ea..4bc13fd5aa7 100644 --- a/elasticsearch/src/test/java/org/elasticsearch/AbstractOldXPackIndicesBackwardsCompatibilityTestCase.java +++ b/elasticsearch/src/test/java/org/elasticsearch/AbstractOldXPackIndicesBackwardsCompatibilityTestCase.java @@ -116,6 +116,8 @@ public abstract class AbstractOldXPackIndicesBackwardsCompatibilityTestCase exte } } + //TODO dm: fix me + @AwaitsFix(bugUrl = "We need to fix BWC first") public void testOldIndexes() throws Exception { Collections.shuffle(dataFiles, random()); for (String dataFile : dataFiles) { diff --git a/elasticsearch/src/test/java/org/elasticsearch/test/SettingsFilterTests.java b/elasticsearch/src/test/java/org/elasticsearch/test/SettingsFilterTests.java index 54d0d439bc6..41b8baa7281 100644 --- a/elasticsearch/src/test/java/org/elasticsearch/test/SettingsFilterTests.java +++ b/elasticsearch/src/test/java/org/elasticsearch/test/SettingsFilterTests.java @@ -116,7 +116,7 @@ public class SettingsFilterTests extends ESTestCase { } private String randomBooleanSetting() { - return randomFrom("true", "1", "on", "yes", "false", "0", "off", "no"); + return randomFrom("true", "false"); } private void configureUnfilteredSetting(String settingName, String value) { diff --git a/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeMigrateToolTests.java b/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeMigrateToolTests.java index a06db65a590..5344b4d7b39 100644 --- a/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeMigrateToolTests.java +++ b/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeMigrateToolTests.java @@ -58,6 +58,8 @@ public class ESNativeMigrateToolTests extends NativeRealmIntegTestCase { return internalCluster().getInstances(Environment.class).iterator().next(); } + //TODO dm: fix me + @AwaitsFix(bugUrl = "We need to fix BWC first") public void testRetrieveUsers() throws Exception { final Environment nodeEnvironment = nodeEnvironment(); String home = Environment.PATH_HOME_SETTING.get(nodeEnvironment.settings()); @@ -98,6 +100,8 @@ public class ESNativeMigrateToolTests extends NativeRealmIntegTestCase { } } + //TODO dm: fix me + @AwaitsFix(bugUrl = "We need to fix BWC first") public void testRetrieveRoles() throws Exception { final Environment nodeEnvironment = nodeEnvironment(); String home = Environment.PATH_HOME_SETTING.get(nodeEnvironment.settings()); diff --git a/elasticsearch/src/test/resources/missing-version-security-index-template.json b/elasticsearch/src/test/resources/missing-version-security-index-template.json index 332b9e3c75a..c4c74f190dd 100644 --- a/elasticsearch/src/test/resources/missing-version-security-index-template.json +++ b/elasticsearch/src/test/resources/missing-version-security-index-template.json @@ -9,7 +9,7 @@ "filter" : { "email" : { "type" : "pattern_capture", - "preserve_original" : 1, + "preserve_original" : true, "patterns" : [ "([^@]+)", "(\\p{L}+)", diff --git a/elasticsearch/src/test/resources/wrong-version-security-index-template.json b/elasticsearch/src/test/resources/wrong-version-security-index-template.json index 46375d4aac2..18da429a08a 100644 --- a/elasticsearch/src/test/resources/wrong-version-security-index-template.json +++ b/elasticsearch/src/test/resources/wrong-version-security-index-template.json @@ -9,7 +9,7 @@ "filter" : { "email" : { "type" : "pattern_capture", - "preserve_original" : 1, + "preserve_original" : true, "patterns" : [ "([^@]+)", "(\\p{L}+)", From 2cb41e690d591d5ef62f2935d7184bd47dd24ecc Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Mon, 19 Dec 2016 13:27:40 +0100 Subject: [PATCH 2/3] Address initial review comments Original commit: elastic/x-pack-elasticsearch@bcd6433afbe3d3e79319ca8d321711a011a5e343 --- .../elasticsearch/xpack/notification/jira/JiraAccount.java | 2 +- .../main/java/org/elasticsearch/xpack/security/Security.java | 4 ++-- .../main/java/org/elasticsearch/xpack/watcher/Watcher.java | 5 ++--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/notification/jira/JiraAccount.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/notification/jira/JiraAccount.java index a8af6c503b6..ff29ba7ff92 100644 --- a/elasticsearch/src/main/java/org/elasticsearch/xpack/notification/jira/JiraAccount.java +++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/notification/jira/JiraAccount.java @@ -53,7 +53,7 @@ public class JiraAccount { try { URI uri = new URI(url); Scheme protocol = Scheme.parse(uri.getScheme()); - if ((protocol == Scheme.HTTP) && (Booleans.isExplicitTrue(settings.get(ALLOW_HTTP_SETTING)) == false)) { + if ((protocol == Scheme.HTTP) && (Booleans.isTrue(settings.get(ALLOW_HTTP_SETTING)) == false)) { throw new SettingsException("invalid jira [" + name + "] account settings. unsecure scheme [" + protocol + "]"); } this.url = uri; diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/Security.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/Security.java index 15c0fb8065a..e495501cf46 100644 --- a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -621,11 +621,11 @@ public class Security implements ActionPlugin, IngestPlugin, NetworkPlugin { String errorMessage = LoggerMessageFormat.format("the [action.auto_create_index] setting value [{}] is too" + " restrictive. disable [action.auto_create_index] or set it to " + "[{}{}]", (Object) value, SecurityTemplateService.SECURITY_INDEX_NAME, auditIndex); - if (Booleans.isExplicitFalse(value)) { + if (Booleans.isFalse(value)) { throw new IllegalArgumentException(errorMessage); } - if (Booleans.isExplicitTrue(value)) { + if (Booleans.isTrue(value)) { return; } diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java index 82f4722daa5..cfb355fc882 100644 --- a/elasticsearch/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java +++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java @@ -28,7 +28,6 @@ import org.elasticsearch.plugins.ScriptPlugin; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptService; -import org.elasticsearch.script.ScriptSettings; import org.elasticsearch.search.SearchRequestParsers; import org.elasticsearch.threadpool.ExecutorBuilder; import org.elasticsearch.threadpool.FixedExecutorBuilder; @@ -437,11 +436,11 @@ public class Watcher implements ActionPlugin, ScriptPlugin { String errorMessage = LoggerMessageFormat.format("the [action.auto_create_index] setting value [{}] is too" + " restrictive. disable [action.auto_create_index] or set it to " + "[{}, {}, {}*]", (Object) value, Watch.INDEX, TriggeredWatchStore.INDEX_NAME, HistoryStore.INDEX_PREFIX); - if (Booleans.isExplicitFalse(value)) { + if (Booleans.isFalse(value)) { throw new IllegalArgumentException(errorMessage); } - if (Booleans.isExplicitTrue(value)) { + if (Booleans.isTrue(value)) { return; } From b973d1068542177769a7f0647d1ecaef88a191fc Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Wed, 21 Dec 2016 16:43:55 +0100 Subject: [PATCH 3/3] Improve BWC Original commit: elastic/x-pack-elasticsearch@f84cb5b1fabfc8405966d9c80823d517a811cd7f --- .../support/xcontent/WatcherXContentParser.java | 12 ++++++++++++ ...ldXPackIndicesBackwardsCompatibilityTestCase.java | 2 -- .../authc/esnative/ESNativeMigrateToolTests.java | 4 ---- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/watcher/support/xcontent/WatcherXContentParser.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/watcher/support/xcontent/WatcherXContentParser.java index 00a20c262e8..b8c7cae702b 100644 --- a/elasticsearch/src/main/java/org/elasticsearch/xpack/watcher/support/xcontent/WatcherXContentParser.java +++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/watcher/support/xcontent/WatcherXContentParser.java @@ -250,6 +250,18 @@ public class WatcherXContentParser implements XContentParser { return parser.booleanValue(); } + @Override + @SuppressWarnings("deprecated") + public boolean isBooleanValueLenient() throws IOException { + return parser.isBooleanValueLenient(); + } + + @Override + @SuppressWarnings("deprecated") + public boolean booleanValueLenient() throws IOException { + return parser.booleanValueLenient(); + } + @Override public byte[] binaryValue() throws IOException { return parser.binaryValue(); diff --git a/elasticsearch/src/test/java/org/elasticsearch/AbstractOldXPackIndicesBackwardsCompatibilityTestCase.java b/elasticsearch/src/test/java/org/elasticsearch/AbstractOldXPackIndicesBackwardsCompatibilityTestCase.java index 4bc13fd5aa7..33405a4c1ea 100644 --- a/elasticsearch/src/test/java/org/elasticsearch/AbstractOldXPackIndicesBackwardsCompatibilityTestCase.java +++ b/elasticsearch/src/test/java/org/elasticsearch/AbstractOldXPackIndicesBackwardsCompatibilityTestCase.java @@ -116,8 +116,6 @@ public abstract class AbstractOldXPackIndicesBackwardsCompatibilityTestCase exte } } - //TODO dm: fix me - @AwaitsFix(bugUrl = "We need to fix BWC first") public void testOldIndexes() throws Exception { Collections.shuffle(dataFiles, random()); for (String dataFile : dataFiles) { diff --git a/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeMigrateToolTests.java b/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeMigrateToolTests.java index 5344b4d7b39..a06db65a590 100644 --- a/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeMigrateToolTests.java +++ b/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeMigrateToolTests.java @@ -58,8 +58,6 @@ public class ESNativeMigrateToolTests extends NativeRealmIntegTestCase { return internalCluster().getInstances(Environment.class).iterator().next(); } - //TODO dm: fix me - @AwaitsFix(bugUrl = "We need to fix BWC first") public void testRetrieveUsers() throws Exception { final Environment nodeEnvironment = nodeEnvironment(); String home = Environment.PATH_HOME_SETTING.get(nodeEnvironment.settings()); @@ -100,8 +98,6 @@ public class ESNativeMigrateToolTests extends NativeRealmIntegTestCase { } } - //TODO dm: fix me - @AwaitsFix(bugUrl = "We need to fix BWC first") public void testRetrieveRoles() throws Exception { final Environment nodeEnvironment = nodeEnvironment(); String home = Environment.PATH_HOME_SETTING.get(nodeEnvironment.settings());