rich7420 commented on code in PR #9521:
URL: https://github.com/apache/ozone/pull/9521#discussion_r2629623358


##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFSUtils.java:
##########
@@ -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;
+        }

Review Comment:
   sure!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to