From commits-return-20184-archive-asf-public=cust-asf.ponee.io@phoenix.apache.org Fri Mar 30 10:01:45 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id C2400180647 for ; Fri, 30 Mar 2018 10:01:44 +0200 (CEST) Received: (qmail 73234 invoked by uid 500); 30 Mar 2018 08:01:43 -0000 Mailing-List: contact commits-help@phoenix.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@phoenix.apache.org Delivered-To: mailing list commits@phoenix.apache.org Received: (qmail 73212 invoked by uid 99); 30 Mar 2018 08:01:43 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 30 Mar 2018 08:01:43 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 6E4AADFF80; Fri, 30 Mar 2018 08:01:43 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: pboado@apache.org To: commits@phoenix.apache.org Date: Fri, 30 Mar 2018 08:01:43 -0000 Message-Id: <79a5bdefbb164ef8a085753436016356@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [1/4] phoenix git commit: PHOENIX-4675 Better parsing around the allowed UDF jar directory configuration [Forced Update!] 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 Authored: Wed Mar 28 00:06:45 2018 +0100 Committer: Pedro Boado 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) {