DRILL-5694: Do not allow queries to access paths outside the current workspace 
root

closes #1050


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/53d71dfd
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/53d71dfd
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/53d71dfd

Branch: refs/heads/master
Commit: 53d71dfdff414d705587a051b6e7ef00614de723
Parents: 5f044f2
Author: Parth Chandra <par...@apache.org>
Authored: Tue Nov 14 15:25:33 2017 -0800
Committer: Arina Ielchiieva <arina.yelchiy...@gmail.com>
Committed: Wed Nov 29 12:22:31 2017 +0200

----------------------------------------------------------------------
 .../drill/exec/store/dfs/FileSelection.java     | 29 +++++++++--
 .../drill/exec/store/dfs/WorkspaceConfig.java   | 24 +++++++--
 .../exec/store/dfs/WorkspaceSchemaFactory.java  | 12 +++--
 .../drill/exec/util/StoragePluginTestUtils.java |  2 +-
 .../resources/bootstrap-storage-plugins.json    |  6 ++-
 .../org/apache/drill/TestExampleQueries.java    |  6 +--
 .../apache/drill/exec/TestWindowFunctions.java  | 26 +++++-----
 .../impersonation/BaseTestImpersonation.java    |  4 +-
 .../org/apache/drill/exec/sql/TestCTTAS.java    |  2 +-
 .../drill/exec/store/dfs/TestFileSelection.java | 53 ++++++++++++++++++++
 .../org/apache/drill/test/ClusterFixture.java   |  2 +-
 11 files changed, 131 insertions(+), 35 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/53d71dfd/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
----------------------------------------------------------------------
diff --git 
a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
 
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
index 6aff1dd..7fa981b 100644
--- 
a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
+++ 
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
@@ -252,11 +252,15 @@ public class FileSelection {
     return builder.toString();
   }
 
-  public static FileSelection create(final DrillFileSystem fs, final String 
parent, final String path) throws IOException {
+  public static FileSelection create(final DrillFileSystem fs, final String 
parent, final String path,
+      final boolean allowAccessOutsideWorkspace) throws IOException {
     Stopwatch timer = Stopwatch.createStarted();
     boolean hasWildcard = path.contains(WILD_CARD);
 
     final Path combined = new Path(parent, removeLeadingSlash(path));
+    if (!allowAccessOutsideWorkspace) {
+      checkBackPaths(new Path(parent).toUri().getPath(), 
combined.toUri().getPath(), path);
+    }
     final FileStatus[] statuses = fs.globStatus(combined); // note: this would 
expand wildcards
     if (statuses == null) {
       return null;
@@ -359,8 +363,8 @@ public class FileSelection {
     }
   }
 
-  private static String removeLeadingSlash(String path) {
-    if (path.charAt(0) == '/') {
+  public static String removeLeadingSlash(String path) {
+    if (!path.isEmpty() && path.charAt(0) == '/') {
       String newPath = path.substring(1);
       return removeLeadingSlash(newPath);
     } else {
@@ -368,6 +372,25 @@ public class FileSelection {
     }
   }
 
+  /**
+   * Check if the path is a valid sub path under the parent after removing 
backpaths. Throw an exception if
+   * it is not. We pass subpath in as a parameter only for the error message
+   *
+   * @param parent The parent path (the workspace directory).
+   * @param combinedPath The workspace directory and (relative) subpath path 
combined.
+   * @param subpath For error message only, the subpath
+   */
+  public static void checkBackPaths(String parent, String combinedPath, String 
subpath) {
+    Preconditions.checkArgument(!parent.isEmpty(), "Invalid root (" + parent + 
") in file selection path.");
+    Preconditions.checkArgument(!combinedPath.isEmpty(), "Empty path (" + 
combinedPath + "( in file selection path.");
+
+    if (!combinedPath.startsWith(parent)) {
+      StringBuilder msg = new StringBuilder();
+      msg.append("Invalid path : ").append(subpath).append(" takes you outside 
the workspace.");
+      throw new IllegalArgumentException(msg.toString());
+    }
+  }
+
   public List<FileStatus> getFileStatuses() {
     return statuses;
   }

http://git-wip-us.apache.org/repos/asf/drill/blob/53d71dfd/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceConfig.java
----------------------------------------------------------------------
diff --git 
a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceConfig.java
 
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceConfig.java
index f9b0097..7ac0744 100644
--- 
a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceConfig.java
+++ 
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceConfig.java
@@ -26,22 +26,28 @@ import com.fasterxml.jackson.annotation.JsonProperty;
  *  - writable flag to indicate whether the location supports creating new 
tables.
  *  - default storage format for new tables created in this workspace.
  */
-@JsonIgnoreProperties(value = {"storageformat"})
+@JsonIgnoreProperties(value = {"storageformat"}, ignoreUnknown = true)
+
 public class WorkspaceConfig {
 
   /** Default workspace is a root directory which supports read, but not 
write. */
-  public static final WorkspaceConfig DEFAULT = new WorkspaceConfig("/", 
false, null);
+  public static final WorkspaceConfig DEFAULT = new WorkspaceConfig("/", 
false, null, false);
 
   private final String location;
   private final boolean writable;
   private final String defaultInputFormat;
-
+  private final boolean allowAccessOutsideWorkspace; // do not allow access 
outside the workspace by default.
+                                                     // For backward 
compatibility, the user can turn this
+                                                     // on.
   public WorkspaceConfig(@JsonProperty("location") String location,
                          @JsonProperty("writable") boolean writable,
-                         @JsonProperty("defaultInputFormat") String 
defaultInputFormat) {
+                         @JsonProperty("defaultInputFormat") String 
defaultInputFormat,
+                         @JsonProperty("allowAccessOutsideWorkspace") boolean 
allowAccessOutsideWorkspace
+      ) {
     this.location = location;
     this.writable = writable;
     this.defaultInputFormat = defaultInputFormat;
+    this.allowAccessOutsideWorkspace = allowAccessOutsideWorkspace;
   }
 
   public String getLocation() {
@@ -56,6 +62,12 @@ public class WorkspaceConfig {
     return defaultInputFormat;
   }
 
+  @JsonProperty
+  public Boolean allowAccessOutsideWorkspace() {
+    return allowAccessOutsideWorkspace;
+  }
+
+
   @Override
   public int hashCode() {
     final int prime = 31;
@@ -63,6 +75,7 @@ public class WorkspaceConfig {
     result = prime * result + ((defaultInputFormat == null) ? 0 : 
defaultInputFormat.hashCode());
     result = prime * result + ((location == null) ? 0 : location.hashCode());
     result = prime * result + (writable ? 1231 : 1237);
+    result = prime * result + (allowAccessOutsideWorkspace ? 1231 : 1237);
     return result;
   }
 
@@ -95,6 +108,9 @@ public class WorkspaceConfig {
     if (writable != other.writable) {
       return false;
     }
+    if (allowAccessOutsideWorkspace != other.allowAccessOutsideWorkspace) {
+      return false;
+    }
     return true;
   }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/53d71dfd/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java
----------------------------------------------------------------------
diff --git 
a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java
 
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java
index 6629fc4..3934958 100644
--- 
a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java
+++ 
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java
@@ -171,7 +171,7 @@ public class WorkspaceSchemaFactory {
        * In other cases, we use access method since it is cheaper.
        */
       if (SystemUtils.IS_OS_WINDOWS && 
fs.getUri().getScheme().equalsIgnoreCase(FileSystemSchemaFactory.LOCAL_FS_SCHEME))
 {
-        fs.listStatus(wsPath);
+      fs.listStatus(wsPath);
       }
       else {
         fs.access(wsPath, FsAction.READ);
@@ -192,7 +192,7 @@ public class WorkspaceSchemaFactory {
   public WorkspaceSchema createSchema(List<String> parentSchemaPath, 
SchemaConfig schemaConfig, DrillFileSystem fs) throws IOException {
     if (!accessible(fs)) {
       return null;
-    }
+  }
     return new WorkspaceSchema(parentSchemaPath, schemaName, schemaConfig, fs);
   }
 
@@ -257,7 +257,7 @@ public class WorkspaceSchemaFactory {
             .validationError()
             .message("Unable to find table [%s] in schema [%s]", sig.name, 
schema.getFullSchemaName())
             .build(logger);
-      }
+    }
       return new DrillTranslatableTable(drillTable);
     }
 
@@ -605,7 +605,8 @@ public class WorkspaceSchemaFactory {
     @Override
     public DrillTable create(TableInstance key) {
       try {
-        final FileSelection fileSelection = FileSelection.create(getFS(), 
config.getLocation(), key.sig.name);
+        final FileSelection fileSelection = FileSelection
+            .create(getFS(), config.getLocation(), key.sig.name, 
config.allowAccessOutsideWorkspace());
         if (fileSelection == null) {
           return null;
         }
@@ -684,7 +685,8 @@ public class WorkspaceSchemaFactory {
      * @throws IOException is case of problems accessing table files
      */
     private boolean isHomogeneous(String tableName) throws IOException {
-      FileSelection fileSelection = FileSelection.create(getFS(), 
config.getLocation(), tableName);
+      FileSelection fileSelection =
+          FileSelection.create(getFS(), config.getLocation(), tableName, 
config.allowAccessOutsideWorkspace());
 
       if (fileSelection == null) {
         throw UserException

http://git-wip-us.apache.org/repos/asf/drill/blob/53d71dfd/exec/java-exec/src/main/java/org/apache/drill/exec/util/StoragePluginTestUtils.java
----------------------------------------------------------------------
diff --git 
a/exec/java-exec/src/main/java/org/apache/drill/exec/util/StoragePluginTestUtils.java
 
b/exec/java-exec/src/main/java/org/apache/drill/exec/util/StoragePluginTestUtils.java
index 689a5bf..83e3985 100644
--- 
a/exec/java-exec/src/main/java/org/apache/drill/exec/util/StoragePluginTestUtils.java
+++ 
b/exec/java-exec/src/main/java/org/apache/drill/exec/util/StoragePluginTestUtils.java
@@ -77,7 +77,7 @@ public class StoragePluginTestUtils {
     for (String schema: schemas) {
       WorkspaceConfig workspaceConfig = pluginConfig.workspaces.get(schema);
       String inputFormat = workspaceConfig == null ? null: 
workspaceConfig.getDefaultInputFormat();
-      WorkspaceConfig newWorkspaceConfig = new 
WorkspaceConfig(tmpDirPath.getAbsolutePath(), true, inputFormat);
+      WorkspaceConfig newWorkspaceConfig = new 
WorkspaceConfig(tmpDirPath.getAbsolutePath(), true, inputFormat, false);
       workspaces.put(schema, newWorkspaceConfig);
     }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/53d71dfd/exec/java-exec/src/main/resources/bootstrap-storage-plugins.json
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/resources/bootstrap-storage-plugins.json 
b/exec/java-exec/src/main/resources/bootstrap-storage-plugins.json
index 18a6967..0b6add0 100644
--- a/exec/java-exec/src/main/resources/bootstrap-storage-plugins.json
+++ b/exec/java-exec/src/main/resources/bootstrap-storage-plugins.json
@@ -6,11 +6,13 @@
       workspaces: {
         "root" : {
           location: "/",
-          writable: false
+          writable: false,
+          allowAccessOutsideWorkspace: false
         },
         "tmp" : {
           location: "/tmp",
-          writable: true
+          writable: true,
+          allowAccessOutsideWorkspace: false
         }
       },
       formats: {

http://git-wip-us.apache.org/repos/asf/drill/blob/53d71dfd/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
----------------------------------------------------------------------
diff --git 
a/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 
b/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
index bcfe8f3..7de64c9 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
@@ -566,8 +566,8 @@ public class TestExampleQueries extends BaseTestQuery {
         expectedRecordCount, actualRecordCount), expectedRecordCount, 
actualRecordCount);
 
     // source is CSV
-    String root = 
DrillFileUtils.getResourceAsFile("/store/text/data/regions.csv").toURI().toString();
-    String query = String.format("select rid, x.name from (select columns[0] 
as RID, columns[1] as NAME from dfs.`%s`) X where X.rid = 2", root);
+    String root = "store/text/data/regions.csv";
+    String query = String.format("select rid, x.name from (select columns[0] 
as RID, columns[1] as NAME from cp.`%s`) X where X.rid = 2", root);
     actualRecordCount = testSql(query);
     expectedRecordCount = 1;
     assertEquals(String.format("Received unexpected number of rows in output: 
expected=%d, received=%s",
@@ -1166,4 +1166,4 @@ public class TestExampleQueries extends BaseTestQuery {
         .build()
         .run();
   }
-}
\ No newline at end of file
+}

http://git-wip-us.apache.org/repos/asf/drill/blob/53d71dfd/exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java
----------------------------------------------------------------------
diff --git 
a/exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java 
b/exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java
index 7d7f4a5..7fac487 100644
--- 
a/exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java
+++ 
b/exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java
@@ -513,9 +513,9 @@ public class TestWindowFunctions extends BaseTestQuery {
 
   @Test
   public void testCompoundIdentifierInWindowDefinition() throws Exception {
-    String root = 
DrillFileUtils.getResourceAsFile("/multilevel/csv/1994/Q1/orders_94_q1.csv").toURI().toString();
+    String root = "/multilevel/csv/1994/Q1/orders_94_q1.csv";
     String query = String.format("SELECT count(*) OVER w as col1, count(*) 
OVER w as col2 \n" +
-        "FROM dfs.`%s` \n" +
+        "FROM cp.`%s` \n" +
         "WINDOW w AS (PARTITION BY columns[1] ORDER BY columns[0] DESC)", 
root);
 
     // Validate the plan
@@ -609,11 +609,11 @@ public class TestWindowFunctions extends BaseTestQuery {
 
   @Test // DRILL-3567
   public void testMultiplePartitions1() throws Exception {
-    String root = 
DrillFileUtils.getResourceAsFile("/store/text/data/t.json").toURI().toString();
+    String root = "/store/text/data/t.json";
     String query = String.format("select count(*) over(partition by b1 order 
by c1) as count1, \n" +
         "sum(a1)  over(partition by b1 order by c1) as sum1, \n" +
         "count(*) over(partition by a1 order by c1) as count2 \n" +
-        "from dfs.`%s`", root);
+        "from cp.`%s`", root);
 
     // Validate the plan
     final String[] expectedPlan = {"Window.*partition \\{2\\} order by 
\\[1\\].*COUNT\\(\\)",
@@ -642,11 +642,11 @@ public class TestWindowFunctions extends BaseTestQuery {
 
   @Test // DRILL-3567
   public void testMultiplePartitions2() throws Exception {
-    String root = 
DrillFileUtils.getResourceAsFile("/store/text/data/t.json").toURI().toString();
+    String root = "/store/text/data/t.json";
     String query = String.format("select count(*) over(partition by b1 order 
by c1) as count1, \n" +
         "count(*) over(partition by a1 order by c1) as count2, \n" +
         "sum(a1)  over(partition by b1 order by c1) as sum1 \n" +
-        "from dfs.`%s`", root);
+        "from cp.`%s`", root);
 
     // Validate the plan
     final String[] expectedPlan = {"Window.*partition \\{2\\} order by 
\\[1\\].*COUNT\\(\\)",
@@ -675,9 +675,9 @@ public class TestWindowFunctions extends BaseTestQuery {
 
   @Test // see DRILL-3574
   public void testWithAndWithoutPartitions() throws Exception {
-    String root = 
DrillFileUtils.getResourceAsFile("/store/text/data/t.json").toURI().toString();
+    String root = "/store/text/data/t.json";
     String query = String.format("select sum(a1) over(partition by b1, c1) as 
s1, sum(a1) over() as s2 \n" +
-        "from dfs.`%s` \n" +
+        "from cp.`%s` \n" +
         "order by a1", root);
     test("alter session set `planner.slice_target` = 1");
 
@@ -706,10 +706,10 @@ public class TestWindowFunctions extends BaseTestQuery {
 
   @Test // see DRILL-3657
   public void testConstantsInMultiplePartitions() throws Exception {
-    String root = 
DrillFileUtils.getResourceAsFile("/store/text/data/t.json").toURI().toString();
+    String root = "/store/text/data/t.json";
     String query = String.format(
         "select sum(1) over(partition by b1 order by a1) as sum1, sum(1) 
over(partition by a1) as sum2, rank() over(order by b1) as rank1, rank() 
over(order by 1) as rank2 \n" +
-        "from dfs.`%s` \n" +
+        "from cp.`%s` \n" +
         "order by 1, 2, 3, 4", root);
 
     // Validate the plan
@@ -740,9 +740,9 @@ public class TestWindowFunctions extends BaseTestQuery {
 
   @Test // DRILL-3580
   public void testExpressionInWindowFunction() throws Exception {
-    String root = 
DrillFileUtils.getResourceAsFile("/store/text/data/t.json").toURI().toString();
+    String root = "/store/text/data/t.json";
     String query = String.format("select a1, b1, sum(b1) over (partition by 
a1) as c1, sum(a1 + b1) over (partition by a1) as c2\n" +
-        "from dfs.`%s`", root);
+        "from cp.`%s`", root);
 
     // Validate the plan
     final String[] expectedPlan = {"Window\\(window#0=\\[window\\(partition 
\\{0\\} order by \\[\\].*\\[SUM\\(\\$1\\), SUM\\(\\$2\\)\\]"};
@@ -935,4 +935,4 @@ public class TestWindowFunctions extends BaseTestQuery {
       assert(ex.getMessage().contains("Expression 
'tpch/nation.parquet.n_nationkey' is not being grouped"));
     }
   }
-}
\ No newline at end of file
+}

http://git-wip-us.apache.org/repos/asf/drill/blob/53d71dfd/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/BaseTestImpersonation.java
----------------------------------------------------------------------
diff --git 
a/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/BaseTestImpersonation.java
 
b/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/BaseTestImpersonation.java
index b2671be..f27b511 100644
--- 
a/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/BaseTestImpersonation.java
+++ 
b/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/BaseTestImpersonation.java
@@ -146,7 +146,7 @@ public class BaseTestImpersonation extends PlanTestBase {
     final Path dirPath = new Path(path);
     FileSystem.mkdirs(fs, dirPath, new FsPermission(permissions));
     fs.setOwner(dirPath, owner, group);
-    final WorkspaceConfig ws = new WorkspaceConfig(path, true, "parquet");
+    final WorkspaceConfig ws = new WorkspaceConfig(path, true, "parquet", 
false);
     workspaces.put(name, ws);
   }
 
@@ -193,4 +193,4 @@ public class BaseTestImpersonation extends PlanTestBase {
     final Path viewFilePath = new Path("/tmp/", viewName + 
DotDrillType.VIEW.getEnding());
     fs.setOwner(viewFilePath, viewOwner, viewGroup);
   }
-}
\ No newline at end of file
+}

http://git-wip-us.apache.org/repos/asf/drill/blob/53d71dfd/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTTAS.java
----------------------------------------------------------------------
diff --git 
a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTTAS.java 
b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTTAS.java
index e83fa37..1553227 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTTAS.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTTAS.java
@@ -75,7 +75,7 @@ public class TestCTTAS extends BaseTestQuery {
 
     StoragePluginRegistry pluginRegistry = getDrillbitContext().getStorage();
     FileSystemConfig pluginConfig = (FileSystemConfig) 
pluginRegistry.getPlugin(DFS_PLUGIN_NAME).getConfig();
-    pluginConfig.workspaces.put(temp2_wk, new 
WorkspaceConfig(tmp2.getAbsolutePath(), true, null));
+    pluginConfig.workspaces.put(temp2_wk, new 
WorkspaceConfig(tmp2.getAbsolutePath(), true, null, false));
     pluginRegistry.createOrUpdate(DFS_PLUGIN_NAME, pluginConfig, true);
 
     fs = getLocalFileSystem();

http://git-wip-us.apache.org/repos/asf/drill/blob/53d71dfd/exec/java-exec/src/test/java/org/apache/drill/exec/store/dfs/TestFileSelection.java
----------------------------------------------------------------------
diff --git 
a/exec/java-exec/src/test/java/org/apache/drill/exec/store/dfs/TestFileSelection.java
 
b/exec/java-exec/src/test/java/org/apache/drill/exec/store/dfs/TestFileSelection.java
index f74bc55..ec3f202 100644
--- 
a/exec/java-exec/src/test/java/org/apache/drill/exec/store/dfs/TestFileSelection.java
+++ 
b/exec/java-exec/src/test/java/org/apache/drill/exec/store/dfs/TestFileSelection.java
@@ -17,6 +17,7 @@
  */
 package org.apache.drill.exec.store.dfs;
 
+import static junit.framework.TestCase.fail;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
@@ -26,6 +27,7 @@ import java.util.List;
 import com.google.common.collect.ImmutableList;
 import org.apache.drill.test.BaseTestQuery;
 import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.Path;
 import org.junit.Test;
 
 public class TestFileSelection extends BaseTestQuery {
@@ -59,4 +61,55 @@ public class TestFileSelection extends BaseTestQuery {
       throw ex;
     }
   }
+
+  @Test
+  public void testBackPathBad() throws Exception {
+    final String[][] badPaths =
+        {
+            {"/tmp", "../../bad"},   //  goes beyond root and outside parent; 
resolves to /../bad
+            {"/tmp", "../etc/bad"},  //  goes outside parent; resolves to 
/etc/bad
+            {"", "/bad"},            //  empty parent
+            {"/", ""},               //  empty path
+        } ;
+
+
+    for (int i = 0; i < badPaths.length; i++) {
+      boolean isPathGood = true;
+      try {
+        String parent = badPaths[i][0];
+        String subPath = FileSelection.removeLeadingSlash(badPaths[i][1]);
+        String path = new Path(parent, subPath).toString();
+        FileSelection.checkBackPaths(parent, path, subPath);
+      } catch (IllegalArgumentException e) {
+        isPathGood = false;
+      }
+      if (isPathGood) {
+        fail("Failed to catch invalid file selection paths.");
+      }
+    }
+  }
+
+  @Test
+  public void testBackPathGood() throws Exception {
+    final String[][] goodPaths =
+        {
+            {"/tmp", "../tmp/good"},
+            {"/", "/tmp/good/../../good"},
+            {"/", "etc/tmp/../../good"},   //  no leading slash in path
+            {"/", "../good"},              //  resolves to /../good which is OK
+            {"/", "/good"}
+        } ;
+
+    for (int i = 0; i < goodPaths.length; i++) {
+      try {
+        String parent = goodPaths[i][0];
+        String subPath = FileSelection.removeLeadingSlash(goodPaths[i][1]);
+        String path = new Path(parent, subPath).toString();
+        FileSelection.checkBackPaths(parent, path, subPath);
+      } catch (IllegalArgumentException e) {
+        fail("Valid path not allowed by selection path validation.");
+      }
+    }
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/53d71dfd/exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java
----------------------------------------------------------------------
diff --git 
a/exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java 
b/exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java
index a4d62d4..6514ac8 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java
@@ -480,7 +480,7 @@ public class ClusterFixture extends BaseFixture implements 
AutoCloseable {
     @SuppressWarnings("resource")
     final FileSystemPlugin plugin = (FileSystemPlugin) 
pluginRegistry.getPlugin(pluginName);
     final FileSystemConfig pluginConfig = (FileSystemConfig) 
plugin.getConfig();
-    final WorkspaceConfig newTmpWSConfig = new WorkspaceConfig(path, true, 
defaultFormat);
+    final WorkspaceConfig newTmpWSConfig = new WorkspaceConfig(path, true, 
defaultFormat, false);
 
     pluginConfig.workspaces.remove(schemaName);
     pluginConfig.workspaces.put(schemaName, newTmpWSConfig);

Reply via email to