Merge pull request #16120 from s1monw/validate_settings_key

Validate the settings key if it's simple chars separated by `.`

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<String, Setting<?>> complexMatchers = new HashMap<>();
     private final Map<String, Setting<?>> 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<Setting<?>> 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 <code>true</code> iff the given key is a valid settings key otherwise <code>false</code>
+     */
+    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;
     }

core/src/main/java/org/elasticsearch/common/settings/SettingsModule.java

@@ -61,6 +61,8 @@ public class SettingsModule extends AbstractModule {
         for (Map.Entry<String, String> 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() + "]");
             }
         }
 

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);

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)));
+    }
+
 }

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();