phoenix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pbo...@apache.org
Subject [1/4] phoenix git commit: PHOENIX-4675 Better parsing around the allowed UDF jar directory configuration [Forced Update!]
Date Fri, 30 Mar 2018 08:01:43 GMT
Repository: phoenix
Updated Branches:
  refs/heads/4.x-cdh5.13 1d0339005 -> ed8a4f28b (forced update)


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

Branch: refs/heads/4.x-cdh5.13
Commit: bd4d15bbd8c562a0a24b72989477cf22cc481ef5
Parents: 881d7aa
Author: Josh Elser <elserj@apache.org>
Authored: Wed Mar 28 00:06:45 2018 +0100
Committer: Pedro Boado <pboado@apache.org>
Committed: Fri Mar 30 08:58:23 2018 +0100

----------------------------------------------------------------------
 .../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/bd4d15bb/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/bd4d15bb/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) {


Mime
View raw message