You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "iprithv (via GitHub)" <gi...@apache.org> on 2026/09/14 09:13:50 UTC

[PR] Fix GCS credential vending crash on bucket-root locations without trailing slash [polaris]

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

   GCS credential vending crashes with an HTTP 500 when a location is a plain bucket root with no trailing slash, like `gs://my-bucket`. The GCS code builds the object prefix using `uri.rawPath().substring(1)`, but `gs://my-bucket` has an empty path, so calling `substring(1)` on an empty string throws. The `gs://my-bucket/` form has a `/` and works fine, so only the no-slash version breaks. This is easy to hit: the vended locations come straight from the table's `location` and the `write.data.path` / `write.metadata.path` properties, which are not forced to end in a slash, so a table pointed at a bucket root fails on every credential request. AWS does the same job safely with a `trimLeadingSlash` helper that checks for the slash first; GCS was the only provider not doing that. This change makes GCS use the same safe trim at the three affected spots (a bucket root now behaves exactly like the trailing-slash form).


-- 
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] Fix GCS credential vending crash on bucket-root locations without trailing slash [polaris]

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


-- 
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] Fix GCS credential vending crash on bucket-root locations without trailing slash [polaris]

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


##########
polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java:
##########
@@ -581,4 +581,16 @@ static String escapeCelLiteral(String value) {
   private static String bucketResource(String bucket) {
     return "//storage.googleapis.com/projects/_/buckets/" + bucket;
   }
+
+  /**
+   * Strips a single leading slash from a raw path, tolerating an empty raw path. A bucket-root
+   * location such as {@code gs://bucket} parses to an empty raw path, so an unguarded {@code
+   * substring(1)} would throw. This mirrors the AWS integration's handling.
+   */
+  private static String trimLeadingSlash(String path) {
+    if (path.startsWith("/")) {
+      path = path.substring(1);
+    }
+    return path;
+  }

Review Comment:
   There are similar copies of this logic
   https://github.com/apache/polaris/blob/08222ac8d8d63f32eb370d156ea1f8ce798676c0/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java#L562-L566
   
   and one inline here:
   https://github.com/apache/polaris/blob/08222ac8d8d63f32eb370d156ea1f8ce798676c0/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureLocation.java#L73
   
   maybe we should try to refactor and avoid duplication



-- 
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] Fix GCS credential vending crash on bucket-root locations without trailing slash [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5512:
URL: https://github.com/apache/polaris/pull/5512#discussion_r4006204773


##########
polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java:
##########
@@ -581,4 +581,16 @@ static String escapeCelLiteral(String value) {
   private static String bucketResource(String bucket) {
     return "//storage.googleapis.com/projects/_/buckets/" + bucket;
   }
+
+  /**
+   * Strips a single leading slash from a raw path, tolerating an empty raw path. A bucket-root
+   * location such as {@code gs://bucket} parses to an empty raw path, so an unguarded {@code
+   * substring(1)} would throw. This mirrors the AWS integration's handling.
+   */
+  private static String trimLeadingSlash(String path) {
+    if (path.startsWith("/")) {
+      path = path.substring(1);
+    }
+    return path;
+  }

Review Comment:
   +1 to avoiding duplication :+1: 



-- 
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] Fix GCS credential vending crash on bucket-root locations without trailing slash [polaris]

Posted by "iprithv (via GitHub)" <gi...@apache.org>.
iprithv commented on PR #5512:
URL: https://github.com/apache/polaris/pull/5512#issuecomment-5680364277

   Good call, thanks both @ayushtkn @dimas-b . To keep this fix focused and preserve the existing approval, I've kept #5512 as-is and pulled the dedup into a small follow-up: a single` StorageLocation.trimLeadingSlash` that the AWS, GCS, and Azure paths all use. I made it null-tolerant so `AzureLocation`'s current null -> "" behaviour is preserved, and the AWS/GCS paths never pass null. Will raise it right after this one merges.


-- 
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] Fix GCS credential vending crash on bucket-root locations without trailing slash [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #5512:
URL: https://github.com/apache/polaris/pull/5512#issuecomment-5665636211

   Thanks for the fix, by the way :+1: Nice catch!


-- 
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