diff --git a/core/src/main/java/org/elasticsearch/common/settings/loader/PropertiesSettingsLoader.java b/core/src/main/java/org/elasticsearch/common/settings/loader/PropertiesSettingsLoader.java index 0bc97376bb0..8bff4ad0255 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/loader/PropertiesSettingsLoader.java +++ b/core/src/main/java/org/elasticsearch/common/settings/loader/PropertiesSettingsLoader.java @@ -20,6 +20,7 @@ package org.elasticsearch.common.settings.loader; import org.apache.lucene.util.IOUtils; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.io.FastStringReader; import org.elasticsearch.common.io.stream.StreamInput; @@ -36,7 +37,7 @@ public class PropertiesSettingsLoader implements SettingsLoader { @Override public Map load(String source) throws IOException { - Properties props = new Properties(); + Properties props = new NoDuplicatesProperties(); FastStringReader reader = new FastStringReader(source); try { props.load(reader); @@ -52,7 +53,7 @@ public class PropertiesSettingsLoader implements SettingsLoader { @Override public Map load(byte[] source) throws IOException { - Properties props = new Properties(); + Properties props = new NoDuplicatesProperties(); StreamInput stream = StreamInput.wrap(source); try { props.load(stream); @@ -65,4 +66,15 @@ public class PropertiesSettingsLoader implements SettingsLoader { IOUtils.closeWhileHandlingException(stream); } } + + class NoDuplicatesProperties extends Properties { + @Override + public synchronized Object put(Object key, Object value) { + Object previousValue = super.put(key, value); + if (previousValue != null) { + throw new ElasticsearchParseException("duplicate settings key [{}] found, previous value [{}], current value [{}]", key, previousValue, value); + } + return previousValue; + } + } } 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 e3e08fb93f2..23c5d447582 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,7 +20,6 @@ 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; @@ -141,7 +140,18 @@ public abstract class XContentSettingsLoader implements SettingsLoader { sb.append(pathEle).append('.'); } sb.append(fieldName); - settings.put(sb.toString(), parser.text()); + String key = sb.toString(); + String currentValue = parser.text(); + String previousValue = settings.put(key, currentValue); + if (previousValue != null) { + throw new ElasticsearchParseException( + "duplicate settings key [{}] found at line number [{}], column number [{}], previous value [{}], current value [{}]", + key, + parser.getTokenLocation().lineNumber, + parser.getTokenLocation().columnNumber, + previousValue, + currentValue + ); + } } - } 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 142d60871aa..0f90b8c3728 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 @@ -19,19 +19,19 @@ package org.elasticsearch.common.settings.loader; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.settings.SettingsException; import org.elasticsearch.test.ESTestCase; import org.junit.Test; import static org.elasticsearch.common.settings.Settings.settingsBuilder; -import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; /** * */ public class JsonSettingsLoaderTests extends ESTestCase { - @Test public void testSimpleJsonSettings() throws Exception { String json = "/org/elasticsearch/common/settings/loader/test-settings.json"; @@ -50,4 +50,17 @@ public class JsonSettingsLoaderTests extends ESTestCase { assertThat(settings.getAsArray("test1.test3")[0], equalTo("test3-1")); assertThat(settings.getAsArray("test1.test3")[1], equalTo("test3-2")); } + + public void testDuplicateKeysThrowsException() { + String json = "{\"foo\":\"bar\",\"foo\":\"baz\"}"; + try { + settingsBuilder() + .loadFromSource(json) + .build(); + fail("expected exception"); + } catch (SettingsException e) { + assertEquals(e.getCause().getClass(), ElasticsearchParseException.class); + assertTrue(e.toString().contains("duplicate settings key [foo] found at line number [1], column number [13], previous value [bar], current value [baz]")); + } + } } 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 new file mode 100644 index 00000000000..7a1897fbaf9 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/common/settings/loader/PropertiesSettingsLoaderTests.java @@ -0,0 +1,47 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.settings.loader; + +import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; +import java.nio.charset.Charset; + +public class PropertiesSettingsLoaderTests extends ESTestCase { + public void testDuplicateKeyFromStringThrowsException() throws IOException { + PropertiesSettingsLoader loader = new PropertiesSettingsLoader(); + try { + loader.load("foo=bar\nfoo=baz"); + fail("expected exception"); + } catch (ElasticsearchParseException e) { + assertEquals(e.getMessage(), "duplicate settings key [foo] found, previous value [bar], current value [baz]"); + } + } + + public void testDuplicateKeysFromBytesThrowsException() throws IOException { + PropertiesSettingsLoader loader = new PropertiesSettingsLoader(); + try { + loader.load("foo=bar\nfoo=baz".getBytes(Charset.defaultCharset())); + } catch (ElasticsearchParseException e) { + assertEquals(e.getMessage(), "duplicate settings key [foo] found, previous value [bar], current value [baz]"); + } + } +} 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 49b5444a52b..60bf80a6e9d 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 @@ -19,6 +19,7 @@ package org.elasticsearch.common.settings.loader; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsException; import org.elasticsearch.test.ESTestCase; @@ -31,7 +32,6 @@ import static org.hamcrest.Matchers.equalTo; * */ public class YamlSettingsLoaderTests extends ESTestCase { - @Test public void testSimpleYamlSettings() throws Exception { String yaml = "/org/elasticsearch/common/settings/loader/test-settings.yml"; @@ -66,4 +66,17 @@ public class YamlSettingsLoaderTests extends ESTestCase { .loadFromStream(yaml, getClass().getResourceAsStream(yaml)) .build(); } -} \ No newline at end of file + + public void testDuplicateKeysThrowsException() { + String yaml = "foo: bar\nfoo: baz"; + try { + settingsBuilder() + .loadFromSource(yaml) + .build(); + fail("expected exception"); + } catch (SettingsException e) { + assertEquals(e.getCause().getClass(), ElasticsearchParseException.class); + assertTrue(e.toString().contains("duplicate settings key [foo] found at line number [2], column number [6], previous value [bar], current value [baz]")); + } + } +}