Add guard against null-valued settings

This commit adds a guard against null-valued settings that are loaded
from yaml or json settings files, and also adds a test that ensures
the same remains true for settings loaded from properties files.
This commit is contained in:
Jason Tedor 2016-03-23 22:02:26 -04:00
parent 0f00c14afc
commit 4aa5426361
8 changed files with 50 additions and 7 deletions

View File

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

View File

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

View File

@ -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<String, String> 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);
}
}

View File

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

View File

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

View File

@ -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]"));
}
}

View File

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

View File

@ -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]"));
}
}