sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From vs...@apache.org
Subject sentry git commit: SENTRY-2014: incorrect handling of HDFS paths with multiple forward slashes (Vadim Spector, reviewed by Sergio Pena and Arjun Mishra)
Date Tue, 24 Oct 2017 17:00:14 GMT
Repository: sentry
Updated Branches:
  refs/heads/master 1f77657c9 -> 7dbadfe85


SENTRY-2014: incorrect handling of HDFS paths with multiple forward slashes (Vadim Spector,
reviewed by Sergio Pena and Arjun Mishra)


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

Branch: refs/heads/master
Commit: 7dbadfe859ae7caf06fcd3aca6e81d20636515d8
Parents: 1f77657
Author: Vadim Spector <vspec@cloudera.com>
Authored: Tue Oct 24 09:57:13 2017 -0700
Committer: Vadim Spector <vspec@cloudera.com>
Committed: Tue Oct 24 09:57:13 2017 -0700

----------------------------------------------------------------------
 .../sentry/core/common/utils/PathUtils.java     |  9 ++++++
 .../org/apache/sentry/hdfs/PathsUpdate.java     |  9 ++++--
 .../org/apache/sentry/hdfs/TestPathsUpdate.java | 32 +++++++++++++++-----
 .../db/service/persistent/SentryStore.java      |  3 +-
 .../service/thrift/NotificationProcessor.java   |  3 +-
 5 files changed, 44 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/7dbadfe8/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java
----------------------------------------------------------------------
diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java
b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java
index 40c9595..cef8bd7 100644
--- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java
+++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java
@@ -211,4 +211,13 @@ public class PathUtils {
     return uriPath.toUri().toString();
   }
 
+  /**
+   * Split path into elements.
+   * May evolve to do something smarter, e.g. path canonicalization,
+   * but for now simple split on "/" is sufficient.
+   */
+  public static String[] splitPath(String path) {
+    return (path != null) ? path.split("/+") : null;
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/sentry/blob/7dbadfe8/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
index 5ff2dc2..c9ecc40 100644
--- a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
+++ b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
@@ -166,8 +166,13 @@ public class PathsUpdate implements Updateable.Update {
       throw new SentryMalformedPathException("Path part of uri does not seem right, was expecting
a non empty path" +
               ": path = " + uriPath + ", uri=" + uri);
     }
-    // Remove leading slash
-    return uriPath.substring(1);
+    // Reduce multiple consecutive forward slashes to one.
+    // It's probably a rare case, so use indexOf() before expensive regex.
+    if (uriPath.indexOf("//") >= 0) {
+      uriPath = uriPath.replaceAll("//*", "/");
+    }
+    // Remove leading and trailing slashes
+    return StringUtils.strip(uriPath, "/");
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/sentry/blob/7dbadfe8/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java
b/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java
index c1a8a74..6c8ed2b 100644
--- a/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java
+++ b/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java
@@ -42,14 +42,30 @@ public class TestPathsUpdate {
 
   @Test
   public void testPositiveParsePath() throws SentryMalformedPathException {
-    String result = PathsUpdate.parsePath("hdfs://hostname.test.com:8020/path");
-    Assert.assertTrue("Parsed path is unexpected", result.equals("path"));
-
-    result = PathsUpdate.parsePath("hdfs://hostname.test.com/path");
-    Assert.assertTrue("Parsed path is unexpected", result.equals("path"));
-
-    result = PathsUpdate.parsePath("hdfs:///path");
-    Assert.assertTrue("Parsed path is unexpected", result.equals("path"));
+    String urls[] = {
+      "hdfs://hostname.test.com:8020/path1/path2/path3",
+      // double slashes
+      "hdfs://hostname.test.com:8020//path1/path2/path3",
+      "hdfs://hostname.test.com:8020/path1//path2/path3",
+      "hdfs://hostname.test.com:8020/path1/path2//path3",
+      "hdfs://hostname.test.com:8020/path1/path2//path3",
+      "hdfs://hostname.test.com:8020/path1/path2//path3//",
+      // triple slashes
+      "hdfs://hostname.test.com:8020///path1/path2/path3",
+      "hdfs://hostname.test.com:8020/path1///path2/path3",
+      "hdfs://hostname.test.com:8020/path1/path2///path3",
+      "hdfs://hostname.test.com:8020/path1/path2/path3///",
+      // no port
+      "hdfs://hostname.test.com/path1/path2/path3",
+      // no host
+      "hdfs:///path1/path2/path3"
+    };
+    String path = "path1/path2/path3";
+      
+    for (String url : urls) {
+      String result = PathsUpdate.parsePath(url);
+      Assert.assertEquals("Unexpected path in " + url, path, result);
+    }
   }
 
   @Test(expected = SentryMalformedPathException.class)

http://git-wip-us.apache.org/repos/asf/sentry/blob/7dbadfe8/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
index f4d84d2..0cd6e48 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
@@ -49,6 +49,7 @@ import org.apache.sentry.core.common.exception.SentryInvalidInputException;
 import org.apache.sentry.core.common.exception.SentryNoSuchObjectException;
 import org.apache.sentry.core.common.exception.SentrySiteConfigurationException;
 import org.apache.sentry.core.common.exception.SentryUserException;
+import org.apache.sentry.core.common.utils.PathUtils;
 import org.apache.sentry.core.common.utils.SentryConstants;
 import org.apache.sentry.core.model.db.AccessConstants;
 import org.apache.sentry.core.model.db.DBModelAuthorizable.AuthorizableType;
@@ -2777,7 +2778,7 @@ public class SentryStore {
       String  objName = authzToPaths.getAuthzObjName();
       // Convert path strings to list of components
       for (String path: authzToPaths.getPathStrings()) {
-        String[] pathComponents = path.split("/");
+        String[] pathComponents = PathUtils.splitPath(path);
         List<String> paths = new ArrayList<>(pathComponents.length);
         Collections.addAll(paths, pathComponents);
         pathUpdate.applyAddChanges(objName, Collections.singletonList(paths));

http://git-wip-us.apache.org/repos/asf/sentry/blob/7dbadfe8/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
index 07a7db4..d92f23e 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
@@ -38,6 +38,7 @@ import org.apache.sentry.binding.metastore.messaging.json.SentryJSONMessageDeser
 import org.apache.sentry.core.common.exception.SentryInvalidHMSEventException;
 import org.apache.sentry.core.common.exception.SentryInvalidInputException;
 import org.apache.sentry.core.common.exception.SentryNoSuchObjectException;
+import org.apache.sentry.core.common.utils.PathUtils;
 import org.apache.sentry.hdfs.PathsUpdate;
 import org.apache.sentry.hdfs.PermissionsUpdate;
 import org.apache.sentry.hdfs.SentryMalformedPathException;
@@ -111,7 +112,7 @@ final class NotificationProcessor {
    * @return list of components, e.g. [foo, bar]
    */
   private static List<String> splitPath(String path) {
-    return (Lists.newArrayList(path.split("/")));
+    return Lists.newArrayList(PathUtils.splitPath(path));
   }
 
   /**


Mime
View raw message