Skip to content

Enforce that java.io.tmpdir exists on startup #28217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th
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) {
Expand Down
20 changes: 19 additions & 1 deletion server/src/main/java/org/elasticsearch/env/Environment.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
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;
Expand Down Expand Up @@ -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();
Expand All @@ -103,6 +109,8 @@ public Environment(final Settings settings, final Path configPath) {
configFile = homeFile.resolve("config");
}

tmpFile = Objects.requireNonNull(tmpPath);

pluginsFile = homeFile.resolve("plugins");

List<String> dataPaths = PATH_DATA_SETTING.get(settings);
Expand Down Expand Up @@ -302,6 +310,16 @@ public Path tmpFile() {
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));
}
Expand Down
25 changes: 23 additions & 2 deletions server/src/test/java/org/elasticsearch/env/EnvironmentTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@
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;

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;
Expand All @@ -37,11 +39,11 @@
* 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())
Expand Down Expand Up @@ -146,4 +148,23 @@ public void testNodeDoesNotRequireLocalStorageButHasPathData() {
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"));
}
}