diff --git a/core/src/main/java/org/elasticsearch/common/settings/loader/JsonSettingsLoader.java b/core/src/main/java/org/elasticsearch/common/settings/loader/JsonSettingsLoader.java index f6f77192c75..29a20e6a7ab 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/loader/JsonSettingsLoader.java +++ b/core/src/main/java/org/elasticsearch/common/settings/loader/JsonSettingsLoader.java @@ -27,6 +27,10 @@ import org.elasticsearch.common.xcontent.XContentType; */ public class JsonSettingsLoader extends XContentSettingsLoader { + public JsonSettingsLoader(boolean guardAgainstNullValuedSettings) { + super(guardAgainstNullValuedSettings); + } + @Override public XContentType contentType() { return XContentType.JSON; diff --git a/core/src/main/java/org/elasticsearch/common/settings/loader/SettingsLoaderFactory.java b/core/src/main/java/org/elasticsearch/common/settings/loader/SettingsLoaderFactory.java index e55cb1092f2..dc4240b239e 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/loader/SettingsLoaderFactory.java +++ b/core/src/main/java/org/elasticsearch/common/settings/loader/SettingsLoaderFactory.java @@ -36,14 +36,14 @@ public final class SettingsLoaderFactory { */ public static SettingsLoader loaderFromResource(String resourceName) { if (resourceName.endsWith(".json")) { - return new JsonSettingsLoader(); + return new JsonSettingsLoader(true); } else if (resourceName.endsWith(".yml") || resourceName.endsWith(".yaml")) { - return new YamlSettingsLoader(); + return new YamlSettingsLoader(true); } else if (resourceName.endsWith(".properties")) { return new PropertiesSettingsLoader(); } else { // lets default to the json one - return new JsonSettingsLoader(); + return new JsonSettingsLoader(true); } } @@ -52,10 +52,10 @@ public final class SettingsLoaderFactory { */ public static SettingsLoader loaderFromSource(String source) { if (source.indexOf('{') != -1 && source.indexOf('}') != -1) { - return new JsonSettingsLoader(); + return new JsonSettingsLoader(false); } if (source.indexOf(':') != -1) { - return new YamlSettingsLoader(); + return new YamlSettingsLoader(false); } return new PropertiesSettingsLoader(); } diff --git a/core/src/main/java/org/elasticsearch/common/settings/loader/XContentSettingsLoader.java b/core/src/main/java/org/elasticsearch/common/settings/loader/XContentSettingsLoader.java index 9c2f973b96e..463dfb83f3e 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/loader/XContentSettingsLoader.java +++ b/core/src/main/java/org/elasticsearch/common/settings/loader/XContentSettingsLoader.java @@ -38,6 +38,12 @@ public abstract class XContentSettingsLoader implements SettingsLoader { public abstract XContentType contentType(); + private final boolean guardAgainstNullValuedSettings; + + XContentSettingsLoader(boolean guardAgainstNullValuedSettings) { + this.guardAgainstNullValuedSettings = guardAgainstNullValuedSettings; + } + @Override public Map load(String source) throws IOException { try (XContentParser parser = XContentFactory.xContent(contentType()).createParser(source)) { @@ -153,6 +159,16 @@ public abstract class XContentSettingsLoader implements SettingsLoader { currentValue ); } + + if (guardAgainstNullValuedSettings && currentValue == null) { + throw new ElasticsearchParseException( + "null-valued setting found for key [{}] found at line number [{}], column number [{}]", + key, + parser.getTokenLocation().lineNumber, + parser.getTokenLocation().columnNumber + ); + } + settings.put(key, currentValue); } } diff --git a/core/src/main/java/org/elasticsearch/common/settings/loader/YamlSettingsLoader.java b/core/src/main/java/org/elasticsearch/common/settings/loader/YamlSettingsLoader.java index 248fe090b5d..6768b00d8e4 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/loader/YamlSettingsLoader.java +++ b/core/src/main/java/org/elasticsearch/common/settings/loader/YamlSettingsLoader.java @@ -30,6 +30,10 @@ import java.util.Map; */ public class YamlSettingsLoader extends XContentSettingsLoader { + public YamlSettingsLoader(boolean guardAgainstNullValuedSettings) { + super(guardAgainstNullValuedSettings); + } + @Override public XContentType contentType() { return XContentType.YAML; diff --git a/core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java b/core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java index e3f2bc1bb2b..39dc93c3181 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java @@ -201,8 +201,8 @@ public class SettingsTests extends ESTestCase { assertThat(settings.getAsArray("value"), arrayContaining("2", "3")); settings = settingsBuilder() - .put(new YamlSettingsLoader().load("value: 1")) - .put(new YamlSettingsLoader().load("value: [ 2, 3 ]")) + .put(new YamlSettingsLoader(true).load("value: 1")) + .put(new YamlSettingsLoader(true).load("value: [ 2, 3 ]")) .build(); assertThat(settings.getAsArray("value"), arrayContaining("2", "3")); diff --git a/core/src/test/java/org/elasticsearch/common/settings/loader/JsonSettingsLoaderTests.java b/core/src/test/java/org/elasticsearch/common/settings/loader/JsonSettingsLoaderTests.java index d7f10891f28..df5e71a1178 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/loader/JsonSettingsLoaderTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/loader/JsonSettingsLoaderTests.java @@ -25,6 +25,7 @@ import org.elasticsearch.common.settings.SettingsException; import org.elasticsearch.test.ESTestCase; import static org.elasticsearch.common.settings.Settings.settingsBuilder; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.Matchers.equalTo; /** @@ -61,4 +62,10 @@ public class JsonSettingsLoaderTests extends ESTestCase { assertTrue(e.toString().contains("duplicate settings key [foo] found at line number [1], column number [20], previous value [bar], current value [baz]")); } } + + public void testNullValuedSettingThrowsException() { + final String json = "{\"foo\":null}"; + final ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class, () -> new JsonSettingsLoader(true).load(json)); + assertThat(e.toString(), containsString("null-valued setting found for key [foo] found at line number [1], column number [8]")); + } } diff --git a/core/src/test/java/org/elasticsearch/common/settings/loader/PropertiesSettingsLoaderTests.java b/core/src/test/java/org/elasticsearch/common/settings/loader/PropertiesSettingsLoaderTests.java index 7a1897fbaf9..e9a20281b72 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/loader/PropertiesSettingsLoaderTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/loader/PropertiesSettingsLoaderTests.java @@ -44,4 +44,10 @@ public class PropertiesSettingsLoaderTests extends ESTestCase { assertEquals(e.getMessage(), "duplicate settings key [foo] found, previous value [bar], current value [baz]"); } } + + public void testThatNoDuplicatesPropertiesDoesNotAcceptNullValues() { + PropertiesSettingsLoader loader = new PropertiesSettingsLoader(); + PropertiesSettingsLoader.NoDuplicatesProperties properties = loader.new NoDuplicatesProperties(); + expectThrows(NullPointerException.class, () -> properties.put("key", null)); + } } diff --git a/core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java b/core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java index 48703044ecd..077cb5088f7 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java @@ -86,4 +86,10 @@ public class YamlSettingsLoaderTests extends ESTestCase { assertTrue(e.toString().contains("duplicate settings key [foo] found at line number [2], column number [6], previous value [bar], current value [baz]")); } } + + public void testNullValuedSettingThrowsException() { + final String yaml = "foo:"; + final ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class, () -> new YamlSettingsLoader(true).load(yaml)); + assertThat(e.toString(), containsString("null-valued setting found for key [foo] found at line number [1], column number [5]")); + } }