[GitHub] [iceberg] electrum commented on a diff in pull request #6772: Core: enforce writing POSIX compatible paths

["electrum (via GitHub)" <gi...@apache.org>]

electrum commented on code in PR #6772:
URL: https://github.com/apache/iceberg/pull/6772#discussion_r1100535487


##########
core/src/main/java/org/apache/iceberg/util/LocationUtil.java:
##########
@@ -33,4 +34,8 @@ public static String stripTrailingSlash(String path) {
     }
     return result;
   }
+
+  public static String posixNormalize(String path) {
+    return Paths.get(path).normalize().toString();

Review Comment:
   This method is not suitable for Iceberg paths, since Java `Path` is for paths, but Iceberg paths are actually URIs. It mangles URIs and doesn't understand the scheme vs path distinction:
   ```
   jshell> Paths.get("s3://foo/bar").toString()
   $2 ==> "s3:/foo/bar"
   
   jshell> Paths.get("s3://foo/bar/../..").normalize().toString()
   $5 ==> "s3:"
   
   jshell> Paths.get("s3://foo/bar/../../..").normalize().toString()
   $6 ==> ""
   ```
   
   `URI.normalize()` seems better, though it has interesting behavior where it preserves leading `..` path segments at the start of the path that would go past the root:
   
   ```
   jshell> URI.create("s3://foo/abc/../../..").normalize()
   $11 ==> s3://foo/../..
   ```



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org