From 8ac105dbb931c1a4e091c3bcd82a198da67ff012 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 24 Jul 2015 13:12:44 -0400 Subject: [PATCH] Add explicit check that we have reached the end of the settings stream when parsing settings Settings are currently parsed by looping over the tokens until an END_OBJECT token is reached. However, this does not mean that the end of the settings stream was reached. This can occur, for example, when parsing a YAML settings file with inconsistent indentation. Currently in this case, some settings will be silently ignored. This commit forces a check that we have in fact reached the end of the settings stream. Closes #12382 --- .../loader/XContentSettingsLoader.java | 18 ++++++++++++++++++ .../common/xcontent/XContentParser.java | 2 ++ .../xcontent/json/JsonXContentParser.java | 5 +++++ .../support/AbstractXContentParser.java | 3 +++ .../loader/YamlSettingsLoaderTests.java | 16 +++++++++++++++- .../settings/loader/indentation-settings.yml | 10 ++++++++++ ...n-with-explicit-document-start-settings.yml | 11 +++++++++++ 7 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 core/src/test/java/org/elasticsearch/common/settings/loader/indentation-settings.yml create mode 100644 core/src/test/java/org/elasticsearch/common/settings/loader/indentation-with-explicit-document-start-settings.yml 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 ffbe1669d47..d7dcff75e27 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 @@ -20,6 +20,7 @@ package org.elasticsearch.common.settings.loader; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; @@ -65,6 +66,23 @@ public abstract class XContentSettingsLoader implements SettingsLoader { throw new ElasticsearchParseException("malformed, expected settings to start with 'object', instead was [{}]", token); } serializeObject(settings, sb, path, jp, null); + + // ensure we reached the end of the stream + Exception exception = null; + XContentParser.Token lastToken = null; + try { + while (!jp.isClosed() && (lastToken = jp.nextToken()) == null); + } catch (Exception e) { + exception = e; + } + if (exception != null || lastToken != null) { + throw new ElasticsearchParseException( + "malformed, expected end of settings but encountered additional content starting at columnNumber: [{}], lineNumber: [{}]", + jp.getTokenLocation().columnNumber, + jp.getTokenLocation().lineNumber + ); + } + return settings; } diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java index 3901a45a181..a6b4f460f47 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java @@ -250,4 +250,6 @@ public interface XContentParser extends Releasable { * @return last token's location or null if cannot be determined */ XContentLocation getTokenLocation(); + + boolean isClosed(); } diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java index 5d3a3f99f4e..787c28324de 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java @@ -248,4 +248,9 @@ public class JsonXContentParser extends AbstractXContentParser { } throw new IllegalStateException("No matching token for json_token [" + token + "]"); } + + @Override + public boolean isClosed() { + return parser.isClosed(); + } } diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java index f0b157b486d..039fd2629eb 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java @@ -319,4 +319,7 @@ public abstract class AbstractXContentParser implements XContentParser { } return null; } + + @Override + public abstract boolean isClosed(); } 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 a9c77e9b310..0ad737cb701 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 @@ -20,11 +20,11 @@ package org.elasticsearch.common.settings.loader; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.settings.SettingsException; import org.elasticsearch.test.ElasticsearchTestCase; import org.junit.Test; import static org.elasticsearch.common.settings.Settings.settingsBuilder; -import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; /** @@ -49,4 +49,18 @@ public class YamlSettingsLoaderTests extends ElasticsearchTestCase { assertThat(settings.getAsArray("test1.test3")[0], equalTo("test3-1")); assertThat(settings.getAsArray("test1.test3")[1], equalTo("test3-2")); } + + @Test(expected = SettingsException.class) + public void testIndentation() { + settingsBuilder() + .loadFromClasspath("org/elasticsearch/common/settings/loader/indentation-settings.yml") + .build(); + } + + @Test(expected = SettingsException.class) + public void testIndentationWithExplicitDocumentStart() { + settingsBuilder() + .loadFromClasspath("org/elasticsearch/common/settings/loader/indentation-with-explicit-document-start-settings.yml") + .build(); + } } \ No newline at end of file diff --git a/core/src/test/java/org/elasticsearch/common/settings/loader/indentation-settings.yml b/core/src/test/java/org/elasticsearch/common/settings/loader/indentation-settings.yml new file mode 100644 index 00000000000..cd14c5f35a2 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/common/settings/loader/indentation-settings.yml @@ -0,0 +1,10 @@ + test1: + value1: value1 + test2: + value2: value2 + value3: 2 + test3: + - test3-1 + - test3-2 +test4: + value4: value4 diff --git a/core/src/test/java/org/elasticsearch/common/settings/loader/indentation-with-explicit-document-start-settings.yml b/core/src/test/java/org/elasticsearch/common/settings/loader/indentation-with-explicit-document-start-settings.yml new file mode 100644 index 00000000000..e02a357d89d --- /dev/null +++ b/core/src/test/java/org/elasticsearch/common/settings/loader/indentation-with-explicit-document-start-settings.yml @@ -0,0 +1,11 @@ + test1: + value1: value1 + test2: + value2: value2 + value3: 2 + test3: + - test3-1 + - test3-2 +--- +test4: + value4: value4