Remove Java URI validations for Blob Storage providers (#1604)

This is a retry at #1586, which I'll close if this PR is on the right direction instead.

Java URI does not actually apply any normalization to URIs if we do not call URI.normalize() (which we currently do not). Additionally, blob storage providers like S3 and GCS can provide ".." and "." as valid fragments in URLs - which Java URI would attempt to normalize incorrectly. As a result, attempting to validate and/or normalize URIs for blob storage providers using the Java URI class is the incorrect behavior. While we may want to add location validation via regex later, removing it first should at least unblock the bug we see in #1545.

Author: adnanhemani

polaris-core/src/main/java/org/apache/polaris/core/storage/StorageLocation.java

@@ -40,12 +40,12 @@ public static StorageLocation of(String location) {
   protected StorageLocation(@Nonnull String location) {
     if (location == null) {
       this.location = null;
-    } else if (location.startsWith("file:/") && !location.startsWith(LOCAL_PATH_PREFIX)) {
+    } else if (location.startsWith("file:/")) {
       this.location = URI.create(location.replaceFirst("file:/+", LOCAL_PATH_PREFIX)).toString();
     } else if (location.startsWith("/")) {
       this.location = URI.create(location.replaceFirst("/+", LOCAL_PATH_PREFIX)).toString();
     } else {
-      this.location = URI.create(location).toString();
+      this.location = location;
     }
   }
 

polaris-core/src/test/java/org/apache/polaris/service/storage/StorageLocationTest.java

@@ -18,18 +18,63 @@
  */
 package org.apache.polaris.service.storage;
 
+import java.net.URISyntaxException;
 import org.apache.polaris.core.storage.StorageLocation;
+import org.apache.polaris.core.storage.azure.AzureLocation;
 import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 public class StorageLocationTest {
 
   @Test
   public void testOfDifferentPrefixes() {
-    StorageLocation StandardLocation = StorageLocation.of("file:///path/to/file");
+    StorageLocation standardLocation = StorageLocation.of("file:///path/to/file");
     StorageLocation slashLeadingLocation = StorageLocation.of("/path/to/file");
+    StorageLocation manySlashLeadingLocation = StorageLocation.of("////////path/to/file");
     StorageLocation fileSingleSlashLocation = StorageLocation.of("file:/path/to/file");
-    Assertions.assertThat(slashLeadingLocation.equals(StandardLocation)).isTrue();
-    Assertions.assertThat(fileSingleSlashLocation.equals(StandardLocation)).isTrue();
+    StorageLocation fileTooManySlashesLocation = StorageLocation.of("file://///////path/to/file");
+    Assertions.assertThat(slashLeadingLocation.equals(standardLocation)).isTrue();
+    Assertions.assertThat(manySlashLeadingLocation.equals(standardLocation)).isTrue();
+    Assertions.assertThat(fileSingleSlashLocation.equals(standardLocation)).isTrue();
+    Assertions.assertThat(fileTooManySlashesLocation.equals(standardLocation)).isTrue();
+    Assertions.assertThat(standardLocation).isExactlyInstanceOf(StorageLocation.class);
+    Assertions.assertThat(slashLeadingLocation).isExactlyInstanceOf(StorageLocation.class);
+    Assertions.assertThat(manySlashLeadingLocation).isExactlyInstanceOf(StorageLocation.class);
+    Assertions.assertThat(fileSingleSlashLocation).isExactlyInstanceOf(StorageLocation.class);
+    Assertions.assertThat(fileTooManySlashesLocation).isExactlyInstanceOf(StorageLocation.class);
+  }
+
+  @Test
+  public void testBlobStorageLocations() {
+    StorageLocation azureLocation =
+        StorageLocation.of("wasb://container@storageaccount.blob.core.windows.net/myfile");
+    Assertions.assertThat(azureLocation instanceof AzureLocation).isTrue();
+    azureLocation =
+        StorageLocation.of("abfss://container@storageaccount.blob.core.windows.net/myfile");
+    Assertions.assertThat(azureLocation instanceof AzureLocation).isTrue();
+
+    String s3LocationStr = "s3://test-bucket/mydirectory";
+    StorageLocation s3Location = StorageLocation.of(s3LocationStr);
+    Assertions.assertThat(s3Location instanceof AzureLocation).isFalse();
+
+    Assertions.assertThat(s3Location.toString()).isEqualTo(s3LocationStr);
+  }
+
+  @Test
+  public void testSpecialCharacters() {
+    // Blob Storage does not have validations
+    String specialCharsBlobStorage = "s3://test-bucket/quote'/equals=/period/../myfile.parquet";
+    StorageLocation s3LocationSpecialCharacters = StorageLocation.of(specialCharsBlobStorage);
+
+    Assertions.assertThat(s3LocationSpecialCharacters.toString())
+        .isEqualTo(specialCharsBlobStorage);
+
+    // But local filesystems do
+    String specialCharsLocalStorage = "file:///var/tmp\"/myfile.parquet";
+    Assertions.assertThatThrownBy(
+            () -> {
+              StorageLocation.of(specialCharsLocalStorage);
+            })
+        .hasCauseInstanceOf(URISyntaxException.class);
   }
 }