From 63cf535bcc3280d2dfa90773f751a4353006e491 Mon Sep 17 00:00:00 2001 From: rich7420 Date: Wed, 17 Dec 2025 15:36:30 +0800 Subject: [PATCH 1/6] Unify isValidKeyPath implementation --- .../hadoop/ozone/om/helpers/OzoneFSUtils.java | 78 +++++++++++++++---- .../ozone/om/request/OMClientRequest.java | 71 +++-------------- 2 files changed, 74 insertions(+), 75 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 e4116876a3c1..77eaa87968a0 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,80 @@ 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) { + if (allowLeadingSlash && i == 0) { + // Allow empty at start for absolute paths + 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 src) { + return validateKeyPathComponents(src, 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/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 fdb1f30a7cb2..e2bd224aa2d6 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 @@ -21,16 +21,16 @@ import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.UNAUTHORIZED; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; import jakarta.annotation.Nonnull; import java.io.IOException; import java.net.InetAddress; import java.nio.file.InvalidPathException; import java.util.LinkedHashMap; import java.util.Map; -import java.util.Objects; import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.hdds.utils.TransactionInfo; -import org.apache.hadoop.ipc_.ProtobufRpcEngine; +import org.apache.hadoop.ipc.ProtobufRpcEngine; import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.audit.AuditAction; @@ -44,11 +44,13 @@ import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.execution.flowcontrol.ExecutionContext; import org.apache.hadoop.ozone.om.helpers.BucketLayout; +import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils; import org.apache.hadoop.ozone.om.helpers.OMAuditLogger; 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; import org.apache.hadoop.ozone.om.response.OMClientResponse; +import org.apache.hadoop.ozone.om.snapshot.ReferenceCounted; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.LayoutVersion; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; @@ -59,7 +61,6 @@ import org.apache.hadoop.ozone.security.acl.RequestContext; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.authentication.client.AuthenticationException; -import org.apache.ratis.util.function.UncheckedAutoCloseableSupplier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -69,7 +70,7 @@ */ public abstract class OMClientRequest implements RequestAuditor { - protected static final Logger LOG = + private static final Logger LOG = LoggerFactory.getLogger(OMClientRequest.class); private OMRequest omRequest; @@ -94,11 +95,10 @@ public enum Result { } public OMClientRequest(OMRequest omRequest) { - Objects.requireNonNull(omRequest, "omRequest == null"); + Preconditions.checkNotNull(omRequest); this.omRequest = omRequest; this.omLockDetails.clear(); } - /** * Perform pre-execute steps on a OMRequest. * @@ -134,13 +134,9 @@ public void handleRequestFailure(OzoneManager ozoneManager) { * Validate the OMRequest and update the cache. * This step should verify that the request can be executed, perform * any authorization steps and update the in-memory cache. - * + * This step does not persist the changes to the database. * - * To coders and reviewers, CAUTION: Do NOT bring external dependencies into this method, doing so could potentially - * cause divergence in OM DB states in HA. If you have to, be extremely careful. - * e.g. Do NOT invoke ACL check inside validateAndUpdateCache, which can use Ranger plugin that relies on external DB. - * * @return the response that will be returned to the client. */ public abstract OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context); @@ -304,7 +300,7 @@ protected void checkACLsWithFSO(OzoneManager ozoneManager, String volumeName, contextBuilder.setOwnerName(bucketOwner); } - try (UncheckedAutoCloseableSupplier rcMetadataReader = + try (ReferenceCounted rcMetadataReader = ozoneManager.getOmMetadataReader()) { OmMetadataReader omMetadataReader = (OmMetadataReader) rcMetadataReader.get(); @@ -370,7 +366,7 @@ public void checkAcls(OzoneManager ozoneManager, String bucketOwner) throws IOException { - try (UncheckedAutoCloseableSupplier rcMetadataReader = + try (ReferenceCounted rcMetadataReader = ozoneManager.getOmMetadataReader()) { OzoneAclUtils.checkAllAcls((OmMetadataReader) rcMetadataReader.get(), resType, storeType, aclType, @@ -516,6 +512,7 @@ public Map buildVolumeAuditMap(String volume) { return auditMap; } + public static String validateAndNormalizeKey(boolean enableFileSystemPaths, String keyName) throws OMException { if (enableFileSystemPaths) { @@ -525,20 +522,6 @@ public static String validateAndNormalizeKey(boolean enableFileSystemPaths, } } - /** - * Normalizes the key path based on the bucket layout. This should be used for existing keys. - * For new key creation, please see {@link #validateAndNormalizeKey(boolean, String, BucketLayout)} - * - * @return normalized key path - */ - public static String normalizeKeyPath(boolean enableFileSystemPaths, - String keyPath, BucketLayout bucketLayout) throws OMException { - if (bucketLayout.shouldNormalizePaths(enableFileSystemPaths)) { - keyPath = OmUtils.normalizeKey(keyPath, false); - } - return keyPath; - } - public static String validateAndNormalizeKey(boolean enableFileSystemPaths, String keyPath, BucketLayout bucketLayout) throws OMException { LOG.debug("Bucket Layout: {}", bucketLayout); @@ -566,39 +549,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() { From 36c251042bff6a2b6917ec91a0b68a94db921ac7 Mon Sep 17 00:00:00 2001 From: rich7420 Date: Wed, 17 Dec 2025 15:59:45 +0800 Subject: [PATCH 2/6] fix error --- .../apache/hadoop/ozone/om/request/OMClientRequest.java | 9 ++++----- .../hadoop/ozone/om/snapshot/ReferenceCounted.java | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) 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 e2bd224aa2d6..7e9417c1317b 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,17 +17,16 @@ 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; -import com.google.common.base.Preconditions; import jakarta.annotation.Nonnull; import java.io.IOException; import java.net.InetAddress; import java.nio.file.InvalidPathException; import java.util.LinkedHashMap; import java.util.Map; +import java.util.Objects; import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.hdds.utils.TransactionInfo; import org.apache.hadoop.ipc.ProtobufRpcEngine; @@ -44,8 +43,8 @@ import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.execution.flowcontrol.ExecutionContext; import org.apache.hadoop.ozone.om.helpers.BucketLayout; -import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils; 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; @@ -95,10 +94,10 @@ public enum Result { } public OMClientRequest(OMRequest omRequest) { - Preconditions.checkNotNull(omRequest); - this.omRequest = omRequest; + this.omRequest = Objects.requireNonNull(omRequest); this.omLockDetails.clear(); } + /** * Perform pre-execute steps on a OMRequest. * diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ReferenceCounted.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ReferenceCounted.java index 6937b5b4e148..4950aa858bee 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ReferenceCounted.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ReferenceCounted.java @@ -24,7 +24,7 @@ /** * Add reference counter to an object instance. */ -class ReferenceCounted { +public class ReferenceCounted { /** * Object that is being reference counted. e.g. OmSnapshot From b10c58dc2a97869ddc85c59c54e5d6d3ff12db63 Mon Sep 17 00:00:00 2001 From: rich7420 Date: Wed, 17 Dec 2025 16:12:47 +0800 Subject: [PATCH 3/6] fix ci errors --- .../apache/hadoop/ozone/om/request/OMClientRequest.java | 9 ++++----- .../hadoop/ozone/om/request/key/OMKeyDeleteRequest.java | 3 ++- 2 files changed, 6 insertions(+), 6 deletions(-) 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 7e9417c1317b..3acb8946f2c4 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 @@ -49,7 +49,6 @@ import org.apache.hadoop.ozone.om.protocolPB.grpc.GrpcClientConstants; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils; import org.apache.hadoop.ozone.om.response.OMClientResponse; -import org.apache.hadoop.ozone.om.snapshot.ReferenceCounted; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.LayoutVersion; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; @@ -60,6 +59,7 @@ import org.apache.hadoop.ozone.security.acl.RequestContext; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.authentication.client.AuthenticationException; +import org.apache.ratis.util.function.UncheckedAutoCloseableSupplier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -69,7 +69,7 @@ */ public abstract class OMClientRequest implements RequestAuditor { - private static final Logger LOG = + protected static final Logger LOG = LoggerFactory.getLogger(OMClientRequest.class); private OMRequest omRequest; @@ -299,7 +299,7 @@ protected void checkACLsWithFSO(OzoneManager ozoneManager, String volumeName, contextBuilder.setOwnerName(bucketOwner); } - try (ReferenceCounted rcMetadataReader = + try (UncheckedAutoCloseableSupplier rcMetadataReader = ozoneManager.getOmMetadataReader()) { OmMetadataReader omMetadataReader = (OmMetadataReader) rcMetadataReader.get(); @@ -365,7 +365,7 @@ public void checkAcls(OzoneManager ozoneManager, String bucketOwner) throws IOException { - try (ReferenceCounted rcMetadataReader = + try (UncheckedAutoCloseableSupplier rcMetadataReader = ozoneManager.getOmMetadataReader()) { OzoneAclUtils.checkAllAcls((OmMetadataReader) rcMetadataReader.get(), resType, storeType, aclType, @@ -511,7 +511,6 @@ public Map buildVolumeAuditMap(String volume) { return auditMap; } - public static String validateAndNormalizeKey(boolean enableFileSystemPaths, String keyName) throws OMException { if (enableFileSystemPaths) { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java index 4726d4af2d5f..aa2eb406a438 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java @@ -83,7 +83,8 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { String keyPath = keyArgs.getKeyName(); OmUtils.verifyKeyNameWithSnapshotReservedWordForDeletion(keyPath); - keyPath = normalizeKeyPath(ozoneManager.getEnableFileSystemPaths(), keyPath, getBucketLayout()); + keyPath = validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(), + keyPath, getBucketLayout()); OzoneManagerProtocolProtos.KeyArgs.Builder newKeyArgs = keyArgs.toBuilder().setModificationTime(Time.now()).setKeyName(keyPath); From 4eed52e3f71d021b4d76e5b43f895e08a5d8cb01 Mon Sep 17 00:00:00 2001 From: rich7420 Date: Thu, 18 Dec 2025 09:49:26 +0800 Subject: [PATCH 4/6] fix error --- .../ozone/om/request/OMClientRequest.java | 22 +++++++++++++++++-- .../om/request/key/OMKeyDeleteRequest.java | 3 +-- 2 files changed, 21 insertions(+), 4 deletions(-) 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 3acb8946f2c4..884c5fa310bc 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 @@ -29,7 +29,7 @@ import java.util.Objects; import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.hdds.utils.TransactionInfo; -import org.apache.hadoop.ipc.ProtobufRpcEngine; +import org.apache.hadoop.ipc_.ProtobufRpcEngine; import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.audit.AuditAction; @@ -133,9 +133,13 @@ public void handleRequestFailure(OzoneManager ozoneManager) { * Validate the OMRequest and update the cache. * This step should verify that the request can be executed, perform * any authorization steps and update the in-memory cache. - + * * This step does not persist the changes to the database. * + * To coders and reviewers, CAUTION: Do NOT bring external dependencies into this method, doing so could potentially + * cause divergence in OM DB states in HA. If you have to, be extremely careful. + * e.g. Do NOT invoke ACL check inside validateAndUpdateCache, which can use Ranger plugin that relies on external DB. + * * @return the response that will be returned to the client. */ public abstract OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context); @@ -520,6 +524,20 @@ public static String validateAndNormalizeKey(boolean enableFileSystemPaths, } } + /** + * Normalizes the key path based on the bucket layout. This should be used for existing keys. + * For new key creation, please see {@link #validateAndNormalizeKey(boolean, String, BucketLayout)} + * + * @return normalized key path + */ + public static String normalizeKeyPath(boolean enableFileSystemPaths, + String keyPath, BucketLayout bucketLayout) throws OMException { + if (bucketLayout.shouldNormalizePaths(enableFileSystemPaths)) { + keyPath = OmUtils.normalizeKey(keyPath, false); + } + return keyPath; + } + public static String validateAndNormalizeKey(boolean enableFileSystemPaths, String keyPath, BucketLayout bucketLayout) throws OMException { LOG.debug("Bucket Layout: {}", bucketLayout); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java index aa2eb406a438..4726d4af2d5f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java @@ -83,8 +83,7 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { String keyPath = keyArgs.getKeyName(); OmUtils.verifyKeyNameWithSnapshotReservedWordForDeletion(keyPath); - keyPath = validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(), - keyPath, getBucketLayout()); + keyPath = normalizeKeyPath(ozoneManager.getEnableFileSystemPaths(), keyPath, getBucketLayout()); OzoneManagerProtocolProtos.KeyArgs.Builder newKeyArgs = keyArgs.toBuilder().setModificationTime(Time.now()).setKeyName(keyPath); From f84cce7a65b49160b30639837d141356329b6969 Mon Sep 17 00:00:00 2001 From: rich7420 Date: Thu, 18 Dec 2025 10:24:10 +0800 Subject: [PATCH 5/6] add test --- .../ozone/om/helpers/TestOzoneFsUtils.java | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) 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 3d0c344b8f4f..4090f303c177 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,37 @@ 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)); + } } From 9b714f14ac0a9bf8f2dc900300dbbbc0115f1954 Mon Sep 17 00:00:00 2001 From: rich7420 Date: Thu, 18 Dec 2025 13:32:02 +0800 Subject: [PATCH 6/6] improve according to suggestions --- .../org/apache/hadoop/ozone/om/helpers/OzoneFSUtils.java | 7 ++++--- .../apache/hadoop/ozone/om/helpers/TestOzoneFsUtils.java | 3 +++ .../apache/hadoop/ozone/om/snapshot/ReferenceCounted.java | 2 +- 3 files changed, 8 insertions(+), 4 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 77eaa87968a0..ebffa3ecb091 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 @@ -129,8 +129,9 @@ private static boolean validateKeyPathComponents(String path, boolean allowLeadi // 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) { - // Allow empty at start for absolute paths continue; } return false; @@ -143,8 +144,8 @@ private static boolean validateKeyPathComponents(String path, boolean allowLeadi * Whether the pathname is valid. Currently prohibits relative paths, * names which contain a ":" or "//", or other non-canonical paths. */ - public static boolean isValidName(String src) { - return validateKeyPathComponents(src, true); + public static boolean isValidName(String path) { + return validateKeyPathComponents(path, true); } /** 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 4090f303c177..bc5264346df7 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 @@ -101,5 +101,8 @@ public void testIsValidKeyPath() throws OMException { // 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/snapshot/ReferenceCounted.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ReferenceCounted.java index 4950aa858bee..6937b5b4e148 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ReferenceCounted.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ReferenceCounted.java @@ -24,7 +24,7 @@ /** * Add reference counter to an object instance. */ -public class ReferenceCounted { +class ReferenceCounted { /** * Object that is being reference counted. e.g. OmSnapshot