diff --git a/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java b/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java index 73d13b2de20..9bddd73d632 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -33,6 +33,7 @@ import java.util.Map; import java.util.Set; import java.util.function.BiConsumer; import java.util.function.Consumer; +import java.util.regex.Pattern; /** * A basic setting service that can be used for per-index and per-cluster settings. @@ -44,6 +45,9 @@ public abstract class AbstractScopedSettings extends AbstractComponent { private final Map> complexMatchers = new HashMap<>(); private final Map> keySettings = new HashMap<>(); private final Setting.Scope scope; + private static final Pattern KEY_PATTERN = Pattern.compile("^(?:[-\\w]+[.])*[-\\w]+$"); + private static final Pattern GROUP_KEY_PATTERN = Pattern.compile("^(?:[-\\w]+[.])+$"); + protected AbstractScopedSettings(Settings settings, Set> settingsSet, Setting.Scope scope) { super(settings); @@ -67,6 +71,9 @@ public abstract class AbstractScopedSettings extends AbstractComponent { if (setting.getScope() != scope) { throw new IllegalArgumentException("Setting must be a " + scope + " setting but was: " + setting.getScope()); } + if (isValidKey(setting.getKey()) == false && (setting.isGroupSetting() && isValidGroupKey(setting.getKey())) == false) { + throw new IllegalArgumentException("illegal settings key: [" + setting.getKey() + "]"); + } if (setting.hasComplexMatcher()) { complexMatchers.putIfAbsent(setting.getKey(), setting); } else { @@ -74,6 +81,17 @@ public abstract class AbstractScopedSettings extends AbstractComponent { } } + /** + * Returns true iff the given key is a valid settings key otherwise false + */ + public static boolean isValidKey(String key) { + return KEY_PATTERN.matcher(key).matches(); + } + + private static boolean isValidGroupKey(String key) { + return GROUP_KEY_PATTERN.matcher(key).matches(); + } + public Setting.Scope getScope() { return this.scope; } diff --git a/core/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/core/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index 8298c0c127d..eec6e734229 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/core/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -61,6 +61,8 @@ public class SettingsModule extends AbstractModule { for (Map.Entry entry : settings.filter(IndexScopedSettings.INDEX_SETTINGS_KEY_PREDICATE.negate()).getAsMap().entrySet()) { if (clusterSettings.get(entry.getKey()) != null) { clusterSettings.validate(entry.getKey(), settings); + } else if (AbstractScopedSettings.isValidKey(entry.getKey()) == false) { + throw new IllegalArgumentException("illegal settings key: [" + entry.getKey() + "]"); } } diff --git a/core/src/test/java/org/elasticsearch/cluster/allocation/ClusterRerouteIT.java b/core/src/test/java/org/elasticsearch/cluster/allocation/ClusterRerouteIT.java index 6835f343563..793cb0ce421 100644 --- a/core/src/test/java/org/elasticsearch/cluster/allocation/ClusterRerouteIT.java +++ b/core/src/test/java/org/elasticsearch/cluster/allocation/ClusterRerouteIT.java @@ -162,8 +162,8 @@ public class ClusterRerouteIT extends ESIntegTestCase { public void testDelayWithALargeAmountOfShards() throws Exception { Settings commonSettings = settingsBuilder() - .put(ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_INCOMING_RECOVERIES_SETTING, 1) - .put(ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_OUTGOING_RECOVERIES_SETTING, 1) + .put(ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_INCOMING_RECOVERIES_SETTING.getKey(), 1) + .put(ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_OUTGOING_RECOVERIES_SETTING.getKey(), 1) .build(); logger.info("--> starting 4 nodes"); String node_1 = internalCluster().startNode(commonSettings); diff --git a/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java b/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java index 088d4fa5ac6..86a37723b57 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java @@ -225,4 +225,33 @@ public class ScopedSettingsTests extends ESTestCase { return metaData; } + public void testKeyPattern() { + assertTrue(AbstractScopedSettings.isValidKey("a.b.c-b.d")); + assertTrue(AbstractScopedSettings.isValidKey("a.b.c.d")); + assertTrue(AbstractScopedSettings.isValidKey("a.b_012.c_b.d")); + assertTrue(AbstractScopedSettings.isValidKey("a")); + assertFalse(AbstractScopedSettings.isValidKey("a b")); + assertFalse(AbstractScopedSettings.isValidKey("")); + assertFalse(AbstractScopedSettings.isValidKey("\"")); + + try { + new IndexScopedSettings( + Settings.EMPTY, Collections.singleton(Setting.groupSetting("boo .", false, Setting.Scope.INDEX))); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("illegal settings key: [boo .]", e.getMessage()); + } + new IndexScopedSettings( + Settings.EMPTY, Collections.singleton(Setting.groupSetting("boo.", false, Setting.Scope.INDEX))); + try { + new IndexScopedSettings( + Settings.EMPTY, Collections.singleton(Setting.boolSetting("boo.", true, false, Setting.Scope.INDEX))); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("illegal settings key: [boo.]", e.getMessage()); + } + new IndexScopedSettings( + Settings.EMPTY, Collections.singleton(Setting.boolSetting("boo", true, false, Setting.Scope.INDEX))); + } + } diff --git a/core/src/test/java/org/elasticsearch/gateway/RecoveryFromGatewayIT.java b/core/src/test/java/org/elasticsearch/gateway/RecoveryFromGatewayIT.java index d26f0fbf412..e93d8fbcf41 100644 --- a/core/src/test/java/org/elasticsearch/gateway/RecoveryFromGatewayIT.java +++ b/core/src/test/java/org/elasticsearch/gateway/RecoveryFromGatewayIT.java @@ -331,10 +331,10 @@ public class RecoveryFromGatewayIT extends ESIntegTestCase { public void testReusePeerRecovery() throws Exception { final Settings settings = settingsBuilder() .put("action.admin.cluster.node.shutdown.delay", "10ms") - .put(MockFSIndexStore.INDEX_CHECK_INDEX_ON_CLOSE_SETTING, false) + .put(MockFSIndexStore.INDEX_CHECK_INDEX_ON_CLOSE_SETTING.getKey(), false) .put("gateway.recover_after_nodes", 4) - .put(ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_INCOMING_RECOVERIES_SETTING, 4) - .put(ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_OUTGOING_RECOVERIES_SETTING, 4) + .put(ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_INCOMING_RECOVERIES_SETTING.getKey(), 4) + .put(ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_OUTGOING_RECOVERIES_SETTING.getKey(), 4) .put(MockFSDirectoryService.CRASH_INDEX_SETTING.getKey(), false).build(); internalCluster().startNodesAsync(4, settings).get();