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..ebffa3ecb09 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" + path, + 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..bc5264346df 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,40 @@ 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("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)); + + // 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() {