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 <[email protected]> Authored: Tue Oct 24 09:57:13 2017 -0700 Committer: Vadim Spector <[email protected]> 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)); } /**
