From 6075e159e5e47e7ed38073e417feb8a818dde259 Mon Sep 17 00:00:00 2001
From: Armin Braun <me@obrown.io>
Date: Tue, 11 Sep 2018 01:24:18 +0200
Subject: [PATCH] Validate list values for settings (#33503)

When we see a settings value, it could be a list. Yet this should only
happen if the underlying setting type is a list setting type. This
commit adds validation that when we get a setting value that is a list,
that the setting that we are getting is a list setting. And similarly,
if we get a value for a list setting, the underlying value should be a
list.
---
 .../common/settings/Setting.java              |  8 +++++--
 .../common/settings/Settings.java             | 24 +++++++++++++++++++
 .../common/settings/SettingTests.java         |  7 ++++++
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java
index 89bbe752a1f..5244cdd726d 100644
--- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java
+++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java
@@ -345,6 +345,11 @@ public class Setting<T> implements ToXContentObject {
         return false;
     }
 
+
+    final boolean isListSetting() {
+        return this instanceof ListSetting;
+    }
+
     boolean hasComplexMatcher() {
         return isGroupSetting();
     }
@@ -453,7 +458,7 @@ public class Setting<T> implements ToXContentObject {
      * @return the raw string representation of the setting value
      */
     String innerGetRaw(final Settings settings) {
-        return settings.get(getKey(), defaultValue.apply(settings));
+        return settings.get(getKey(), defaultValue.apply(settings), isListSetting());
     }
 
     /** Logs a deprecation warning if the setting is deprecated and used. */
@@ -1305,7 +1310,6 @@ public class Setting<T> implements ToXContentObject {
                 }
             }
         }
-
     }
 
     static void logSettingUpdate(Setting setting, Settings current, Settings previous, Logger logger) {
diff --git a/server/src/main/java/org/elasticsearch/common/settings/Settings.java b/server/src/main/java/org/elasticsearch/common/settings/Settings.java
index 2eb14f7ac65..1aeed2aee51 100644
--- a/server/src/main/java/org/elasticsearch/common/settings/Settings.java
+++ b/server/src/main/java/org/elasticsearch/common/settings/Settings.java
@@ -245,6 +245,30 @@ public final class Settings implements ToXContentFragment {
         return retVal == null ? defaultValue : retVal;
     }
 
+    /**
+     * Returns the setting value associated with the setting key. If it does not exists,
+     * returns the default value provided.
+     */
+    String get(String setting, String defaultValue, boolean isList) {
+        Object value = settings.get(setting);
+        if (value != null) {
+            if (value instanceof List) {
+                if (isList == false) {
+                    throw new IllegalArgumentException(
+                        "Found list type value for setting [" + setting + "] but but did not expect a list for it."
+                    );
+                }
+            } else if (isList) {
+                throw new IllegalArgumentException(
+                    "Expected list type value for setting [" + setting + "] but found [" + value.getClass() + ']'
+                );
+            }
+            return toString(value);
+        } else {
+            return defaultValue;
+        }
+    }
+
     /**
      * Returns the setting value (as float) associated with the setting key. If it does not exists,
      * returns the default value provided.
diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java
index b13988b7050..30cfee81ddd 100644
--- a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java
+++ b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java
@@ -180,6 +180,13 @@ public class SettingTests extends ESTestCase {
         }
     }
 
+    public void testValidateStringSetting() {
+        Settings settings = Settings.builder().putList("foo.bar", Arrays.asList("bla-a", "bla-b")).build();
+        Setting<String> stringSetting = Setting.simpleString("foo.bar", Property.NodeScope);
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> stringSetting.get(settings));
+        assertEquals("Found list type value for setting [foo.bar] but but did not expect a list for it.", e.getMessage());
+    }
+
     private static final Setting<String> FOO_BAR_SETTING = new Setting<>(
             "foo.bar",
             "foobar",