You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by ru...@apache.org on 2026/09/11 15:10:51 UTC
(iceberg) branch main updated: Core, REST: Fix path segment encoding to use RFC 3986 percent-encoding (#15989)
This is an automated email from the ASF dual-hosted git repository.
RussellSpitzer pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/main by this push:
new 6585738915 Core, REST: Fix path segment encoding to use RFC 3986 percent-encoding (#15989)
6585738915 is described below
commit 6585738915edcc8a10ab40e5fd490e6efdef2333
Author: Alexandre Dutra <ad...@apache.org>
AuthorDate: Fri Sep 11 17:10:43 2026 +0200
Core, REST: Fix path segment encoding to use RFC 3986 percent-encoding (#15989)
---
.../java/org/apache/iceberg/rest/RESTUtil.java | 122 ++++++++++-
.../org/apache/iceberg/rest/ResourcePaths.java | 18 +-
.../apache/iceberg/rest/RESTCatalogAdapter.java | 8 +-
.../org/apache/iceberg/rest/TestHTTPRequest.java | 12 +-
.../org/apache/iceberg/rest/TestRESTCatalog.java | 22 ++
.../java/org/apache/iceberg/rest/TestRESTUtil.java | 149 +++++++++++++
.../org/apache/iceberg/rest/TestResourcePaths.java | 238 ++++++++++++++++++++-
7 files changed, 542 insertions(+), 27 deletions(-)
diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTUtil.java b/core/src/main/java/org/apache/iceberg/rest/RESTUtil.java
index 55de6d0e3d..53c12cd4a9 100644
--- a/core/src/main/java/org/apache/iceberg/rest/RESTUtil.java
+++ b/core/src/main/java/org/apache/iceberg/rest/RESTUtil.java
@@ -22,6 +22,7 @@ import java.net.URLDecoder;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Map;
+import org.apache.hc.core5.net.PercentCodec;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.relocated.com.google.common.base.Joiner;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
@@ -130,12 +131,17 @@ public class RESTUtil {
}
/**
- * Encodes a string using URL encoding
+ * Encodes a string using application/x-www-form-urlencoded encoding, where spaces are encoded as
+ * {@code +}.
+ *
+ * <p>This method is suitable for encoding form data (e.g. OAuth2 token requests) but <b>not</b>
+ * for URL path segments, where {@code +} is a literal character. Use {@link
+ * #encodePathSegment(String)} for path segments.
*
* <p>{@link #decodeString(String)} should be used to decode.
*
* @param toEncode string to encode
- * @return UTF-8 encoded string, suitable for use as a URL parameter
+ * @return form-encoded string, suitable for use in application/x-www-form-urlencoded content
*/
public static String encodeString(String toEncode) {
Preconditions.checkArgument(toEncode != null, "Invalid string to encode: null");
@@ -143,9 +149,13 @@ public class RESTUtil {
}
/**
- * Decodes a URL-encoded string.
+ * Decodes a string that was encoded using application/x-www-form-urlencoded encoding, where
+ * {@code +} is decoded as a space.
+ *
+ * <p>This method is suitable for decoding form data but <b>not</b> for URL path segments. Use
+ * {@link #decodePathSegment(String)} for path segments.
*
- * <p>See also {@link #encodeString(String)} for URL encoding.
+ * <p>See also {@link #encodeString(String)} for form encoding.
*
* @param encoded a string to decode
* @return a decoded string
@@ -155,6 +165,34 @@ public class RESTUtil {
return URLDecoder.decode(encoded, StandardCharsets.UTF_8);
}
+ /**
+ * Encodes a string for use as a URL path segment per RFC 3986. Spaces are encoded as {@code %20}
+ * (not {@code +}), and other non-unreserved characters are percent-encoded.
+ *
+ * <p>{@link #decodePathSegment(String)} should be used to decode.
+ *
+ * @param segment string to encode
+ * @return percent-encoded string suitable for use in URL path segments
+ */
+ public static String encodePathSegment(String segment) {
+ Preconditions.checkArgument(segment != null, "Invalid string to encode: null");
+ return PercentCodec.RFC3986.encode(segment);
+ }
+
+ /**
+ * Decodes a URL path segment per RFC 3986. Unlike {@link #decodeString(String)}, this method does
+ * <b>not</b> treat {@code +} as a space — it is left as a literal {@code +} character.
+ *
+ * <p>See also {@link #encodePathSegment(String)} for encoding.
+ *
+ * @param encoded a percent-encoded path segment
+ * @return a decoded string
+ */
+ public static String decodePathSegment(String encoded) {
+ Preconditions.checkArgument(encoded != null, "Invalid string to decode: null");
+ return PercentCodec.RFC3986.decode(encoded);
+ }
+
/**
* This converts the given namespace to a string and separates each part in a multipart namespace
* using the unicode character '\u001f'. Note that this method is different from {@link
@@ -238,13 +276,14 @@ public class RESTUtil {
}
/**
- * Returns a String representation of a namespace that is suitable for use in a URL / URI.
+ * Returns a String representation of a namespace that is suitable for use with
+ * application/x-www-form-urlencoded encoding.
*
- * <p>This function needs to be called when a namespace is used as a path variable (or query
- * parameter etc.), to format the namespace per the spec.
+ * <p>This function needs to be called when a namespace is used in a POST request body, to format
+ * the namespace per the spec.
*
* <p>{@link RESTUtil#decodeNamespace(String, String)} should be used to parse the namespace from
- * a URL parameter.
+ * a request body.
*
* @param namespace namespace to encode
* @param separator The namespace separator to be used for encoding. The separator will be used
@@ -266,10 +305,10 @@ public class RESTUtil {
}
/**
- * Takes in a string representation of a namespace as used for a URL parameter and returns the
- * corresponding namespace.
+ * Takes in a string representation of a namespace encoded with application/x-www-form-urlencoded
+ * encoding, and returns the corresponding namespace.
*
- * <p>See also {@link #encodeNamespace} for generating correctly formatted URLs.
+ * <p>See also {@link #encodeNamespace} for generating correctly formatted POST requests.
*
* @param encodedNamespace a namespace to decode
* @param separator The namespace separator to be used as-is for decoding. This should be the same
@@ -298,6 +337,67 @@ public class RESTUtil {
return Namespace.of(levels);
}
+ /**
+ * Returns a String representation of a namespace that is suitable for use in a URL path segment
+ * per RFC 3986. Spaces are encoded as {@code %20} (not {@code +}).
+ *
+ * <p>This method should be used instead of {@link #encodeNamespace(Namespace, String)} when the
+ * result is placed into a URL path.
+ *
+ * <p>{@link #decodeNamespaceAsPathSegment(String, String)} should be used to decode the result.
+ *
+ * @param namespace namespace to encode
+ * @param separator The namespace separator to be used for encoding. The separator will be used
+ * as-is and won't be encoded.
+ * @return percent-encoded string representing the namespace, suitable for use in URL path
+ * segments
+ */
+ public static String encodeNamespaceAsPathSegment(Namespace namespace, String separator) {
+ Preconditions.checkArgument(namespace != null, "Invalid namespace: null");
+ Preconditions.checkArgument(
+ !Strings.isNullOrEmpty(separator), "Invalid separator: null or empty");
+ String[] levels = namespace.levels();
+ String[] encodedLevels = new String[levels.length];
+
+ for (int i = 0; i < levels.length; i++) {
+ encodedLevels[i] = encodePathSegment(levels[i]);
+ }
+
+ return Joiner.on(separator).join(encodedLevels);
+ }
+
+ /**
+ * Decodes a URL path segment per RFC 3986 into a namespace. Unlike {@link
+ * #decodeNamespace(String, String)}, this method does <b>not</b> treat {@code +} as a space.
+ *
+ * <p>{@link #encodeNamespaceAsPathSegment(Namespace, String)} should be used for encoding path
+ * segments.
+ *
+ * @param encodedNamespace a percent-encoded namespace path segment
+ * @param separator The namespace separator used during encoding
+ * @return a namespace
+ */
+ public static Namespace decodeNamespaceAsPathSegment(String encodedNamespace, String separator) {
+ Preconditions.checkArgument(encodedNamespace != null, "Invalid namespace: null");
+ Preconditions.checkArgument(
+ !Strings.isNullOrEmpty(separator), "Invalid separator: null or empty");
+
+ // use legacy splitter for backwards compatibility in case an old client encoded the namespace
+ // with %1F
+ Splitter splitter =
+ Splitter.on(
+ encodedNamespace.contains(NAMESPACE_SEPARATOR_URLENCODED_UTF_8)
+ ? NAMESPACE_SEPARATOR_URLENCODED_UTF_8
+ : separator);
+ String[] levels = Iterables.toArray(splitter.split(encodedNamespace), String.class);
+
+ for (int i = 0; i < levels.length; i++) {
+ levels[i] = decodePathSegment(levels[i]);
+ }
+
+ return Namespace.of(levels);
+ }
+
/**
* Returns the catalog URI suffixed by the relative endpoint path. If the endpoint path is an
* absolute path, then the absolute endpoint path is returned without using the catalog URI.
diff --git a/core/src/main/java/org/apache/iceberg/rest/ResourcePaths.java b/core/src/main/java/org/apache/iceberg/rest/ResourcePaths.java
index d259263476..5757a6d176 100644
--- a/core/src/main/java/org/apache/iceberg/rest/ResourcePaths.java
+++ b/core/src/main/java/org/apache/iceberg/rest/ResourcePaths.java
@@ -101,7 +101,7 @@ public class ResourcePaths {
"namespaces",
pathEncode(ident.namespace()),
"tables",
- RESTUtil.encodeString(ident.name()));
+ RESTUtil.encodePathSegment(ident.name()));
}
public String register(Namespace ns) {
@@ -119,7 +119,7 @@ public class ResourcePaths {
"namespaces",
pathEncode(identifier.namespace()),
"tables",
- RESTUtil.encodeString(identifier.name()),
+ RESTUtil.encodePathSegment(identifier.name()),
"metrics");
}
@@ -130,7 +130,7 @@ public class ResourcePaths {
"namespaces",
pathEncode(identifier.namespace()),
"tables",
- RESTUtil.encodeString(identifier.name()),
+ RESTUtil.encodePathSegment(identifier.name()),
"sign");
}
@@ -149,7 +149,7 @@ public class ResourcePaths {
"namespaces",
pathEncode(ident.namespace()),
"views",
- RESTUtil.encodeString(ident.name()));
+ RESTUtil.encodePathSegment(ident.name()));
}
public String renameView() {
@@ -167,7 +167,7 @@ public class ResourcePaths {
"namespaces",
pathEncode(ident.namespace()),
"tables",
- RESTUtil.encodeString(ident.name()),
+ RESTUtil.encodePathSegment(ident.name()),
"plan");
}
@@ -178,9 +178,9 @@ public class ResourcePaths {
"namespaces",
pathEncode(ident.namespace()),
"tables",
- RESTUtil.encodeString(ident.name()),
+ RESTUtil.encodePathSegment(ident.name()),
"plan",
- RESTUtil.encodeString(planId));
+ RESTUtil.encodePathSegment(planId));
}
public String fetchScanTasks(TableIdentifier ident) {
@@ -190,11 +190,11 @@ public class ResourcePaths {
"namespaces",
pathEncode(ident.namespace()),
"tables",
- RESTUtil.encodeString(ident.name()),
+ RESTUtil.encodePathSegment(ident.name()),
"tasks");
}
private String pathEncode(Namespace ns) {
- return RESTUtil.encodeNamespace(ns, namespaceSeparator);
+ return RESTUtil.encodeNamespaceAsPathSegment(ns, namespaceSeparator);
}
}
diff --git a/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java b/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java
index 8c6dc52b15..b99d7ffb63 100644
--- a/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java
+++ b/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java
@@ -723,22 +723,22 @@ public class RESTCatalogAdapter extends BaseHTTPClient {
}
private static Namespace namespaceFromPathVars(Map<String, String> pathVars) {
- return RESTUtil.decodeNamespace(
+ return RESTUtil.decodeNamespaceAsPathSegment(
pathVars.get("namespace"), NAMESPACE_SEPARATOR_URLENCODED_UTF_8);
}
private static TableIdentifier tableIdentFromPathVars(Map<String, String> pathVars) {
return TableIdentifier.of(
- namespaceFromPathVars(pathVars), RESTUtil.decodeString(pathVars.get("table")));
+ namespaceFromPathVars(pathVars), RESTUtil.decodePathSegment(pathVars.get("table")));
}
private static TableIdentifier viewIdentFromPathVars(Map<String, String> pathVars) {
return TableIdentifier.of(
- namespaceFromPathVars(pathVars), RESTUtil.decodeString(pathVars.get("view")));
+ namespaceFromPathVars(pathVars), RESTUtil.decodePathSegment(pathVars.get("view")));
}
private static String planIDFromPathVars(Map<String, String> pathVars) {
- return RESTUtil.decodeString(pathVars.get("plan-id"));
+ return RESTUtil.decodePathSegment(pathVars.get("plan-id"));
}
private static SnapshotMode snapshotModeFromQueryParams(Map<String, String> queryParams) {
diff --git a/core/src/test/java/org/apache/iceberg/rest/TestHTTPRequest.java b/core/src/test/java/org/apache/iceberg/rest/TestHTTPRequest.java
index 4bf36d300f..2a37f86191 100644
--- a/core/src/test/java/org/apache/iceberg/rest/TestHTTPRequest.java
+++ b/core/src/test/java/org/apache/iceberg/rest/TestHTTPRequest.java
@@ -88,7 +88,17 @@ class TestHTTPRequest {
// absolute path with trailing slash: should be preserved
.path("http://authserver.com/token/")
.build(),
- URI.create("http://authserver.com/token/")));
+ URI.create("http://authserver.com/token/")),
+ Arguments.of(
+ ImmutableHTTPRequest.builder()
+ .baseUri(URI.create("http://localhost:8080"))
+ .method(HTTPRequest.HTTPMethod.GET)
+ // percent-encoded path segments must survive URIBuilder unchanged:
+ // %20 must not become %2520, %7C must not become %257C
+ .path("v1/main%7Cwarehouse/namespaces/sales%20report/tables")
+ .build(),
+ URI.create(
+ "http://localhost:8080/v1/main%7Cwarehouse/namespaces/sales%20report/tables")));
}
@Test
diff --git a/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java b/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java
index d48ac50781..09d0095e71 100644
--- a/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java
+++ b/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java
@@ -4075,6 +4075,28 @@ public class TestRESTCatalog extends CatalogTests<RESTCatalog> {
}
}
+ @Test
+ public void testLoadTableWithSpecialChars() {
+ Namespace ns1 = Namespace.of("ns 1 ?=-+");
+ Namespace ns2 = Namespace.of("ns 1 ?=-+", "ns 2 ?=-+");
+
+ if (requiresNamespaceCreate()) {
+ restCatalog.createNamespace(ns1);
+ restCatalog.createNamespace(ns2);
+ }
+
+ TableIdentifier t1 = TableIdentifier.of(ns2, "table 1 ?=-+");
+
+ restCatalog.buildTable(t1, SCHEMA).create();
+ assertThat(restCatalog.tableExists(t1)).as("Table should exist").isTrue();
+
+ Table table = restCatalog.loadTable(t1);
+
+ String metadataFileLocation =
+ ((HasTableOperations) table).operations().current().metadataFileLocation();
+ assertThat(metadataFileLocation).contains("ns 1 ?=-+/ns 2 ?=-+/table 1 ?=-+");
+ }
+
private RESTCatalog catalog(RESTCatalogAdapter adapter) {
RESTCatalog catalog =
new RESTCatalog(SessionCatalog.SessionContext.createEmpty(), (config) -> adapter);
diff --git a/core/src/test/java/org/apache/iceberg/rest/TestRESTUtil.java b/core/src/test/java/org/apache/iceberg/rest/TestRESTUtil.java
index de30a0e8e1..bfdffe92b3 100644
--- a/core/src/test/java/org/apache/iceberg/rest/TestRESTUtil.java
+++ b/core/src/test/java/org/apache/iceberg/rest/TestRESTUtil.java
@@ -23,10 +23,13 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.util.Map;
+import java.util.stream.Stream;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;
public class TestRESTUtil {
@@ -105,6 +108,98 @@ public class TestRESTUtil {
}
}
+ @ParameterizedTest
+ @ValueSource(strings = {"%1F", "%2D", "%2E", "#", "_"})
+ public void testRoundTripEncodeDecodeNamespaceAsPathSegment(String namespaceSeparator) {
+ Object[][] testCases =
+ new Object[][] {
+ new Object[] {new String[] {"dogs"}, "dogs"},
+ new Object[] {new String[] {"dogs.named.hank"}, "dogs.named.hank"},
+ new Object[] {new String[] {"dogs/named/hank"}, "dogs%2Fnamed%2Fhank"},
+ new Object[] {new String[] {"dogs named hank"}, "dogs%20named%20hank"},
+ new Object[] {new String[] {"dogs+named+hank"}, "dogs%2Bnamed%2Bhank"},
+ new Object[] {
+ new String[] {"dogs", "named", "hank"},
+ String.format("dogs%snamed%shank", namespaceSeparator, namespaceSeparator)
+ },
+ new Object[] {
+ new String[] {"dogs.and.cats", "named", "hank.or.james-westfall"},
+ String.format(
+ "dogs.and.cats%snamed%shank.or.james-westfall",
+ namespaceSeparator, namespaceSeparator),
+ }
+ };
+
+ for (Object[] namespaceWithEncoding : testCases) {
+ String[] levels = (String[]) namespaceWithEncoding[0];
+ String encodedNs = (String) namespaceWithEncoding[1];
+
+ Namespace namespace = Namespace.of(levels);
+
+ assertThat(RESTUtil.encodeNamespaceAsPathSegment(namespace, namespaceSeparator))
+ .isEqualTo(encodedNs);
+
+ assertThat(RESTUtil.decodeNamespaceAsPathSegment(encodedNs, namespaceSeparator))
+ .isEqualTo(namespace);
+ }
+ }
+
+ @Test
+ public void testDecodeNamespacePathSegmentPreservesPlusSign() {
+ String separator = "%1F";
+ Namespace expected = Namespace.of("a+b", "c+d");
+ // Both encoded forms are valid
+ assertThat(RESTUtil.decodeNamespaceAsPathSegment("a+b%1Fc+d", separator)).isEqualTo(expected);
+ assertThat(RESTUtil.decodeNamespaceAsPathSegment("a%2Bb%1Fc%2Bd", separator))
+ .isEqualTo(expected);
+ }
+
+ @Test
+ public void encodePathAsOldAndNewClientDecodeAsOldServer() {
+ String input = " +%20";
+
+ // old Java client would call encodeString
+ String encodedOldJava = RESTUtil.encodeString(input);
+ assertThat(encodedOldJava).isEqualTo("+%2B%2520");
+
+ // new Java client would call encodePathSegment
+ String encodedNewJava = RESTUtil.encodePathSegment(input);
+ assertThat(encodedNewJava).isEqualTo("%20%2B%2520");
+
+ // another client (e.g. Iceberg Go) would encode using strict RFC 3986, "+" is not
+ // percent-encoded
+ String encodedOther = "%20+%2520";
+
+ // old server would decode with decodeString; should work for both old and new Java clients,
+ // but not for clients sending a literal "+"
+ assertThat(RESTUtil.decodeString(encodedOldJava)).isEqualTo(input);
+ assertThat(RESTUtil.decodeString(encodedNewJava)).isEqualTo(input);
+ assertThat(RESTUtil.decodeString(encodedOther)).isNotEqualTo(input).isEqualTo(" %20");
+ }
+
+ @Test
+ public void encodePathAsOldAndNewClientDecodeAsNewServer() {
+ String input = " +%20";
+
+ // old Java client would call encodeString
+ String encodedOldJava = RESTUtil.encodeString(input);
+ assertThat(encodedOldJava).isEqualTo("+%2B%2520");
+
+ // new Java client would call encodePathSegment
+ String encodedNewJava = RESTUtil.encodePathSegment(input);
+ assertThat(encodedNewJava).isEqualTo("%20%2B%2520");
+
+ // another client (e.g. Iceberg Go) would encode using strict RFC 3986, "+" is not
+ // percent-encoded
+ String encodedOther = "%20+%2520";
+
+ // new server would decode with decodePathSegment; should work for both new Java clients and
+ // clients sending a literal "+", but not for old Java clients
+ assertThat(RESTUtil.decodePathSegment(encodedOldJava)).isNotEqualTo(input).isEqualTo("++%20");
+ assertThat(RESTUtil.decodePathSegment(encodedNewJava)).isEqualTo(input);
+ assertThat(RESTUtil.decodePathSegment(encodedOther)).isEqualTo(input);
+ }
+
@Test
public void encodeAsOldClientAndDecodeAsNewServer() {
Namespace namespace = Namespace.of("first", "second", "third");
@@ -145,12 +240,50 @@ public class TestRESTUtil {
@SuppressWarnings("checkstyle:AvoidEscapedUnicodeCharacters")
public void testOAuth2URLEncoding() {
// from OAuth2, RFC 6749 Appendix B.
+ // encodeString uses form encoding: space -> +
String utf8 = "\u0020\u0025\u0026\u002B\u00A3\u20AC";
String expected = "+%25%26%2B%C2%A3%E2%82%AC";
assertThat(RESTUtil.encodeString(utf8)).isEqualTo(expected);
}
+ @SuppressWarnings("checkstyle:AvoidEscapedUnicodeCharacters")
+ static Stream<Arguments> pathSegmentEncodingCases() {
+ return Stream.of(
+ Arguments.of("simple", "simple"),
+ Arguments.of("a b", "a%20b"),
+ Arguments.of("a+b", "a%2Bb"),
+ Arguments.of("a/b", "a%2Fb"),
+ Arguments.of("a+b c/d", "a%2Bb%20c%2Fd"),
+ Arguments.of("caf\u00e9", "caf%C3%A9"),
+ Arguments.of("\u0020\u0025\u0026\u002B\u00A3\u20AC", "%20%25%26%2B%C2%A3%E2%82%AC"));
+ }
+
+ @ParameterizedTest
+ @MethodSource("pathSegmentEncodingCases")
+ public void testRoundTripEncodeDecodePathSegment(String input, String expectedEncoded) {
+ String actual = RESTUtil.encodePathSegment(input);
+ assertThat(actual).isEqualTo(expectedEncoded);
+ assertThat(RESTUtil.decodePathSegment(actual)).isEqualTo(input);
+ }
+
+ @Test
+ public void testDecodePathSegmentPreservesPlusSign() {
+ // Both encoded forms are valid
+ assertThat(RESTUtil.decodePathSegment("a%2Bb%2Bc%2Bd")).isEqualTo("a+b+c+d");
+ assertThat(RESTUtil.decodePathSegment("a+b+c+d")).isEqualTo("a+b+c+d");
+ }
+
+ @Test
+ public void testPathSegmentEncodeDecodeNull() {
+ assertThatThrownBy(() -> RESTUtil.encodePathSegment(null))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Invalid string to encode: null");
+ assertThatThrownBy(() -> RESTUtil.decodePathSegment(null))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Invalid string to decode: null");
+ }
+
@Test
@SuppressWarnings("checkstyle:AvoidEscapedUnicodeCharacters")
public void testOAuth2FormDataEncoding() {
@@ -264,6 +397,22 @@ public class TestRESTUtil {
.isInstanceOf(IllegalArgumentException.class)
.hasMessage(errorMsg);
+ assertThatThrownBy(() -> RESTUtil.encodeNamespaceAsPathSegment(Namespace.empty(), null))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(errorMsg);
+
+ assertThatThrownBy(() -> RESTUtil.encodeNamespaceAsPathSegment(Namespace.empty(), ""))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(errorMsg);
+
+ assertThatThrownBy(() -> RESTUtil.decodeNamespaceAsPathSegment("namespace", null))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(errorMsg);
+
+ assertThatThrownBy(() -> RESTUtil.decodeNamespaceAsPathSegment("namespace", ""))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(errorMsg);
+
assertThatThrownBy(() -> RESTUtil.namespaceToQueryParam(Namespace.empty(), null))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage(errorMsg);
diff --git a/core/src/test/java/org/apache/iceberg/rest/TestResourcePaths.java b/core/src/test/java/org/apache/iceberg/rest/TestResourcePaths.java
index c4e3f763fc..be3c705ea0 100644
--- a/core/src/test/java/org/apache/iceberg/rest/TestResourcePaths.java
+++ b/core/src/test/java/org/apache/iceberg/rest/TestResourcePaths.java
@@ -152,6 +152,23 @@ public class TestResourcePaths {
assertThat(RESTUtil.decodeNamespace(newEncodedSeparator, newSeparator)).isEqualTo(namespace);
}
+ @Test
+ public void nestedNamespaceAsPathSegmentWithCustomSeparator() {
+ Namespace namespace = Namespace.of("first second", "third");
+ String separator = RESTCatalogAdapter.NAMESPACE_SEPARATOR_URLENCODED_UTF_8;
+
+ ResourcePaths pathsWithCustomSeparator =
+ ResourcePaths.forCatalogProperties(
+ ImmutableMap.of(RESTCatalogProperties.NAMESPACE_SEPARATOR, separator));
+
+ String actual = pathsWithCustomSeparator.namespace(namespace);
+ assertThat(actual)
+ .contains(RESTUtil.encodeNamespaceAsPathSegment(namespace, separator))
+ .contains(separator)
+ .contains("%20")
+ .doesNotContain("+");
+ }
+
@Test
public void testNamespaceProperties() {
Namespace ns = Namespace.of("ns");
@@ -176,6 +193,22 @@ public class TestResourcePaths {
assertThat(withoutPrefix.namespaceProperties(ns)).isEqualTo("v1/namespaces/n%1Fs/properties");
}
+ @Test
+ public void testNamespacePropertiesWithSpace() {
+ Namespace ns = Namespace.of("n s");
+ assertThat(withPrefix.namespaceProperties(ns))
+ .isEqualTo("v1/ws/catalog/namespaces/n%20s/properties");
+ assertThat(withoutPrefix.namespaceProperties(ns)).isEqualTo("v1/namespaces/n%20s/properties");
+ }
+
+ @Test
+ public void testNamespacePropertiesWithPlusSign() {
+ Namespace ns = Namespace.of("n+s");
+ assertThat(withPrefix.namespaceProperties(ns))
+ .isEqualTo("v1/ws/catalog/namespaces/n%2Bs/properties");
+ assertThat(withoutPrefix.namespaceProperties(ns)).isEqualTo("v1/namespaces/n%2Bs/properties");
+ }
+
@Test
public void testTables() {
Namespace ns = Namespace.of("ns");
@@ -197,6 +230,20 @@ public class TestResourcePaths {
assertThat(withoutPrefix.tables(ns)).isEqualTo("v1/namespaces/n%1Fs/tables");
}
+ @Test
+ public void testTablesWithSpace() {
+ Namespace ns = Namespace.of("n s");
+ assertThat(withPrefix.tables(ns)).isEqualTo("v1/ws/catalog/namespaces/n%20s/tables");
+ assertThat(withoutPrefix.tables(ns)).isEqualTo("v1/namespaces/n%20s/tables");
+ }
+
+ @Test
+ public void testTablesWithPlusSign() {
+ Namespace ns = Namespace.of("n+s");
+ assertThat(withPrefix.tables(ns)).isEqualTo("v1/ws/catalog/namespaces/n%2Bs/tables");
+ assertThat(withoutPrefix.tables(ns)).isEqualTo("v1/namespaces/n%2Bs/tables");
+ }
+
@Test
public void testTable() {
TableIdentifier ident = TableIdentifier.of("ns", "table");
@@ -218,6 +265,62 @@ public class TestResourcePaths {
assertThat(withoutPrefix.table(ident)).isEqualTo("v1/namespaces/n%1Fs/tables/table");
}
+ @Test
+ public void testNamespaceWithSpace() {
+ Namespace ns = Namespace.of("n s");
+ assertThat(withPrefix.namespace(ns)).isEqualTo("v1/ws/catalog/namespaces/n%20s");
+ assertThat(withoutPrefix.namespace(ns)).isEqualTo("v1/namespaces/n%20s");
+ }
+
+ @Test
+ public void testMultipartNamespaceWithSpace() {
+ Namespace ns = Namespace.of("n s", "a b");
+ assertThat(withPrefix.namespace(ns)).isEqualTo("v1/ws/catalog/namespaces/n%20s%1Fa%20b");
+ assertThat(withoutPrefix.namespace(ns)).isEqualTo("v1/namespaces/n%20s%1Fa%20b");
+ }
+
+ @Test
+ public void testNamespaceWithPlusSign() {
+ Namespace ns = Namespace.of("n+s");
+ assertThat(withPrefix.namespace(ns)).isEqualTo("v1/ws/catalog/namespaces/n%2Bs");
+ assertThat(withoutPrefix.namespace(ns)).isEqualTo("v1/namespaces/n%2Bs");
+ }
+
+ @Test
+ public void testMultipartNamespaceWithPlusSign() {
+ Namespace ns = Namespace.of("n+s", "a+b");
+ assertThat(withPrefix.namespace(ns)).isEqualTo("v1/ws/catalog/namespaces/n%2Bs%1Fa%2Bb");
+ assertThat(withoutPrefix.namespace(ns)).isEqualTo("v1/namespaces/n%2Bs%1Fa%2Bb");
+ }
+
+ @Test
+ public void testTableWithSpace() {
+ TableIdentifier ident = TableIdentifier.of("ns", "my table");
+ assertThat(withPrefix.table(ident)).isEqualTo("v1/ws/catalog/namespaces/ns/tables/my%20table");
+ assertThat(withoutPrefix.table(ident)).isEqualTo("v1/namespaces/ns/tables/my%20table");
+ }
+
+ @Test
+ public void testTableWithPlusSign() {
+ TableIdentifier ident = TableIdentifier.of("ns", "a+b");
+ assertThat(withPrefix.table(ident)).isEqualTo("v1/ws/catalog/namespaces/ns/tables/a%2Bb");
+ assertThat(withoutPrefix.table(ident)).isEqualTo("v1/namespaces/ns/tables/a%2Bb");
+ }
+
+ @Test
+ public void testViewWithSpace() {
+ TableIdentifier ident = TableIdentifier.of("ns", "my view");
+ assertThat(withPrefix.view(ident)).isEqualTo("v1/ws/catalog/namespaces/ns/views/my%20view");
+ assertThat(withoutPrefix.view(ident)).isEqualTo("v1/namespaces/ns/views/my%20view");
+ }
+
+ @Test
+ public void testViewWithPlusSign() {
+ TableIdentifier ident = TableIdentifier.of("ns", "a+b");
+ assertThat(withPrefix.view(ident)).isEqualTo("v1/ws/catalog/namespaces/ns/views/a%2Bb");
+ assertThat(withoutPrefix.view(ident)).isEqualTo("v1/namespaces/ns/views/a%2Bb");
+ }
+
@Test
public void testRegister() {
Namespace ns = Namespace.of("ns");
@@ -225,6 +328,20 @@ public class TestResourcePaths {
assertThat(withoutPrefix.register(ns)).isEqualTo("v1/namespaces/ns/register");
}
+ @Test
+ public void testRegisterWithSpace() {
+ Namespace ns = Namespace.of("n s");
+ assertThat(withPrefix.register(ns)).isEqualTo("v1/ws/catalog/namespaces/n%20s/register");
+ assertThat(withoutPrefix.register(ns)).isEqualTo("v1/namespaces/n%20s/register");
+ }
+
+ @Test
+ public void testRegisterWithPlusSign() {
+ Namespace ns = Namespace.of("n+s");
+ assertThat(withPrefix.register(ns)).isEqualTo("v1/ws/catalog/namespaces/n%2Bs/register");
+ assertThat(withoutPrefix.register(ns)).isEqualTo("v1/namespaces/n%2Bs/register");
+ }
+
@Test
public void views() {
Namespace ns = Namespace.of("ns");
@@ -246,6 +363,20 @@ public class TestResourcePaths {
assertThat(withoutPrefix.views(ns)).isEqualTo("v1/namespaces/n%1Fs/views");
}
+ @Test
+ public void viewsWithSpace() {
+ Namespace ns = Namespace.of("n s");
+ assertThat(withPrefix.views(ns)).isEqualTo("v1/ws/catalog/namespaces/n%20s/views");
+ assertThat(withoutPrefix.views(ns)).isEqualTo("v1/namespaces/n%20s/views");
+ }
+
+ @Test
+ public void viewsWithPlusSign() {
+ Namespace ns = Namespace.of("n+s");
+ assertThat(withPrefix.views(ns)).isEqualTo("v1/ws/catalog/namespaces/n%2Bs/views");
+ assertThat(withoutPrefix.views(ns)).isEqualTo("v1/namespaces/n%2Bs/views");
+ }
+
@Test
public void view() {
TableIdentifier ident = TableIdentifier.of("ns", "view-name");
@@ -275,6 +406,22 @@ public class TestResourcePaths {
assertThat(withoutPrefix.registerView(ns)).isEqualTo("v1/namespaces/ns/register-view");
}
+ @Test
+ public void testRegisterViewWithSpace() {
+ Namespace ns = Namespace.of("n s");
+ assertThat(withPrefix.registerView(ns))
+ .isEqualTo("v1/ws/catalog/namespaces/n%20s/register-view");
+ assertThat(withoutPrefix.registerView(ns)).isEqualTo("v1/namespaces/n%20s/register-view");
+ }
+
+ @Test
+ public void testRegisterViewWithPlusSign() {
+ Namespace ns = Namespace.of("n+s");
+ assertThat(withPrefix.registerView(ns))
+ .isEqualTo("v1/ws/catalog/namespaces/n%2Bs/register-view");
+ assertThat(withoutPrefix.registerView(ns)).isEqualTo("v1/namespaces/n%2Bs/register-view");
+ }
+
@Test
public void planEndpointPath() {
TableIdentifier tableId = TableIdentifier.of("test_namespace", "test_table");
@@ -292,6 +439,24 @@ public class TestResourcePaths {
.isEqualTo("v1/namespaces/db%1Fschema/tables/my_table/plan");
}
+ @Test
+ public void testPlanTableScanWithSpace() {
+ TableIdentifier ident = TableIdentifier.of("n s", "my table");
+ assertThat(withPrefix.planTableScan(ident))
+ .isEqualTo("v1/ws/catalog/namespaces/n%20s/tables/my%20table/plan");
+ assertThat(withoutPrefix.planTableScan(ident))
+ .isEqualTo("v1/namespaces/n%20s/tables/my%20table/plan");
+ }
+
+ @Test
+ public void testPlanTableScanWithPlusSign() {
+ TableIdentifier ident = TableIdentifier.of("n+s", "a+b");
+ assertThat(withPrefix.planTableScan(ident))
+ .isEqualTo("v1/ws/catalog/namespaces/n%2Bs/tables/a%2Bb/plan");
+ assertThat(withoutPrefix.planTableScan(ident))
+ .isEqualTo("v1/namespaces/n%2Bs/tables/a%2Bb/plan");
+ }
+
@Test
public void fetchScanTasksPath() {
TableIdentifier tableId = TableIdentifier.of("test_namespace", "test_table");
@@ -309,6 +474,24 @@ public class TestResourcePaths {
.isEqualTo("v1/namespaces/db%1Fschema/tables/my_table/tasks");
}
+ @Test
+ public void testFetchScanTasksWithSpace() {
+ TableIdentifier ident = TableIdentifier.of("n s", "my table");
+ assertThat(withPrefix.fetchScanTasks(ident))
+ .isEqualTo("v1/ws/catalog/namespaces/n%20s/tables/my%20table/tasks");
+ assertThat(withoutPrefix.fetchScanTasks(ident))
+ .isEqualTo("v1/namespaces/n%20s/tables/my%20table/tasks");
+ }
+
+ @Test
+ public void testFetchScanTasksWithPlusSign() {
+ TableIdentifier ident = TableIdentifier.of("n+s", "a+b");
+ assertThat(withPrefix.fetchScanTasks(ident))
+ .isEqualTo("v1/ws/catalog/namespaces/n%2Bs/tables/a%2Bb/tasks");
+ assertThat(withoutPrefix.fetchScanTasks(ident))
+ .isEqualTo("v1/namespaces/n%2Bs/tables/a%2Bb/tasks");
+ }
+
@Test
public void cancelPlanEndpointPath() {
TableIdentifier tableId = TableIdentifier.of("test_namespace", "test_table");
@@ -321,8 +504,8 @@ public class TestResourcePaths {
// The planId contains a space which needs to be encoded
String spaceSeparatedPlanId = "plan with spaces";
- // The expected encoded version of the planId
- String encodedPlanId = "plan+with+spaces";
+ // The expected encoded version of the planId (RFC 3986: space -> %20)
+ String encodedPlanId = "plan%20with%20spaces";
assertThat(withPrefix.plan(tableId, spaceSeparatedPlanId))
.isEqualTo(
@@ -338,6 +521,15 @@ public class TestResourcePaths {
.isEqualTo("v1/namespaces/db%1Fschema/tables/my_table/plan/plan-xyz-789");
}
+ @Test
+ public void cancelPlanEndpointPathWithPlusSign() {
+ TableIdentifier tableId = TableIdentifier.of("ns", "table");
+ assertThat(withPrefix.plan(tableId, "plan+id"))
+ .isEqualTo("v1/ws/catalog/namespaces/ns/tables/table/plan/plan%2Bid");
+ assertThat(withoutPrefix.plan(tableId, "plan+id"))
+ .isEqualTo("v1/namespaces/ns/tables/table/plan/plan%2Bid");
+ }
+
@Test
public void testRemoteSign() {
TableIdentifier tableId = TableIdentifier.of("test_namespace", "test_table");
@@ -353,4 +545,46 @@ public class TestResourcePaths {
assertThat(withoutPrefix.remoteSign(complexId))
.isEqualTo("v1/namespaces/db%1Fschema/tables/my_table/sign");
}
+
+ @Test
+ public void testRemoteSignWithSpace() {
+ TableIdentifier ident = TableIdentifier.of("n s", "my table");
+ assertThat(withPrefix.remoteSign(ident))
+ .isEqualTo("v1/ws/catalog/namespaces/n%20s/tables/my%20table/sign");
+ assertThat(withoutPrefix.remoteSign(ident))
+ .isEqualTo("v1/namespaces/n%20s/tables/my%20table/sign");
+ }
+
+ @Test
+ public void testRemoteSignWithPlusSign() {
+ TableIdentifier ident = TableIdentifier.of("n+s", "a+b");
+ assertThat(withPrefix.remoteSign(ident))
+ .isEqualTo("v1/ws/catalog/namespaces/n%2Bs/tables/a%2Bb/sign");
+ assertThat(withoutPrefix.remoteSign(ident)).isEqualTo("v1/namespaces/n%2Bs/tables/a%2Bb/sign");
+ }
+
+ @Test
+ public void testMetrics() {
+ TableIdentifier ident = TableIdentifier.of("ns", "table");
+ assertThat(withPrefix.metrics(ident))
+ .isEqualTo("v1/ws/catalog/namespaces/ns/tables/table/metrics");
+ assertThat(withoutPrefix.metrics(ident)).isEqualTo("v1/namespaces/ns/tables/table/metrics");
+ }
+
+ @Test
+ public void testMetricsWithSpace() {
+ TableIdentifier ident = TableIdentifier.of("n s", "my table");
+ assertThat(withPrefix.metrics(ident))
+ .isEqualTo("v1/ws/catalog/namespaces/n%20s/tables/my%20table/metrics");
+ assertThat(withoutPrefix.metrics(ident))
+ .isEqualTo("v1/namespaces/n%20s/tables/my%20table/metrics");
+ }
+
+ @Test
+ public void testMetricsWithPlusSign() {
+ TableIdentifier ident = TableIdentifier.of("n+s", "a+b");
+ assertThat(withPrefix.metrics(ident))
+ .isEqualTo("v1/ws/catalog/namespaces/n%2Bs/tables/a%2Bb/metrics");
+ assertThat(withoutPrefix.metrics(ident)).isEqualTo("v1/namespaces/n%2Bs/tables/a%2Bb/metrics");
+ }
}