You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by el...@apache.org on 2018/03/28 14:16:20 UTC
[2/7] phoenix git commit: PHOENIX-4675 Better parsing around the
allowed UDF jar directory configuration
PHOENIX-4675 Better parsing around the allowed UDF jar directory configuration
The parsing/validation logic on the allowed path for UDF jar loading was lacking
in that it didn't correctly handle a trailing slash.
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/f6d3d2da
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/f6d3d2da
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/f6d3d2da
Branch: refs/heads/4.x-HBase-1.3
Commit: f6d3d2da0b87ab0319a9a6aa35ce5ccec87e78b2
Parents: d2b290c
Author: Josh Elser <el...@apache.org>
Authored: Tue Mar 27 19:06:45 2018 -0400
Committer: Josh Elser <el...@apache.org>
Committed: Tue Mar 27 20:21:38 2018 -0400
----------------------------------------------------------------------
.../phoenix/end2end/UserDefinedFunctionsIT.java | 42 ++++++++++----------
.../expression/function/UDFExpression.java | 21 ++++++----
2 files changed, 35 insertions(+), 28 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/phoenix/blob/f6d3d2da/phoenix-core/src/it/java/org/apache/phoenix/end2end/UserDefinedFunctionsIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/UserDefinedFunctionsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/UserDefinedFunctionsIT.java
index 943119d..ebb2462 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/UserDefinedFunctionsIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/UserDefinedFunctionsIT.java
@@ -201,7 +201,6 @@ public class UserDefinedFunctionsIT extends BaseOwnClusterIT {
public void cleanUpAfterTest() throws Exception {
Connection conn = driver.connect(url, EMPTY_PROPS);
Statement stmt = conn.createStatement();
- ResultSet rs = stmt.executeQuery("list jars");
stmt.execute("delete jar '"+ util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar1.jar'");
stmt.execute("delete jar '"+ util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar2.jar'");
stmt.execute("delete jar '"+ util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar3.jar'");
@@ -280,7 +279,8 @@ public class UserDefinedFunctionsIT extends BaseOwnClusterIT {
util.startMiniDFSCluster(1);
util.startMiniZKCluster(1);
String string = util.getConfiguration().get("fs.defaultFS");
- conf.set(DYNAMIC_JARS_DIR_KEY, string+"/hbase/tmpjars");
+ // PHOENIX-4675 setting the trailing slash implicitly tests that we're doing some path normalization
+ conf.set(DYNAMIC_JARS_DIR_KEY, string+"/hbase/tmpjars/");
util.startMiniHBaseCluster(1, 1);
UDFExpression.setConfig(conf);
@@ -297,20 +297,21 @@ public class UserDefinedFunctionsIT extends BaseOwnClusterIT {
@Test
public void testListJars() throws Exception {
Connection conn = driver.connect(url, EMPTY_PROPS);
+ Path jarPath = new Path(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY));
Statement stmt = conn.createStatement();
ResultSet rs = stmt.executeQuery("list jars");
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar1.jar", rs.getString("jar_location"));
+ assertEquals(new Path(jarPath, "myjar1.jar").toString(), rs.getString("jar_location"));
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar2.jar", rs.getString("jar_location"));
+ assertEquals(new Path(jarPath, "myjar2.jar").toString(), rs.getString("jar_location"));
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar3.jar", rs.getString("jar_location"));
+ assertEquals(new Path(jarPath, "myjar3.jar").toString(), rs.getString("jar_location"));
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar4.jar", rs.getString("jar_location"));
+ assertEquals(new Path(jarPath, "myjar4.jar").toString(), rs.getString("jar_location"));
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar5.jar", rs.getString("jar_location"));
+ assertEquals(new Path(jarPath, "myjar5.jar").toString(), rs.getString("jar_location"));
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar6.jar", rs.getString("jar_location"));
+ assertEquals(new Path(jarPath, "myjar6.jar").toString(), rs.getString("jar_location"));
assertFalse(rs.next());
}
@@ -320,30 +321,31 @@ public class UserDefinedFunctionsIT extends BaseOwnClusterIT {
Statement stmt = conn.createStatement();
ResultSet rs = stmt.executeQuery("list jars");
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar1.jar", rs.getString("jar_location"));
+ Path jarPath = new Path(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY));
+ assertEquals(new Path(jarPath, "myjar1.jar").toString(), rs.getString("jar_location"));
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar2.jar", rs.getString("jar_location"));
+ assertEquals(new Path(jarPath, "myjar2.jar").toString(), rs.getString("jar_location"));
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar3.jar", rs.getString("jar_location"));
+ assertEquals(new Path(jarPath, "myjar3.jar").toString(), rs.getString("jar_location"));
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar4.jar", rs.getString("jar_location"));
+ assertEquals(new Path(jarPath, "myjar4.jar").toString(), rs.getString("jar_location"));
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar5.jar", rs.getString("jar_location"));
+ assertEquals(new Path(jarPath, "myjar5.jar").toString(), rs.getString("jar_location"));
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar6.jar", rs.getString("jar_location"));
+ assertEquals(new Path(jarPath, "myjar6.jar").toString(), rs.getString("jar_location"));
assertFalse(rs.next());
- stmt.execute("delete jar '"+ util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar4.jar'");
+ stmt.execute("delete jar '"+ new Path(jarPath, "myjar4.jar").toString() + "'");
rs = stmt.executeQuery("list jars");
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar1.jar", rs.getString("jar_location"));
+ assertEquals(new Path(jarPath, "myjar1.jar").toString(), rs.getString("jar_location"));
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar2.jar", rs.getString("jar_location"));
+ assertEquals(new Path(jarPath, "myjar2.jar").toString(), rs.getString("jar_location"));
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar3.jar", rs.getString("jar_location"));
+ assertEquals(new Path(jarPath, "myjar3.jar").toString(), rs.getString("jar_location"));
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar5.jar", rs.getString("jar_location"));
+ assertEquals(new Path(jarPath, "myjar5.jar").toString(), rs.getString("jar_location"));
assertTrue(rs.next());
- assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar6.jar", rs.getString("jar_location"));
+ assertEquals(new Path(jarPath, "myjar6.jar").toString(), rs.getString("jar_location"));
assertFalse(rs.next());
}
http://git-wip-us.apache.org/repos/asf/phoenix/blob/f6d3d2da/phoenix-core/src/main/java/org/apache/phoenix/expression/function/UDFExpression.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/UDFExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/UDFExpression.java
index c8636fd..80a569a 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/UDFExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/UDFExpression.java
@@ -173,21 +173,18 @@ public class UDFExpression extends ScalarFunction {
public static DynamicClassLoader getClassLoader(final PName tenantId, final String jarPath) {
DynamicClassLoader cl = tenantIdSpecificCls.get(tenantId);
- String parent = null;
+ Path parent = null;
if (cl != null) return cl;
if(jarPath != null && !jarPath.isEmpty()) {
cl = pathSpecificCls.get(jarPath);
if (cl != null) return cl;
- Path path = new Path(jarPath);
- if(jarPath.endsWith(".jar")) {
- parent = path.getParent().toString();
- } else {
- parent = path.toString();
- }
+ parent = getPathForParent(jarPath);
}
+ // Parse the DYNAMIC_JARS_DIR_KEY value as a Path if it's present in the configuration
+ Path allowedDynamicJarsPath = config.get(DYNAMIC_JARS_DIR_KEY) != null ? new Path(config.get(DYNAMIC_JARS_DIR_KEY)) : null;
// The case jarPath is not provided, or it is provided and the jar is inside hbase.dynamic.jars.dir
if (jarPath == null || jarPath.isEmpty()
- || config.get(DYNAMIC_JARS_DIR_KEY) != null && (parent != null && parent.equals(config.get(DYNAMIC_JARS_DIR_KEY)))) {
+ || (allowedDynamicJarsPath != null && parent != null && parent.equals(allowedDynamicJarsPath))) {
cl = tenantIdSpecificCls.get(tenantId);
if (cl == null) {
cl = new DynamicClassLoader(config, UDFExpression.class.getClassLoader());
@@ -204,6 +201,14 @@ public class UDFExpression extends ScalarFunction {
throw new SecurityException("Loading jars from " + jarPath + " is not allowed. The only location that is allowed is "+ config.get(DYNAMIC_JARS_DIR_KEY));
}
}
+
+ public static Path getPathForParent(String jarPath) {
+ Path path = new Path(jarPath);
+ if (jarPath.endsWith(".jar")) {
+ return path.getParent();
+ }
+ return path;
+ }
@VisibleForTesting
public static void setConfig(Configuration conf) {