Skip to content

Commit c79aa1e

Browse files
committed
fix(catalog): encode default table locations with UTF-8
buildPrefixedLocation derived a table's default storage location by percent-encoding the namespace levels and table name with URLEncoder.encode(..., Charset.defaultCharset()). For non-ASCII identifiers the persisted location then depended on the JVM's default charset, so the same table could resolve to a different (or lossy) location on a non-UTF-8 JVM, breaking portability and round-trip expectations. Encode both segments with StandardCharsets.UTF_8, matching PolarisEntityUtils.encodeNamespace and every other URLEncoder call site in the service. Add a regression test that creates a table under non-ASCII namespace and table identifiers with object-storage prefixing enabled and asserts the derived location uses the UTF-8 percent-encoded form regardless of the platform default charset.
1 parent 7b832fb commit c79aa1e

3 files changed

Lines changed: 71 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
7979
- Inheritable policy mapping inserts in the JDBC backend now use the active transaction connection, so they roll back correctly with the surrounding transaction.
8080
- Generic table drop now accepts table-scoped `TABLE_DROP` privilege.
8181
- Azure SAS tokens are now signed for the configured duration instead of a hardcoded 1 hour, so long jobs no longer fail with expired credentials.
82+
- Default table storage locations with object-storage prefixing enabled are now percent-encoded with UTF-8 instead of the JVM default charset. Previously the persisted location of a table under a non-ASCII namespace or table name depended on the platform default charset, so the same table could resolve to a different (or lossy) location on a non-UTF-8 JVM.
8283

8384
## [1.5.0]
8485

runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
import java.io.Closeable;
3535
import java.io.IOException;
3636
import java.net.URLEncoder;
37-
import java.nio.charset.Charset;
37+
import java.nio.charset.StandardCharsets;
3838
import java.util.ArrayList;
3939
import java.util.Arrays;
4040
import java.util.Collections;
@@ -1128,11 +1128,11 @@ private String buildPrefixedLocation(TableIdentifier tableIdentifier) {
11281128
locationBuilder.append(LocationUtils.computeHash(tableIdentifier.toString()));
11291129

11301130
for (String ns : tableIdentifier.namespace().levels()) {
1131-
locationBuilder.append("/").append(URLEncoder.encode(ns, Charset.defaultCharset()));
1131+
locationBuilder.append("/").append(URLEncoder.encode(ns, StandardCharsets.UTF_8));
11321132
}
11331133
locationBuilder
11341134
.append("/")
1135-
.append(URLEncoder.encode(tableIdentifier.name(), Charset.defaultCharset()))
1135+
.append(URLEncoder.encode(tableIdentifier.name(), StandardCharsets.UTF_8))
11361136
.append("/");
11371137
return locationBuilder.toString();
11381138
}

runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergOverlappingTableTest.java

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2828

2929
import jakarta.ws.rs.core.Response;
30+
import java.net.URLEncoder;
31+
import java.nio.charset.StandardCharsets;
3032
import java.nio.file.Path;
3133
import java.util.List;
3234
import java.util.Map;
@@ -144,14 +146,18 @@ private int createTableStaged(TestServices services, String location) {
144146
* creation fails, this should return null
145147
*/
146148
private String createTableWithName(TestServices services, String name) {
149+
return createTableWithName(services, namespace, name);
150+
}
151+
152+
private String createTableWithName(TestServices services, String namespaceName, String name) {
147153
CreateTableRequest createTableRequest =
148154
CreateTableRequest.builder().withName(name).withSchema(SCHEMA).build();
149155
try (Response response =
150156
services
151157
.restApi()
152158
.createTable(
153159
catalog,
154-
namespace,
160+
namespaceName,
155161
createTableRequest,
156162
null,
157163
IDEMPOTENCY_KEY,
@@ -169,6 +175,14 @@ private String createTableWithName(TestServices services, String name) {
169175

170176
private void createCatalogAndNamespace(
171177
TestServices services, Map<String, String> catalogConfig, String catalogLocation) {
178+
createCatalogAndNamespace(services, catalogConfig, catalogLocation, namespace);
179+
}
180+
181+
private void createCatalogAndNamespace(
182+
TestServices services,
183+
Map<String, String> catalogConfig,
184+
String catalogLocation,
185+
String namespaceName) {
172186
CatalogProperties.Builder propertiesBuilder =
173187
CatalogProperties.builder()
174188
.setDefaultBaseLocation(String.format("%s/%s", catalogLocation, catalog))
@@ -198,7 +212,7 @@ private void createCatalogAndNamespace(
198212
}
199213

200214
CreateNamespaceRequest createNamespaceRequest =
201-
CreateNamespaceRequest.builder().withNamespace(Namespace.of(namespace)).build();
215+
CreateNamespaceRequest.builder().withNamespace(Namespace.of(namespaceName)).build();
202216
try (Response response =
203217
services
204218
.restApi()
@@ -727,4 +741,55 @@ void testParentChildOverlapWithOptimizedSiblingCheck(@TempDir Path tempDir) {
727741
assertThat(createTable(services, derivedLocation + "/child"))
728742
.isEqualTo(Response.Status.FORBIDDEN.getStatusCode());
729743
}
744+
745+
@Test
746+
@DisplayName("Hashed default location encodes non-ASCII identifiers with UTF-8")
747+
void testHashedTableLocationsEncodeNonAsciiIdentifiersAsUtf8(@TempDir Path tempDir) {
748+
// Uses non-ASCII namespace and table identifiers to pin the durable location encoding to
749+
// UTF-8. With the platform default charset the persisted percent-encoding would vary (or be
750+
// lossy) across JVMs, so the derived location must always be the UTF-8 encoded form.
751+
Map<String, Object> strictServicesWithOptimizedOverlapCheck =
752+
Map.of(
753+
"ALLOW_UNSTRUCTURED_TABLE_LOCATION",
754+
"false",
755+
"ALLOW_TABLE_LOCATION_OVERLAP",
756+
"false",
757+
"ALLOW_INSECURE_STORAGE_TYPES",
758+
"true",
759+
"SUPPORTED_CATALOG_STORAGE_TYPES",
760+
List.of("FILE", "S3"),
761+
OPTIMIZED_SIBLING_CHECK.key(),
762+
"true");
763+
Map<String, String> hashedCatalog =
764+
Map.of(
765+
DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED.catalogConfig(),
766+
"true",
767+
ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(),
768+
"true");
769+
770+
TestServices services =
771+
TestServices.builder().config(strictServicesWithOptimizedOverlapCheck).build();
772+
773+
String baseLocation = tempDir.toAbsolutePath().toUri().toString();
774+
if (baseLocation.endsWith("/")) {
775+
baseLocation = baseLocation.substring(0, baseLocation.length() - 1);
776+
}
777+
778+
String nonAsciiNamespace = "n\u00e9\u6f22"; // né漢
779+
createCatalogAndNamespace(services, hashedCatalog, baseLocation, nonAsciiNamespace);
780+
781+
String nonAsciiTableName = "t\u00e9\u6f22"; // té漢
782+
String tableLocation = createTableWithName(services, nonAsciiNamespace, nonAsciiTableName);
783+
Assertions.assertNotNull(tableLocation);
784+
785+
String expectedEncodedSuffix =
786+
String.format(
787+
"/%s/%s",
788+
URLEncoder.encode(nonAsciiNamespace, StandardCharsets.UTF_8),
789+
URLEncoder.encode(nonAsciiTableName, StandardCharsets.UTF_8));
790+
assertThat(tableLocation).endsWith(expectedEncodedSuffix);
791+
// The raw (unencoded) identifiers must never appear verbatim in the durable location.
792+
assertThat(tableLocation).doesNotContain(nonAsciiNamespace);
793+
assertThat(tableLocation).doesNotContain(nonAsciiTableName);
794+
}
730795
}

0 commit comments

Comments
 (0)