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:19 UTC

[1/7] phoenix git commit: PHOENIX-4675 Better parsing around the allowed UDF jar directory configuration

Repository: phoenix
Updated Branches:
  refs/heads/4.x-HBase-0.98 dbed33cad -> 5f805e615
  refs/heads/4.x-HBase-1.1 c6b98ce85 -> 372cd525a
  refs/heads/4.x-HBase-1.2 8da901e1c -> 9c7ad77d9
  refs/heads/4.x-HBase-1.3 d2b290ce0 -> f6d3d2da0
  refs/heads/5.x-HBase-2.0 43455b41d -> 4ca23644a
  refs/heads/master 96f2c8b3b -> d04db9966


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/d04db996
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/d04db996
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/d04db996

Branch: refs/heads/master
Commit: d04db9966ada85998a8830cec86c4134528e5ea3
Parents: 96f2c8b
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:22 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/d04db996/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/d04db996/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) {


[5/7] phoenix git commit: PHOENIX-4675 Better parsing around the allowed UDF jar directory configuration

Posted by el...@apache.org.
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/372cd525
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/372cd525
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/372cd525

Branch: refs/heads/4.x-HBase-1.1
Commit: 372cd525a92fe8e1a644ef08ebbad60d932b0262
Parents: cfd548c
Author: Josh Elser <el...@apache.org>
Authored: Tue Mar 27 19:06:45 2018 -0400
Committer: Josh Elser <el...@apache.org>
Committed: Wed Mar 28 09:31:34 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/372cd525/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/372cd525/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) {


[3/7] phoenix git commit: PHOENIX-4675 Better parsing around the allowed UDF jar directory configuration

Posted by el...@apache.org.
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/9c7ad77d
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/9c7ad77d
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/9c7ad77d

Branch: refs/heads/4.x-HBase-1.2
Commit: 9c7ad77d98dba5a94df53bf93416e2fab3733b31
Parents: 8da901e
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:51:10 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/9c7ad77d/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/9c7ad77d/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) {


[6/7] phoenix git commit: PHOENIX-4675 Better parsing around the allowed UDF jar directory configuration

Posted by el...@apache.org.
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/5f805e61
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/5f805e61
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/5f805e61

Branch: refs/heads/4.x-HBase-0.98
Commit: 5f805e6159414e4916ef846630efbebbcf363180
Parents: dbed33c
Author: Josh Elser <el...@apache.org>
Authored: Tue Mar 27 19:06:45 2018 -0400
Committer: Josh Elser <el...@apache.org>
Committed: Wed Mar 28 09:52:58 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/5f805e61/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/5f805e61/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) {


[7/7] phoenix git commit: PHOENIX-4675 Better parsing around the allowed UDF jar directory configuration

Posted by el...@apache.org.
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/4ca23644
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/4ca23644
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/4ca23644

Branch: refs/heads/5.x-HBase-2.0
Commit: 4ca23644a7237a8a17f241db229f0edecf3139d5
Parents: 43455b4
Author: Josh Elser <el...@apache.org>
Authored: Tue Mar 27 19:06:45 2018 -0400
Committer: Josh Elser <el...@apache.org>
Committed: Wed Mar 28 10:02: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/4ca23644/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/4ca23644/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) {


[4/7] phoenix git commit: PHOENIX-4231 Support restriction of remote UDF load sources

Posted by el...@apache.org.
PHOENIX-4231 Support restriction of remote UDF load sources

Signed-off-by: Andrew Purtell <ap...@apache.org>


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

Branch: refs/heads/4.x-HBase-1.1
Commit: cfd548c145423588509e2c6df9bfac2e27549ff3
Parents: c6b98ce
Author: aertoria <ew...@apache.org>
Authored: Tue Feb 27 15:02:07 2018 -0800
Committer: Josh Elser <el...@apache.org>
Committed: Wed Mar 28 09:31:30 2018 -0400

----------------------------------------------------------------------
 .../phoenix/end2end/UserDefinedFunctionsIT.java | 127 +++++++++++++++++--
 .../expression/function/UDFExpression.java      |  20 +--
 2 files changed, 120 insertions(+), 27 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/cfd548c1/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 f58f750..943119d 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
@@ -27,15 +27,12 @@ import static org.apache.phoenix.util.PhoenixRuntime.PHOENIX_TEST_DRIVER_URL_PAR
 import static org.apache.phoenix.util.TestUtil.JOIN_ITEM_TABLE_FULL_NAME;
 import static org.apache.phoenix.util.TestUtil.JOIN_SUPPLIER_TABLE_FULL_NAME;
 import static org.apache.phoenix.util.TestUtil.LOCALHOST;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.*;
 
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
+import java.io.IOException;
 import java.sql.Connection;
 import java.sql.DriverManager;
 import java.sql.PreparedStatement;
@@ -53,6 +50,10 @@ import java.util.jar.Manifest;
 import javax.tools.JavaCompiler;
 import javax.tools.ToolProvider;
 
+import org.apache.commons.lang.exception.ExceptionUtils;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.FileUtil;
+import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseConfiguration;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
@@ -66,13 +67,15 @@ import org.apache.phoenix.util.PhoenixRuntime;
 import org.apache.phoenix.util.QueryUtil;
 import org.apache.phoenix.util.ReadOnlyProps;
 import org.junit.After;
+import org.junit.Before;
 import org.junit.BeforeClass;
+import org.junit.Rule;
 import org.junit.Test;
 
 import com.google.common.collect.Maps;
+import org.junit.rules.TestName;
 
 public class UserDefinedFunctionsIT extends BaseOwnClusterIT {
-    
     protected static final String TENANT_ID = "ZZTop";
     private static String url;
     private static PhoenixTestDriver driver;
@@ -190,10 +193,36 @@ public class UserDefinedFunctionsIT extends BaseOwnClusterIT {
     private static String GETY_CLASSNAME_PROGRAM = getProgram(GETY_CLASSNAME, GETY_EVALUATE_METHOD, "return PInteger.INSTANCE;");
     private static Properties EMPTY_PROPS = new Properties();
     
+    @Rule
+    public TestName name = new TestName();
 
     @Override
     @After
-    public void cleanUpAfterTest() throws Exception {}
+    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'");
+        stmt.execute("delete jar '"+ util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar4.jar'");
+        stmt.execute("delete jar '"+ util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar5.jar'");
+        stmt.execute("delete jar '"+ util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar6.jar'");
+        stmt.execute("delete jar '"+ util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar7.jar'");
+        stmt.execute("delete jar '"+ util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar8.jar'");
+        conn.commit();
+        conn.close();
+    }
+
+    @Before
+    public void doSetupBeforeTest() throws Exception {
+        compileTestClass(MY_REVERSE_CLASS_NAME, MY_REVERSE_PROGRAM, 1);
+        compileTestClass(MY_SUM_CLASS_NAME, MY_SUM_PROGRAM, 2);
+        compileTestClass(MY_ARRAY_INDEX_CLASS_NAME, MY_ARRAY_INDEX_PROGRAM, 3);
+        compileTestClass(MY_ARRAY_INDEX_CLASS_NAME, MY_ARRAY_INDEX_PROGRAM, 4);
+        compileTestClass(GETX_CLASSNAME, GETX_CLASSNAME_PROGRAM, 5);
+        compileTestClass(GETY_CLASSNAME, GETY_CLASSNAME_PROGRAM, 6);
+    }
 
     private static String getProgram(String className, String evaluateMethod, String returnType) {
         return new StringBuffer()
@@ -263,12 +292,6 @@ public class UserDefinedFunctionsIT extends BaseOwnClusterIT {
         props.put(QueryServices.ALLOW_USER_DEFINED_FUNCTIONS_ATTRIB, "true");
         props.put(QueryServices.DYNAMIC_JARS_DIR_KEY,string+"/hbase/tmpjars/");
         driver = initAndRegisterTestDriver(url, new ReadOnlyProps(props.entrySet().iterator()));
-        compileTestClass(MY_REVERSE_CLASS_NAME, MY_REVERSE_PROGRAM, 1);
-        compileTestClass(MY_SUM_CLASS_NAME, MY_SUM_PROGRAM, 2);
-        compileTestClass(MY_ARRAY_INDEX_CLASS_NAME, MY_ARRAY_INDEX_PROGRAM, 3);
-        compileTestClass(MY_ARRAY_INDEX_CLASS_NAME, MY_ARRAY_INDEX_PROGRAM, 4);
-        compileTestClass(GETX_CLASSNAME, GETX_CLASSNAME_PROGRAM, 5);
-        compileTestClass(GETY_CLASSNAME, GETY_CLASSNAME_PROGRAM, 6);
     }
     
     @Test
@@ -283,6 +306,8 @@ public class UserDefinedFunctionsIT extends BaseOwnClusterIT {
         assertTrue(rs.next());
         assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar3.jar", rs.getString("jar_location"));
         assertTrue(rs.next());
+        assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar4.jar", rs.getString("jar_location"));
+        assertTrue(rs.next());
         assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar5.jar", rs.getString("jar_location"));
         assertTrue(rs.next());
         assertEquals(util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY)+"/"+"myjar6.jar", rs.getString("jar_location"));
@@ -1052,6 +1077,7 @@ public class UserDefinedFunctionsIT extends BaseOwnClusterIT {
 
     /**
      * Compiles the test class with bogus code into a .class file.
+     * Upon finish, the bogus jar will be left at dynamic.jar.dir location
      */
     private static void compileTestClass(String className, String program, int counter) throws Exception {
         String javaFileName = className+".java";
@@ -1112,4 +1138,79 @@ public class UserDefinedFunctionsIT extends BaseOwnClusterIT {
         }
     }
 
+    /**
+     * Test creating functions using hbase.dynamic.jars.dir
+     * @throws Exception
+     */
+    @Test
+    public void testCreateFunctionDynamicJarDir() throws Exception {
+        Connection conn = driver.connect(url, EMPTY_PROPS);
+        String tableName = "table" + name.getMethodName();
+
+        conn.createStatement().execute("create table " + tableName + "(tenant_id varchar not null, k integer not null, "
+                + "firstname varchar, lastname varchar constraint pk primary key(tenant_id,k)) MULTI_TENANT=true");
+        String tenantId = "tenId" + name.getMethodName();
+        Connection tenantConn = driver.connect(url + ";" + PhoenixRuntime.TENANT_ID_ATTRIB + "=" + tenantId, EMPTY_PROPS);
+        Statement stmtTenant = tenantConn.createStatement();
+        stmtTenant.execute("upsert into " + tableName + " values(1,'foo','jock')");
+        tenantConn.commit();
+
+        compileTestClass(MY_REVERSE_CLASS_NAME, MY_REVERSE_PROGRAM, 7);
+
+        String sql="create function myfunction(VARCHAR) returns VARCHAR as 'org.apache.phoenix.end2end." + MY_REVERSE_CLASS_NAME
+                + "' using jar '" + util.getConfiguration().get(QueryServices.DYNAMIC_JARS_DIR_KEY).toString() + "'";
+        stmtTenant.execute(sql);
+        ResultSet rs = stmtTenant.executeQuery("select myfunction(firstname) from " + tableName);
+        assertTrue(rs.next());
+        assertEquals("oof",rs.getString(1));
+    }
+
+
+    /**
+     * Test creating functions using dir otherthan hbase.dynamic.jars.dir
+     * @throws Exception
+     */
+    @Test
+    public void testCreateFunctionNonDynamicJarDir() throws Exception {
+        Connection conn = driver.connect(url, EMPTY_PROPS);
+        String tableName = "table" + name.getMethodName();
+
+        conn.createStatement().execute("create table " + tableName + "(tenant_id varchar not null, k integer not null, "
+                + "firstname varchar, lastname varchar constraint pk primary key(tenant_id,k)) MULTI_TENANT=true");
+        String tenantId = "tenId" + name.getMethodName();
+        Connection tenantConn = driver.connect(url + ";" + PhoenixRuntime.TENANT_ID_ATTRIB + "=" + tenantId, EMPTY_PROPS);
+        Statement stmtTenant = tenantConn.createStatement();
+        tenantConn.commit();
+
+        compileTestClass(MY_REVERSE_CLASS_NAME, MY_REVERSE_PROGRAM, 8);
+        Path destJarPathOnHDFS = copyJarsFromDynamicJarsDirToDummyHDFSDir("myjar8.jar");
+
+        try {
+            String sql =
+                    "create function myfunction(VARCHAR) returns VARCHAR as 'org.apache.phoenix.end2end."
+                            + MY_REVERSE_CLASS_NAME + "' using jar '" + destJarPathOnHDFS.toString()
+                            + "'";
+            stmtTenant.execute(sql);
+            ResultSet rs = stmtTenant.executeQuery("select myfunction(firstname) from " + tableName);
+            fail("expecting java.lang.SecurityException");
+        }catch(Exception e){
+            assertTrue(ExceptionUtils.getRootCause(e) instanceof SecurityException);
+        }finally {
+            stmtTenant.execute("drop function myfunction");
+        }
+    }
+    /**
+    * Move the jars from the hbase.dynamic.jars.dir to data test directory
+    * @param jarName
+    * @return The destination jar file path.
+    * @throws IOException
+    */
+    private Path copyJarsFromDynamicJarsDirToDummyHDFSDir(String jarName) throws IOException {
+        Path srcPath = new Path(util.getConfiguration().get(DYNAMIC_JARS_DIR_KEY) + "/" + jarName);
+        FileSystem srcFs = srcPath.getFileSystem(util.getConfiguration());
+        Path destPath = new Path(util.getDataTestDirOnTestFS().toString() + "/" + jarName);
+        FileSystem destFs = destPath.getFileSystem(util.getConfiguration());
+        FileUtil.copy(srcFs, srcPath, destFs, destPath, false, true, util.getConfiguration());
+        return destPath;
+    }
 }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/cfd548c1/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 091f931..c8636fd 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
@@ -185,8 +185,9 @@ public class UDFExpression extends ScalarFunction {
                 parent = path.toString();
             }
         }
-        if (jarPath == null || jarPath.isEmpty() || config.get(DYNAMIC_JARS_DIR_KEY) != null
-                && (parent != null && parent.equals(config.get(DYNAMIC_JARS_DIR_KEY)))) {
+        // 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)))) {
             cl = tenantIdSpecificCls.get(tenantId);
             if (cl == null) {
                 cl = new DynamicClassLoader(config, UDFExpression.class.getClassLoader());
@@ -198,18 +199,9 @@ public class UDFExpression extends ScalarFunction {
             }
             return cl;
         } else {
-            cl = pathSpecificCls.get(jarPath);
-            if (cl == null) {
-                Configuration conf = HBaseConfiguration.create(config);
-                conf.set(DYNAMIC_JARS_DIR_KEY, parent);
-                cl = new DynamicClassLoader(conf, UDFExpression.class.getClassLoader());
-            }
-            // Cache class loader as a weak value, will be GC'ed when no reference left
-            DynamicClassLoader prev = pathSpecificCls.putIfAbsent(jarPath, cl);
-            if (prev != null) {
-                cl = prev;
-            }
-            return cl;
+            //The case jarPath is provided as not part of DYNAMIC_JARS_DIR_KEY
+            //As per PHOENIX-4231, DYNAMIC_JARS_DIR_KEY is the only place where loading a udf jar is allowed
+            throw new SecurityException("Loading jars from " + jarPath + " is not allowed. The only location that is allowed is "+ config.get(DYNAMIC_JARS_DIR_KEY));
         }
     }
     


[2/7] phoenix git commit: PHOENIX-4675 Better parsing around the allowed UDF jar directory configuration

Posted by el...@apache.org.
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) {