Do not swallow exceptions thrown while parsing settings

This commit fixes an issue that was causing Elasticsearch to silently
ignore settings files that contain garbage. The underlying issue was
swallowing an SettingsException under the assumption that the only
reason that an exception could be throw was due to the settings file
not existing (in this case the IOException would be the cause of the
swallowed SettingsException). This assumption is mistaken as an
IOException could also be thrown due to an access error or a read
error. Additionally, a SettingsException could be thrown exactly
because garbage was found in the settings file. We should instead
explicitly check that the settings file exists, and bomb on an
exception thrown for any reason.

Closes #13028
This commit is contained in:
Jason Tedor 2015-08-21 10:43:48 -04:00
parent 13c1c27d5a
commit f4774d17a6
3 changed files with 25 additions and 6 deletions

View File

@ -114,10 +114,9 @@ public class InternalSettingsPreparer {
} }
if (loadFromEnv) { if (loadFromEnv) {
for (String allowedSuffix : ALLOWED_SUFFIXES) { for (String allowedSuffix : ALLOWED_SUFFIXES) {
try { Path path = environment.configFile().resolve("elasticsearch" + allowedSuffix);
settingsBuilder.loadFromPath(environment.configFile().resolve("elasticsearch" + allowedSuffix)); if (Files.exists(path)) {
} catch (SettingsException e) { settingsBuilder.loadFromPath(path);
// ignore
} }
} }
} }

View File

@ -23,15 +23,15 @@ import org.elasticsearch.common.cli.CliToolTestCase;
import org.elasticsearch.common.cli.Terminal; import org.elasticsearch.common.cli.Terminal;
import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.settings.SettingsException;
import org.elasticsearch.env.Environment; import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.ESTestCase;
import org.junit.After; import org.junit.After;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
import java.util.ArrayList; import java.util.ArrayList;
@ -235,4 +235,17 @@ public class InternalSettingsPreparerTests extends ESTestCase {
assertThat(settings.get("name"), is("prompted name 0")); assertThat(settings.get("name"), is("prompted name 0"));
assertThat(settings.get("node.name"), is("prompted name 0")); assertThat(settings.get("node.name"), is("prompted name 0"));
} }
@Test(expected = SettingsException.class)
public void testGarbageIsNotSwallowed() throws IOException {
InputStream garbage = getClass().getResourceAsStream("/config/garbage/garbage.yml");
Path home = createTempDir();
Path config = home.resolve("config");
Files.createDirectory(config);
Files.copy(garbage, config.resolve("elasticsearch.yml"));
InternalSettingsPreparer.prepareSettings(settingsBuilder()
.put("config.ignore_system_properties", true)
.put("path.home", home)
.build(), true);
}
} }

View File

@ -0,0 +1,7 @@
SKDFLK@$#L%@KL#%L#@$#@L$ #L$@$ #L@K$#L $L $K#L#@L $#L
!!@!@$(#%#)(@)% #(%)
#(%#@)%@#)% (@#%()
()#%@#% (@ )%@%(@#)% @( %)@ %(@)
)(%)@()(%)()(#%)@#
node.name: "Hiro Takachiho"