Skip to content

Commit

Permalink
DRILL-7936: Remove usage of Guava Files#createTempDir method
Browse files Browse the repository at this point in the history
Guava's Files#createTempDir() method has some security issues, and since
better alternatives exist (including Java 7
Files#createTempDirectory(String) method), the function has been
deprecated in Guava 30.0 (but the security issues haven't been
addressed).

Introduce a drop-in replacement for this method in DrillFileUtils based
on the Java7 Files API and replace usage of the Guava method with the
new method in the codebase.
  • Loading branch information
laurentgo authored and luocooong committed May 27, 2021
1 parent 89be428 commit 72b4ae0
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,19 @@ public static File getResourceAsFile(String fileName) throws IOException {
public static String getResourceAsString(String fileName) throws IOException {
return Files.asCharSource(getResourceAsFile(fileName), Charsets.UTF_8).read();
}

/**
* Creates a temporary directory under the default temporary directory location.
* This is a safe replacement for Guava {@code Files#createTempDir()}
*
* @return a temporary directory
* @throws IllegalStateException if the directory cannot be created
*/
public static File createTempDir() {
try {
return java.nio.file.Files.createTempDirectory(System.currentTimeMillis() + "-").toFile();
} catch (IOException e) {
throw new IllegalStateException("Failed to create temporary directory");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.apache.drill.common.expression.fn.FunctionReplacementUtils;
import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
import org.apache.drill.shaded.guava.com.google.common.collect.Sets;
import org.apache.drill.shaded.guava.com.google.common.io.Files;
import com.typesafe.config.ConfigFactory;
import org.apache.commons.io.FileUtils;
import org.apache.drill.common.config.ConfigConstants;
Expand All @@ -49,6 +48,7 @@
import org.apache.drill.common.types.TypeProtos.DataMode;
import org.apache.drill.common.types.TypeProtos.MajorType;
import org.apache.drill.common.types.TypeProtos.MinorType;
import org.apache.drill.common.util.DrillFileUtils;
import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.coord.store.TransientStoreEvent;
import org.apache.drill.exec.coord.store.TransientStoreListener;
Expand Down Expand Up @@ -532,7 +532,7 @@ private Path getLocalUdfDir(DrillConfig config) {
/**
* First tries to get drill temporary directory value from from config ${drill.tmp-dir},
* then checks environmental variable $DRILL_TMP_DIR.
* If value is still missing, generates directory using {@link Files#createTempDir()}.
* If value is still missing, generates directory using {@link DrillFileUtils#createTempDir()}.
* If temporary directory was generated, sets {@link #deleteTmpDir} to true
* to delete directory on drillbit exit.
*
Expand All @@ -549,7 +549,7 @@ private File getTmpDir(DrillConfig config) {

if (drillTempDir == null) {
deleteTmpDir = true;
return Files.createTempDir();
return DrillFileUtils.createTempDir();
}

return new File(drillTempDir);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@
package org.apache.drill.exec.planner.sql.handlers;

import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
import org.apache.drill.shaded.guava.com.google.common.io.Files;
import org.apache.calcite.sql.SqlCharStringLiteral;
import org.apache.calcite.sql.SqlNode;
import org.apache.commons.io.FileUtils;
import org.apache.drill.common.exceptions.DrillRuntimeException;
import org.apache.drill.common.exceptions.UserException;
import org.apache.drill.common.util.DrillFileUtils;
import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.exception.FunctionValidationException;
import org.apache.drill.exec.exception.JarValidationException;
Expand Down Expand Up @@ -236,7 +236,7 @@ private class JarManager {
this.registryBinary = new Path(remoteRegistry.getRegistryArea(), binaryName);
this.registrySource = new Path(remoteRegistry.getRegistryArea(), sourceName);

this.localTmpDir = new Path(Files.createTempDir().toURI());
this.localTmpDir = new Path(DrillFileUtils.createTempDir().toURI());
this.fs = remoteRegistry.getFs();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.apache.commons.lang3.StringUtils;
import org.apache.drill.common.config.DrillConfig;
import org.apache.drill.common.exceptions.DrillException;
import org.apache.drill.common.util.DrillFileUtils;
import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.exception.DrillbitStartupException;
import org.apache.drill.exec.expr.fn.registry.FunctionHolder;
Expand Down Expand Up @@ -393,7 +394,7 @@ public void close() throws Exception {
*/
public File getOrCreateTmpJavaScriptDir() {
if (tmpJavaScriptDir == null && this.drillbit.getContext() != null) {
tmpJavaScriptDir = org.apache.drill.shaded.guava.com.google.common.io.Files.createTempDir();
tmpJavaScriptDir = DrillFileUtils.createTempDir();
// Perform All auto generated files at this point
try {
generateOptionsDescriptionJSFile();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
package org.apache.drill.exec.store;

import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
import org.apache.drill.shaded.guava.com.google.common.io.Files;
import org.apache.drill.common.util.DrillFileUtils;
import org.apache.drill.exec.ExecTest;
import org.apache.drill.test.BaseTest;
import org.apache.hadoop.fs.FileSystem;
Expand Down Expand Up @@ -173,7 +173,7 @@ public void testInvalidUmask() throws Exception {
}

private Path prepareStorageDirectory() throws IOException {
File storageDirectory = Files.createTempDir();
File storageDirectory = DrillFileUtils.createTempDir();
storageDirectory.deleteOnExit();
Path path = new Path(storageDirectory.toURI().getPath());
fs.setPermission(path, FULL_PERMISSION);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
package org.apache.drill.exec.util;

import org.apache.drill.shaded.guava.com.google.common.base.Strings;
import org.apache.drill.shaded.guava.com.google.common.io.Files;
import org.apache.commons.io.FileUtils;
import org.apache.drill.common.util.DrillFileUtils;
import org.apache.drill.exec.ExecTest;
import org.apache.drill.test.BaseTest;
import org.apache.hadoop.fs.FileSystem;
Expand Down Expand Up @@ -68,7 +68,7 @@ public static void setup() throws IOException {
fs = ExecTest.getLocalFileSystem();

// create temporary directory with sub-folders and files
final File tempDir = Files.createTempDir();
final File tempDir = DrillFileUtils.createTempDir();
Runtime.getRuntime().addShutdownHook(new Thread(() -> FileUtils.deleteQuietly(tempDir)));
base = new Path(tempDir.toURI().getPath());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@

import org.apache.commons.io.FileUtils;
import org.apache.drill.common.config.DrillConfig;
import org.apache.drill.common.util.DrillFileUtils;
import org.apache.drill.exec.memory.BufferAllocator;
import org.apache.drill.exec.vector.ValueVector;
import org.apache.drill.shaded.guava.com.google.common.io.Files;

import io.netty.buffer.DrillBuf;

Expand All @@ -46,7 +46,7 @@ public class BaseFixture {
*/

public static File getTempDir(final String dirName) {
final File dir = Files.createTempDir();
final File dir = DrillFileUtils.createTempDir();
Runtime.getRuntime().addShutdownHook(new Thread() {
@Override
public void run() {
Expand Down

0 comments on commit 72b4ae0

Please sign in to comment.