This is an automated email from the ASF dual-hosted git repository.
ivandika pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 72d69e5e420 HDDS-13444. Unify isValidKeyPath implementation (#9521)
72d69e5e420 is described below
commit 72d69e5e420047c163989fc255319f815ce50484
Author: KUAN-HAO HUANG <[email protected]>
AuthorDate: Thu Feb 19 13:10:44 2026 +0800
HDDS-13444. Unify isValidKeyPath implementation (#9521)
---
.../hadoop/ozone/om/helpers/OzoneFSUtils.java | 79 ++++++++++++++++++----
.../hadoop/ozone/om/helpers/TestOzoneFsUtils.java | 40 +++++++++++
.../hadoop/ozone/om/request/OMClientRequest.java | 39 +----------
3 files changed, 107 insertions(+), 51 deletions(-)
diff --git
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFSUtils.java
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFSUtils.java
index e4116876a3c..9893f3c5c03 100644
---
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFSUtils.java
+++
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFSUtils.java
@@ -19,6 +19,7 @@
import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
import static org.apache.hadoop.ozone.OzoneConsts.OZONE_URI_DELIMITER;
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_KEY_NAME;
import jakarta.annotation.Nonnull;
import java.nio.file.Paths;
@@ -26,6 +27,7 @@
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hdds.conf.ConfigurationSource;
import org.apache.hadoop.ozone.OzoneConfigKeys;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.util.StringUtils;
import org.apache.hadoop.util.Time;
import org.slf4j.Logger;
@@ -97,34 +99,81 @@ public static boolean isFile(String keyName) {
}
/**
- * Whether the pathname is valid. Currently prohibits relative paths,
- * names which contain a ":" or "//", or other non-canonical paths.
+ * Core validation logic for key path components.
+ * Checks for invalid characters: ".", "..", ":", "/", and "//" in the
middle.
+ *
+ * @param path the path to validate
+ * @param allowLeadingSlash whether leading slash is allowed (true) or
invalid (false)
+ * @return true if path components are valid, false otherwise
*/
- public static boolean isValidName(String src) {
- // Path must be absolute.
- if (!src.startsWith(Path.SEPARATOR)) {
- return false;
+ private static boolean validateKeyPathComponents(String path, boolean
allowLeadingSlash) {
+ if (allowLeadingSlash) {
+ if (!path.startsWith(Path.SEPARATOR)) {
+ return false;
+ }
+ } else {
+ if (path.startsWith(Path.SEPARATOR)) {
+ return false;
+ }
}
- // Check for ".." "." ":" "/"
- String[] components = StringUtils.split(src, '/');
+ String[] components = StringUtils.split(path, '/');
for (int i = 0; i < components.length; i++) {
String element = components[i];
- if (element.equals(".") ||
- (element.contains(":")) ||
- (element.contains("/") || element.equals(".."))) {
+ if (element.equals(".") ||
+ element.contains(":") ||
+ element.contains("/") ||
+ element.equals("..")) {
return false;
}
- // The string may start or end with a /, but not have
- // "//" in the middle.
- if (element.isEmpty() && i != components.length - 1 &&
- i != 0) {
+
+ // The string may end with a /, but not have "//" in the middle.
+ if (element.isEmpty() && i != components.length - 1) {
+ // Allow empty at start for absolute paths (e.g., "/a/b" → ["", "a",
"b"])
+ // This is needed for isValidName but not for isValidKeyPath (which
rejects leading slashes)
+ if (allowLeadingSlash && i == 0) {
+ continue;
+ }
return false;
}
}
return true;
}
+ /**
+ * Whether the pathname is valid. Currently prohibits relative paths,
+ * names which contain a ":" or "//", or other non-canonical paths.
+ */
+ public static boolean isValidName(String path) {
+ return validateKeyPathComponents(path, true);
+ }
+
+ /**
+ * Whether the pathname is valid. Check key names which contain a
+ * ":", ".", "..", "//", "". If it has any of these characters throws
+ * OMException, else return the path.
+ *
+ * @param path the path to validate
+ * @param throwOnEmpty whether to throw exception if path is empty
+ * @return the path if valid
+ * @throws OMException if path is invalid
+ */
+ public static String isValidKeyPath(String path, boolean throwOnEmpty)
throws OMException {
+ if (path.isEmpty()) {
+ if (throwOnEmpty) {
+ throw new OMException("Invalid KeyPath, empty keyName",
+ INVALID_KEY_NAME);
+ }
+ return path;
+ }
+
+ if (!validateKeyPathComponents(path, false)) {
+ throw new OMException("Invalid KeyPath " + path, INVALID_KEY_NAME);
+ }
+
+ return path;
+ }
+
/**
* Checks whether the bucket layout is valid for File System operations
* otherwise throws IllegalArgumentException.
diff --git
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOzoneFsUtils.java
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOzoneFsUtils.java
index 3d0c344b8f4..f53cc0e03e2 100644
---
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOzoneFsUtils.java
+++
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOzoneFsUtils.java
@@ -19,10 +19,12 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.ozone.OzoneConfigKeys;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
@@ -67,4 +69,42 @@ void testCanEnableHsync(boolean canEnableHsync,
assertEquals(canEnableHsync, OzoneFSUtils.canEnableHsync(conf, isClient));
}
+
+ @Test
+ public void testIsValidKeyPath() throws OMException {
+ // Valid paths
+ assertEquals("a/b/c", OzoneFSUtils.isValidKeyPath("a/b/c", true));
+ assertEquals("a/b/c", OzoneFSUtils.isValidKeyPath("a/b/c", false));
+ assertEquals("file", OzoneFSUtils.isValidKeyPath("file", true));
+ assertEquals("file.txt", OzoneFSUtils.isValidKeyPath("file.txt", true));
+ assertEquals("dir/subdir/file",
OzoneFSUtils.isValidKeyPath("dir/subdir/file", true));
+
+ // Empty path - throwOnEmpty=true should throw exception
+ assertThrows(OMException.class, () -> OzoneFSUtils.isValidKeyPath("",
true));
+
+ // Empty path - throwOnEmpty=false should return empty string
+ assertEquals("", OzoneFSUtils.isValidKeyPath("", false));
+
+ // Invalid paths - leading slash
+ assertThrows(OMException.class, () -> OzoneFSUtils.isValidKeyPath("/a/b",
true));
+ assertThrows(OMException.class, () -> OzoneFSUtils.isValidKeyPath("/a/b",
false));
+
+ // Invalid paths - contains ".."
+ assertThrows(OMException.class, () ->
OzoneFSUtils.isValidKeyPath("a/../b", true));
+ assertThrows(OMException.class, () ->
OzoneFSUtils.isValidKeyPath("../a/b", true));
+
+ // Invalid paths - contains "."
+ assertThrows(OMException.class, () -> OzoneFSUtils.isValidKeyPath("a/./b",
true));
+ assertThrows(OMException.class, () ->
OzoneFSUtils.isValidKeyPath("./file", true));
+
+ // Invalid paths - contains ":"
+ assertThrows(OMException.class, () -> OzoneFSUtils.isValidKeyPath("a:b",
true));
+ assertThrows(OMException.class, () -> OzoneFSUtils.isValidKeyPath("a/b:c",
true));
+
+ // Invalid paths - contains "//" in the middle
+ assertThrows(OMException.class, () -> OzoneFSUtils.isValidKeyPath("a//b",
true));
+
+ // Valid path ending with "/"
+ assertEquals("a/b/", OzoneFSUtils.isValidKeyPath("a/b/", true));
+ }
}
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java
index fdb1f30a7cb..884c5fa310b 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java
@@ -17,7 +17,6 @@
package org.apache.hadoop.ozone.om.request;
-import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_KEY_NAME;
import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UNAUTHORIZED;
import com.google.common.annotations.VisibleForTesting;
@@ -45,6 +44,7 @@
import org.apache.hadoop.ozone.om.execution.flowcontrol.ExecutionContext;
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
import org.apache.hadoop.ozone.om.helpers.OMAuditLogger;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
import org.apache.hadoop.ozone.om.lock.OMLockDetails;
import org.apache.hadoop.ozone.om.protocolPB.grpc.GrpcClientConstants;
import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils;
@@ -94,8 +94,7 @@ public enum Result {
}
public OMClientRequest(OMRequest omRequest) {
- Objects.requireNonNull(omRequest, "omRequest == null");
- this.omRequest = omRequest;
+ this.omRequest = Objects.requireNonNull(omRequest);
this.omLockDetails.clear();
}
@@ -566,39 +565,7 @@ public static String validateAndNormalizeKey(String
keyName)
* OMException, else return the path.
*/
public static String isValidKeyPath(String path) throws OMException {
- boolean isValid = true;
-
- // If keyName is empty string throw error.
- if (path.isEmpty()) {
- throw new OMException("Invalid KeyPath, empty keyName" + path,
- INVALID_KEY_NAME);
- } else if (path.startsWith("/")) {
- isValid = false;
- } else {
- // Check for ".." "." ":" "/"
- String[] components = StringUtils.split(path, '/');
- for (int i = 0; i < components.length; i++) {
- String element = components[i];
- if (element.equals(".") ||
- (element.contains(":")) ||
- (element.contains("/") || element.equals(".."))) {
- isValid = false;
- break;
- }
-
- // The string may end with a /, but not have
- // "//" in the middle.
- if (element.isEmpty() && i != components.length - 1) {
- isValid = false;
- }
- }
- }
-
- if (isValid) {
- return path;
- } else {
- throw new OMException("Invalid KeyPath " + path, INVALID_KEY_NAME);
- }
+ return OzoneFSUtils.isValidKeyPath(path, true);
}
public OMLockDetails getOmLockDetails() {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]