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: [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]
