From 5bf92ca3b3f5aa4957fb9fb33ae2cc4140f1c10b Mon Sep 17 00:00:00 2001 From: David Roberts Date: Wed, 14 Mar 2018 15:43:53 +0000 Subject: [PATCH] Enforce that java.io.tmpdir exists on startup (#28217) If the default java.io.tmpdir is used then the startup script creates it, but if a custom java.io.tmpdir is used then the user must ensure it exists before running Elasticsearch. If they forget then it can cause errors that are hard to understand, so this change adds an explicit check early in the bootstrap and reports a clear error if java.io.tmpdir is not an accessible directory. --- .../bootstrap/Elasticsearch.java | 7 ++++++ .../org/elasticsearch/env/Environment.java | 20 ++++++++++++++- .../elasticsearch/env/EnvironmentTests.java | 25 +++++++++++++++++-- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java index 1538f0cdf00..a0646288b1a 100644 --- a/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java +++ b/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java @@ -108,6 +108,13 @@ class Elasticsearch extends EnvironmentAwareCommand { final Path pidFile = pidfileOption.value(options); final boolean quiet = options.has(quietOption); + // a misconfigured java.io.tmpdir can cause hard-to-diagnose problems later, so reject it immediately + try { + env.validateTmpFile(); + } catch (IOException e) { + throw new UserException(ExitCodes.CONFIG, e.getMessage()); + } + try { init(daemonize, pidFile, quiet, env); } catch (NodeValidationException e) { diff --git a/server/src/main/java/org/elasticsearch/env/Environment.java b/server/src/main/java/org/elasticsearch/env/Environment.java index 2433ccf6e8e..1f4940007af 100644 --- a/server/src/main/java/org/elasticsearch/env/Environment.java +++ b/server/src/main/java/org/elasticsearch/env/Environment.java @@ -27,6 +27,7 @@ import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; +import java.io.FileNotFoundException; import java.io.IOException; import java.net.MalformedURLException; import java.net.URISyntaxException; @@ -87,9 +88,14 @@ public class Environment { private final Path pidFile; /** Path to the temporary file directory used by the JDK */ - private final Path tmpFile = PathUtils.get(System.getProperty("java.io.tmpdir")); + private final Path tmpFile; public Environment(final Settings settings, final Path configPath) { + this(settings, configPath, PathUtils.get(System.getProperty("java.io.tmpdir"))); + } + + // Should only be called directly by this class's unit tests + Environment(final Settings settings, final Path configPath, final Path tmpPath) { final Path homeFile; if (PATH_HOME_SETTING.exists(settings)) { homeFile = PathUtils.get(PATH_HOME_SETTING.get(settings)).normalize(); @@ -103,6 +109,8 @@ public class Environment { configFile = homeFile.resolve("config"); } + tmpFile = Objects.requireNonNull(tmpPath); + pluginsFile = homeFile.resolve("plugins"); List dataPaths = PATH_DATA_SETTING.get(settings); @@ -302,6 +310,16 @@ public class Environment { return tmpFile; } + /** Ensure the configured temp directory is a valid directory */ + public void validateTmpFile() throws IOException { + if (Files.exists(tmpFile) == false) { + throw new FileNotFoundException("Temporary file directory [" + tmpFile + "] does not exist or is not accessible"); + } + if (Files.isDirectory(tmpFile) == false) { + throw new IOException("Configured temporary file directory [" + tmpFile + "] is not a directory"); + } + } + public static FileStore getFileStore(final Path path) throws IOException { return new ESFileStore(Files.getFileStore(path)); } diff --git a/server/src/test/java/org/elasticsearch/env/EnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/EnvironmentTests.java index 5ca3f4dc6a5..5ada31b6129 100644 --- a/server/src/test/java/org/elasticsearch/env/EnvironmentTests.java +++ b/server/src/test/java/org/elasticsearch/env/EnvironmentTests.java @@ -21,6 +21,7 @@ package org.elasticsearch.env; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; +import java.io.FileNotFoundException; import java.io.IOException; import java.net.URL; import java.nio.file.Path; @@ -28,6 +29,7 @@ import java.nio.file.Path; import static org.hamcrest.CoreMatchers.endsWith; import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.Matchers.arrayWithSize; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -37,11 +39,11 @@ import static org.hamcrest.Matchers.hasToString; * Simple unit-tests for Environment.java */ public class EnvironmentTests extends ESTestCase { - public Environment newEnvironment() throws IOException { + public Environment newEnvironment() { return newEnvironment(Settings.EMPTY); } - public Environment newEnvironment(Settings settings) throws IOException { + public Environment newEnvironment(Settings settings) { Settings build = Settings.builder() .put(settings) .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath()) @@ -146,4 +148,23 @@ public class EnvironmentTests extends ESTestCase { assertThat(e, hasToString(containsString("node does not require local storage yet path.data is set to [" + pathData + "]"))); } + public void testNonExistentTempPathValidation() { + Settings build = Settings.builder() + .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) + .build(); + Environment environment = new Environment(build, null, createTempDir().resolve("this_does_not_exist")); + FileNotFoundException e = expectThrows(FileNotFoundException.class, environment::validateTmpFile); + assertThat(e.getMessage(), startsWith("Temporary file directory [")); + assertThat(e.getMessage(), endsWith("this_does_not_exist] does not exist or is not accessible")); + } + + public void testTempPathValidationWhenRegularFile() throws IOException { + Settings build = Settings.builder() + .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) + .build(); + Environment environment = new Environment(build, null, createTempFile("something", ".test")); + IOException e = expectThrows(IOException.class, environment::validateTmpFile); + assertThat(e.getMessage(), startsWith("Configured temporary file directory [")); + assertThat(e.getMessage(), endsWith(".test] is not a directory")); + } }