[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[ https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16361642#comment-16361642 ] Bridget Bevens commented on DRILL-5964: --- Doc updated to include new parameter: [https://drill.apache.org/docs/plugin-configuration-basics/#list-of-attributes-and-definitions] > Do not allow queries to access paths outside the current workspace root > --- > > Key: DRILL-5964 > URL: https://issues.apache.org/jira/browse/DRILL-5964 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.11.0 >Reporter: Parth Chandra >Assignee: Parth Chandra >Priority: Major > Labels: doc-impacting, ready-to-commit > Fix For: 1.12.0 > > > Workspace definitions in the dfs plugin are intended to provide a convenient > shortcut to long directory paths. However, some users may wish to disallow > access to paths outside the root of the workspace, possibly to prevent > accidental access. Note that this is a convenience option and not a > substitute for permissions on the file system. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[ https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16270606#comment-16270606 ] ASF GitHub Bot commented on DRILL-5964: --- Github user asfgit closed the pull request at: https://github.com/apache/drill/pull/1050 > Do not allow queries to access paths outside the current workspace root > --- > > Key: DRILL-5964 > URL: https://issues.apache.org/jira/browse/DRILL-5964 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.11.0 >Reporter: Parth Chandra >Assignee: Parth Chandra > Labels: doc-impacting, ready-to-commit > Fix For: 1.12.0 > > > Workspace definitions in the dfs plugin are intended to provide a convenient > shortcut to long directory paths. However, some users may wish to disallow > access to paths outside the root of the workspace, possibly to prevent > accidental access. Note that this is a convenience option and not a > substitute for permissions on the file system. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[ https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16270318#comment-16270318 ] ASF GitHub Bot commented on DRILL-5964: --- Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/1050 +1, LGTM. > Do not allow queries to access paths outside the current workspace root > --- > > Key: DRILL-5964 > URL: https://issues.apache.org/jira/browse/DRILL-5964 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.11.0 >Reporter: Parth Chandra >Assignee: Parth Chandra > Labels: doc-impacting, ready-to-commit > Fix For: 1.12.0 > > > Workspace definitions in the dfs plugin are intended to provide a convenient > shortcut to long directory paths. However, some users may wish to disallow > access to paths outside the root of the workspace, possibly to prevent > accidental access. Note that this is a convenience option and not a > substitute for permissions on the file system. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[ https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16269961#comment-16269961 ] ASF GitHub Bot commented on DRILL-5964: --- Github user parthchandra commented on the issue: https://github.com/apache/drill/pull/1050 Changed the type of allowAccessOutsideWorkspace to boolean. This changes behavior. All existing dfs workspaces will also disallow access outside the workspace. Ideally, we should deprecate this parameter in a future release, but for the moment it allows existing users to access paths outside the workspace if they want to. > Do not allow queries to access paths outside the current workspace root > --- > > Key: DRILL-5964 > URL: https://issues.apache.org/jira/browse/DRILL-5964 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.11.0 >Reporter: Parth Chandra >Assignee: Parth Chandra > Labels: doc-impacting > > Workspace definitions in the dfs plugin are intended to provide a convenient > shortcut to long directory paths. However, some users may wish to disallow > access to paths outside the root of the workspace, possibly to prevent > accidental access. Note that this is a convenience option and not a > substitute for permissions on the file system. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[
https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16268450#comment-16268450
]
ASF GitHub Bot commented on DRILL-5964:
---
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r153436976
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceConfig.java
---
@@ -30,18 +30,24 @@
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; // allow access
outside the workspace by default. This
--- End diff --
1. Can we adjust the variable to be false by default, i.e. rename it to
`disallowAccessOutsideWorkspace`? Thus we'll be able to use primitive, right?
2. In the below code you always set `this.allowAccessOutsideWorkspace =
true;`, block with `this.allowAccessOutsideWorkspace =
allowAccessOutsideWorkspace != null ? allowAccessOutsideWorkspace : false ;` is
commented. I guess this is a mistake.
> Do not allow queries to access paths outside the current workspace root
> ---
>
> Key: DRILL-5964
> URL: https://issues.apache.org/jira/browse/DRILL-5964
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.11.0
>Reporter: Parth Chandra
>Assignee: Parth Chandra
> Labels: doc-impacting
>
> Workspace definitions in the dfs plugin are intended to provide a convenient
> shortcut to long directory paths. However, some users may wish to disallow
> access to paths outside the root of the workspace, possibly to prevent
> accidental access. Note that this is a convenience option and not a
> substitute for permissions on the file system.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[
https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16268044#comment-16268044
]
ASF GitHub Bot commented on DRILL-5964:
---
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r153388800
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceConfig.java
---
@@ -30,18 +30,24 @@
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; // allow access
outside the workspace by default. This
--- End diff --
Yes it would, I believe. But we want the value to be `true` for backward
compatibility. (This also addresses your next comment). So we need to know if
the value is missing. Can only do that with a non primitive AFAIK.
> Do not allow queries to access paths outside the current workspace root
> ---
>
> Key: DRILL-5964
> URL: https://issues.apache.org/jira/browse/DRILL-5964
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.11.0
>Reporter: Parth Chandra
>Assignee: Parth Chandra
> Labels: doc-impacting
>
> Workspace definitions in the dfs plugin are intended to provide a convenient
> shortcut to long directory paths. However, some users may wish to disallow
> access to paths outside the root of the workspace, possibly to prevent
> accidental access. Note that this is a convenience option and not a
> substitute for permissions on the file system.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[
https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16267672#comment-16267672
]
ASF GitHub Bot commented on DRILL-5964:
---
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r153336979
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceConfig.java
---
@@ -30,18 +30,24 @@
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; // allow access
outside the workspace by default. This
--- End diff --
As far I remember `@JsonProperty("allowAccessOutsideWorkspace") boolean
allowAccessOutsideWorkspace` will set false by default if value is not present
during deserialization. Could you please check?
> Do not allow queries to access paths outside the current workspace root
> ---
>
> Key: DRILL-5964
> URL: https://issues.apache.org/jira/browse/DRILL-5964
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.11.0
>Reporter: Parth Chandra
>Assignee: Parth Chandra
> Labels: doc-impacting
>
> Workspace definitions in the dfs plugin are intended to provide a convenient
> shortcut to long directory paths. However, some users may wish to disallow
> access to paths outside the root of the workspace, possibly to prevent
> accidental access. Note that this is a convenience option and not a
> substitute for permissions on the file system.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[
https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16267671#comment-16267671
]
ASF GitHub Bot commented on DRILL-5964:
---
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r153337275
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceConfig.java
---
@@ -30,18 +30,25 @@
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; // allow access
outside the workspace by default. This
+ // field is a Boolean
(not boolean) so that we can
+ // assign a default
value if it is not defined in a
+ // storage plugin
config
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 !=
null ? allowAccessOutsideWorkspace : false ;
+this.allowAccessOutsideWorkspace = true;
--- End diff --
It seems we should not always set true...
> Do not allow queries to access paths outside the current workspace root
> ---
>
> Key: DRILL-5964
> URL: https://issues.apache.org/jira/browse/DRILL-5964
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.11.0
>Reporter: Parth Chandra
>Assignee: Parth Chandra
> Labels: doc-impacting
>
> Workspace definitions in the dfs plugin are intended to provide a convenient
> shortcut to long directory paths. However, some users may wish to disallow
> access to paths outside the root of the workspace, possibly to prevent
> accidental access. Note that this is a convenience option and not a
> substitute for permissions on the file system.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[
https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16267504#comment-16267504
]
ASF GitHub Bot commented on DRILL-5964:
---
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152862722
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
---
@@ -359,15 +363,30 @@ private static Path handleWildCard(final String root)
{
}
}
- 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 {
return path;
}
}
+ // 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
+ public static boolean checkBackPaths(String parent, String combinedPath,
String subpath) {
+Preconditions.checkArgument(!parent.isEmpty());
+Preconditions.checkArgument(!combinedPath.isEmpty());
--- End diff --
Done
> Do not allow queries to access paths outside the current workspace root
> ---
>
> Key: DRILL-5964
> URL: https://issues.apache.org/jira/browse/DRILL-5964
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.11.0
>Reporter: Parth Chandra
>Assignee: Parth Chandra
> Labels: doc-impacting
>
> Workspace definitions in the dfs plugin are intended to provide a convenient
> shortcut to long directory paths. However, some users may wish to disallow
> access to paths outside the root of the workspace, possibly to prevent
> accidental access. Note that this is a convenience option and not a
> substitute for permissions on the file system.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[
https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16267508#comment-16267508
]
ASF GitHub Bot commented on DRILL-5964:
---
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r153318818
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/store/dfs/TestFileSelection.java
---
@@ -59,4 +61,53 @@ public void testEmptyFolderThrowsTableNotFound() throws
Exception {
throw ex;
}
}
+
+ @Test
+ public void testBackPath() throws Exception {
--- End diff --
Done
> Do not allow queries to access paths outside the current workspace root
> ---
>
> Key: DRILL-5964
> URL: https://issues.apache.org/jira/browse/DRILL-5964
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.11.0
>Reporter: Parth Chandra
>Assignee: Parth Chandra
> Labels: doc-impacting
>
> Workspace definitions in the dfs plugin are intended to provide a convenient
> shortcut to long directory paths. However, some users may wish to disallow
> access to paths outside the root of the workspace, possibly to prevent
> accidental access. Note that this is a convenience option and not a
> substitute for permissions on the file system.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[
https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16267505#comment-16267505
]
ASF GitHub Bot commented on DRILL-5964:
---
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152862665
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
---
@@ -359,15 +363,30 @@ private static Path handleWildCard(final String root)
{
}
}
- 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 {
return path;
}
}
+ // Check if the path is a valid sub path under the parent after removing
backpaths. Throw an exception if
--- End diff --
Done
> Do not allow queries to access paths outside the current workspace root
> ---
>
> Key: DRILL-5964
> URL: https://issues.apache.org/jira/browse/DRILL-5964
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.11.0
>Reporter: Parth Chandra
>Assignee: Parth Chandra
> Labels: doc-impacting
>
> Workspace definitions in the dfs plugin are intended to provide a convenient
> shortcut to long directory paths. However, some users may wish to disallow
> access to paths outside the root of the workspace, possibly to prevent
> accidental access. Note that this is a convenience option and not a
> substitute for permissions on the file system.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[
https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16267507#comment-16267507
]
ASF GitHub Bot commented on DRILL-5964:
---
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152862954
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceConfig.java
---
@@ -30,18 +30,24 @@
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; // allow access
outside the workspace by default. This
--- End diff --
I need to check if the value is not present (i.e. null). That will be the
case with all storage plugin configurations that have already been created.
> Do not allow queries to access paths outside the current workspace root
> ---
>
> Key: DRILL-5964
> URL: https://issues.apache.org/jira/browse/DRILL-5964
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.11.0
>Reporter: Parth Chandra
>Assignee: Parth Chandra
> Labels: doc-impacting
>
> Workspace definitions in the dfs plugin are intended to provide a convenient
> shortcut to long directory paths. However, some users may wish to disallow
> access to paths outside the root of the workspace, possibly to prevent
> accidental access. Note that this is a convenience option and not a
> substitute for permissions on the file system.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[
https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16267503#comment-16267503
]
ASF GitHub Bot commented on DRILL-5964:
---
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152862636
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
---
@@ -252,11 +252,15 @@ private static String buildPath(final String[] path,
final int folderIndex) {
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(parent, combined.toString(), path);
--- End diff --
Done
> Do not allow queries to access paths outside the current workspace root
> ---
>
> Key: DRILL-5964
> URL: https://issues.apache.org/jira/browse/DRILL-5964
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.11.0
>Reporter: Parth Chandra
>Assignee: Parth Chandra
> Labels: doc-impacting
>
> Workspace definitions in the dfs plugin are intended to provide a convenient
> shortcut to long directory paths. However, some users may wish to disallow
> access to paths outside the root of the workspace, possibly to prevent
> accidental access. Note that this is a convenience option and not a
> substitute for permissions on the file system.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[
https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16267506#comment-16267506
]
ASF GitHub Bot commented on DRILL-5964:
---
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152862693
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
---
@@ -359,15 +363,30 @@ private static Path handleWildCard(final String root)
{
}
}
- 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 {
return path;
}
}
+ // 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
+ public static boolean checkBackPaths(String parent, String combinedPath,
String subpath) {
--- End diff --
Done
> Do not allow queries to access paths outside the current workspace root
> ---
>
> Key: DRILL-5964
> URL: https://issues.apache.org/jira/browse/DRILL-5964
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.11.0
>Reporter: Parth Chandra
>Assignee: Parth Chandra
> Labels: doc-impacting
>
> Workspace definitions in the dfs plugin are intended to provide a convenient
> shortcut to long directory paths. However, some users may wish to disallow
> access to paths outside the root of the workspace, possibly to prevent
> accidental access. Note that this is a convenience option and not a
> substitute for permissions on the file system.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[
https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16264194#comment-16264194
]
ASF GitHub Bot commented on DRILL-5964:
---
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152773573
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceConfig.java
---
@@ -30,18 +30,24 @@
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; // allow access
outside the workspace by default. This
--- End diff --
Consider using primitive, we don't want unnecessary boxing / unboxing.
> Do not allow queries to access paths outside the current workspace root
> ---
>
> Key: DRILL-5964
> URL: https://issues.apache.org/jira/browse/DRILL-5964
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.11.0
>Reporter: Parth Chandra
> Labels: doc-impacting
>
> Workspace definitions in the dfs plugin are intended to provide a convenient
> shortcut to long directory paths. However, some users may wish to disallow
> access to paths outside the root of the workspace, possibly to prevent
> accidental access. Note that this is a convenience option and not a
> substitute for permissions on the file system.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[
https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16264193#comment-16264193
]
ASF GitHub Bot commented on DRILL-5964:
---
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152773919
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
---
@@ -252,11 +252,15 @@ private static String buildPath(final String[] path,
final int folderIndex) {
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(parent, combined.toString(), path);
--- End diff --
I usually void using `toString` for `Path`, consider using
`combined.toUri().getPath()`.
> Do not allow queries to access paths outside the current workspace root
> ---
>
> Key: DRILL-5964
> URL: https://issues.apache.org/jira/browse/DRILL-5964
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.11.0
>Reporter: Parth Chandra
> Labels: doc-impacting
>
> Workspace definitions in the dfs plugin are intended to provide a convenient
> shortcut to long directory paths. However, some users may wish to disallow
> access to paths outside the root of the workspace, possibly to prevent
> accidental access. Note that this is a convenience option and not a
> substitute for permissions on the file system.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[
https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16264192#comment-16264192
]
ASF GitHub Bot commented on DRILL-5964:
---
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152774087
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
---
@@ -359,15 +363,30 @@ private static Path handleWildCard(final String root)
{
}
}
- 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 {
return path;
}
}
+ // 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
+ public static boolean checkBackPaths(String parent, String combinedPath,
String subpath) {
+Preconditions.checkArgument(!parent.isEmpty());
+Preconditions.checkArgument(!combinedPath.isEmpty());
--- End diff --
Please add message for pre-conditions so error message will be more clear.
> Do not allow queries to access paths outside the current workspace root
> ---
>
> Key: DRILL-5964
> URL: https://issues.apache.org/jira/browse/DRILL-5964
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.11.0
>Reporter: Parth Chandra
> Labels: doc-impacting
>
> Workspace definitions in the dfs plugin are intended to provide a convenient
> shortcut to long directory paths. However, some users may wish to disallow
> access to paths outside the root of the workspace, possibly to prevent
> accidental access. Note that this is a convenience option and not a
> substitute for permissions on the file system.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[
https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16264195#comment-16264195
]
ASF GitHub Bot commented on DRILL-5964:
---
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152772486
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
---
@@ -359,15 +363,30 @@ private static Path handleWildCard(final String root)
{
}
}
- 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 {
return path;
}
}
+ // 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
+ public static boolean checkBackPaths(String parent, String combinedPath,
String subpath) {
--- End diff --
This method can return void, since it never will return false. I see you
use this during testing but it can be tested with void type as well.
> Do not allow queries to access paths outside the current workspace root
> ---
>
> Key: DRILL-5964
> URL: https://issues.apache.org/jira/browse/DRILL-5964
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.11.0
>Reporter: Parth Chandra
> Labels: doc-impacting
>
> Workspace definitions in the dfs plugin are intended to provide a convenient
> shortcut to long directory paths. However, some users may wish to disallow
> access to paths outside the root of the workspace, possibly to prevent
> accidental access. Note that this is a convenience option and not a
> substitute for permissions on the file system.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[
https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16264191#comment-16264191
]
ASF GitHub Bot commented on DRILL-5964:
---
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152772350
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
---
@@ -359,15 +363,30 @@ private static Path handleWildCard(final String root)
{
}
}
- 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 {
return path;
}
}
+ // Check if the path is a valid sub path under the parent after removing
backpaths. Throw an exception if
--- End diff --
Please use java doc.
> Do not allow queries to access paths outside the current workspace root
> ---
>
> Key: DRILL-5964
> URL: https://issues.apache.org/jira/browse/DRILL-5964
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.11.0
>Reporter: Parth Chandra
> Labels: doc-impacting
>
> Workspace definitions in the dfs plugin are intended to provide a convenient
> shortcut to long directory paths. However, some users may wish to disallow
> access to paths outside the root of the workspace, possibly to prevent
> accidental access. Note that this is a convenience option and not a
> substitute for permissions on the file system.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[
https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16264196#comment-16264196
]
ASF GitHub Bot commented on DRILL-5964:
---
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152774820
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/store/dfs/TestFileSelection.java
---
@@ -59,4 +61,53 @@ public void testEmptyFolderThrowsTableNotFound() throws
Exception {
throw ex;
}
}
+
+ @Test
+ public void testBackPath() throws Exception {
--- End diff --
Please split into two tests.
> Do not allow queries to access paths outside the current workspace root
> ---
>
> Key: DRILL-5964
> URL: https://issues.apache.org/jira/browse/DRILL-5964
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.11.0
>Reporter: Parth Chandra
> Labels: doc-impacting
>
> Workspace definitions in the dfs plugin are intended to provide a convenient
> shortcut to long directory paths. However, some users may wish to disallow
> access to paths outside the root of the workspace, possibly to prevent
> accidental access. Note that this is a convenience option and not a
> substitute for permissions on the file system.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5964) Do not allow queries to access paths outside the current workspace root
[ https://issues.apache.org/jira/browse/DRILL-5964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16263734#comment-16263734 ] Parth Chandra commented on DRILL-5964: -- Added check to prevent users from accessing a path outside the current workspace. Documentation notes: bq. For backward compatibility this introduces a new parameter 'allowAccessOutsideWorkspace' in the dfs storage plugin configuration that allows the user to override the check and allow access outside the workspace. The default value for the parameter is false. Any existing storage plugin configurations which do not have the parameter specified will no longer be able to access paths outside the workspace. > Do not allow queries to access paths outside the current workspace root > --- > > Key: DRILL-5964 > URL: https://issues.apache.org/jira/browse/DRILL-5964 > Project: Apache Drill > Issue Type: Improvement >Reporter: Parth Chandra > Labels: doc-impacting > > Workspace definitions in the dfs plugin are intended to provide a convenient > shortcut to long directory paths. However, some users may wish to disallow > access to paths outside the root of the workspace, possibly to prevent > accidental access. Note that this is a convenience option and not a > substitute for permissions on the file system. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
