You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "adutra (via GitHub)" <gi...@apache.org> on 2026/10/22 10:15:16 UTC

[PR] Make allowed-location checks consistent for paths with empty segments [polaris]

adutra opened a new pull request, #5849:
URL: https://github.com/apache/polaris/pull/5849

   `StorageLocationValidator` trimmed one trailing slash from allowed locations before comparing them. This is redundant with `StorageLocation.isChildOf`, which already handles trailing slashes, and it changed the meaning of locations ending with an empty path segment (e.g. "s3://bucket//" was handled as "s3://bucket/"). Stop trimming so that allowed locations are interpreted consistently.
   
   Likewise, `AzureLocation.isChildOf` only re-added the leading slash of the file path when it was missing, making "abfss://container@host//a" equivalent to "abfss://container@host/a". Always re-add the separator slash instead, and remove the now-unused `StorageLocation.ensureLeadingSlash`.
   
   <!--
   ๐Ÿ“ Describe what changes you're proposing, especially breaking or user-facing changes. 
   ๐Ÿ“– See https://github.com/apache/polaris/blob/main/CONTRIBUTING.md for more.
   -->
   
   ## Checklist
   - [ ] ๐Ÿ›ก๏ธ Don't disclose security issues! (contact security@apache.org)
   - [ ] ๐Ÿ”— Clearly explained why the changes are needed, or linked related issues: Fixes #
   - [ ] ๐Ÿงช Added/updated tests with good coverage, or manually tested (and explained how)
   - [ ] ๐Ÿ’ก Added comments for complex logic
   - [ ] ๐Ÿงพ Updated `CHANGELOG.md` (if needed)
   - [ ] ๐Ÿ“š Updated documentation in `site/content/in-dev/unreleased` (if needed)
   


-- 
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@polaris.apache.org

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


Re: [PR] Make allowed-location checks consistent for paths with empty segments [polaris]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #5849:
URL: https://github.com/apache/polaris/pull/5849#discussion_r4347257573


##########
polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureLocation.java:
##########
@@ -122,9 +122,10 @@ public boolean isChildOf(@NonNull StorageLocation potentialParent) {
     if (potentialParent instanceof AzureLocation potentialAzureParent) {
       if (this.container.equals(potentialAzureParent.container)) {
         if (this.storageAccount.equals(potentialAzureParent.storageAccount)) {
-          String formattedFilePath = ensureLeadingSlash(ensureTrailingSlash(this.filePath));
-          String formattedParentFilePath =
-              ensureLeadingSlash(ensureTrailingSlash(potentialAzureParent.filePath));
+          // filePath has its separator slash removed, so always add it back (only adding it when
+          // missing would make "container@host//a" and "container@host/a" equivalent).
+          String formattedFilePath = ensureTrailingSlash("/" + this.filePath);
+          String formattedParentFilePath = ensureTrailingSlash("/" + potentialAzureParent.filePath);

Review Comment:
   The parent-path formatting line is quite long and both formatted strings use the same normalization pattern. Consider extracting a small private helper (e.g., `formatForComparison(filePath)`) and wrapping the long assignment for readability (and to avoid potential Checkstyle line-length failures). This also centralizes the invariant that Azure `filePath` is stored without the separator slash.



-- 
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@polaris.apache.org

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


Re: [PR] Make allowed-location checks consistent for paths with empty segments [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra merged PR #5849:
URL: https://github.com/apache/polaris/pull/5849


-- 
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@polaris.apache.org

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