This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new de25ce3bd8 [#13781] fix(client): Encode spaces as %20 in REST path
segments (#13784)
de25ce3bd8 is described below
commit de25ce3bd82771d746253606156877988caeafb1
Author: Yuhui <[email protected]>
AuthorDate: Tue Oct 13 13:42:22 2026 +0800
[#13781] fix(client): Encode spaces as %20 in REST path segments (#13784)
### What changes were proposed in this pull request?
`RESTUtils.encodeString` now encodes a space as `%20` instead of `+`. A
literal `+` in the input is
still encoded as `%2B`, so the result is valid as a path segment and as
a query or form value.
### Why are the changes needed?
`URLEncoder` follows form encoding and turns a space into `+`, which the
server keeps as a literal
`+` in a path segment. Objects whose names contain a space could be
listed but not loaded, for
example a JDBC table named `with space` is shown by the Trino connector
but cannot be queried.
Fix: #13781
### Does this PR introduce _any_ user-facing change?
Objects whose names contain a space can be accessed through the Java
client.
### How was this patch tested?
- Updated `TestRESTUtils.testEncodeString`.
- Added `TestTableOperations.testLoadTableWithSpaceInName`, which loads
a table through the server
with a path built by `RESTUtils.encodeString`; it fails without this
change.
---
.../gravitino/client/TestRelationalCatalog.java | 24 ++++++++++++++++++++
.../java/org/apache/gravitino/rest/RESTUtils.java | 24 ++++++++++++++------
.../org/apache/gravitino/rest/TestRESTUtils.java | 10 +++++++--
.../server/web/rest/TestTableOperations.java | 26 ++++++++++++++++++++++
4 files changed, 75 insertions(+), 9 deletions(-)
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/TestRelationalCatalog.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/TestRelationalCatalog.java
index 64feaa121c..7c11f94707 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/TestRelationalCatalog.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/TestRelationalCatalog.java
@@ -816,6 +816,30 @@ public class TestRelationalCatalog extends TestBase {
Assertions.assertTrue(ex.getMessage().contains("table not found"));
}
+ @Test
+ public void testLoadTableWithSpaceInName() throws JsonProcessingException {
+ NameIdentifier tableId = NameIdentifier.of("schema1", "table 1");
+ Namespace fullNamespace = Namespace.of(metalakeName, catalogName,
tableId.namespace().level(0));
+ // MockServer matches the raw request path, so the space must be sent as
%20.
+ String tablePath =
+ withSlash(RelationalCatalog.formatTableRequestPath(fullNamespace) +
"/table%201");
+ ColumnDTO[] columns =
+ new ColumnDTO[] {createMockColumn("col1", Types.StringType.get(),
"comment1")};
+ TableDTO expectedTable =
+ createMockTable(
+ "table 1",
+ columns,
+ "comment",
+ Collections.emptyMap(),
+ new Partitioning[0],
+ DistributionDTO.NONE,
+ SortOrderDTO.EMPTY_SORT);
+ buildMockResource(Method.GET, tablePath, null, new
TableResponse(expectedTable), SC_OK);
+
+ Table table = catalog.asTableCatalog().loadTable(tableId);
+ Assertions.assertEquals("table 1", table.name());
+ }
+
@Test
public void testRenameTable() throws JsonProcessingException {
NameIdentifier tableId = NameIdentifier.of("schema1", "table1");
diff --git a/common/src/main/java/org/apache/gravitino/rest/RESTUtils.java
b/common/src/main/java/org/apache/gravitino/rest/RESTUtils.java
index 8b95669cee..bdcb873f8d 100644
--- a/common/src/main/java/org/apache/gravitino/rest/RESTUtils.java
+++ b/common/src/main/java/org/apache/gravitino/rest/RESTUtils.java
@@ -44,6 +44,8 @@ public class RESTUtils {
private static final Joiner.MapJoiner FORM_JOINER =
Joiner.on("&").withKeyValueSeparator("=");
private static final Splitter.MapSplitter FORM_SPLITTER =
Splitter.on("&").withKeyValueSeparator("=");
+ private static final String FORM_ENCODED_SPACE = "+";
+ private static final String PERCENT_ENCODED_SPACE = "%20";
private RESTUtils() {}
@@ -75,7 +77,7 @@ public class RESTUtils {
ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
formData.forEach(
(key, value) ->
- builder.put(encodeString(String.valueOf(key)),
encodeString(String.valueOf(value))));
+ builder.put(formEncode(String.valueOf(key)),
formEncode(String.valueOf(value))));
return FORM_JOINER.join(builder.build());
}
@@ -93,7 +95,8 @@ public class RESTUtils {
}
/**
- * URL encode a string.
+ * URL encode a string. Spaces are encoded as {@code %20}, so the result is
valid both as a path
+ * segment and as a query or form value.
*
* @param toEncode The string to encode.
* @return The URL encoded string.
@@ -102,11 +105,9 @@ public class RESTUtils {
*/
public static String encodeString(String toEncode) {
Preconditions.checkArgument(toEncode != null, "Invalid string to encode:
null");
- try {
- return URLEncoder.encode(toEncode, StandardCharsets.UTF_8.name());
- } catch (UnsupportedEncodingException e) {
- throw new GravitinoRuntimeException("Failed to encode string: %s",
toEncode);
- }
+ // URLEncoder follows the form encoding and turns a space into '+', which
is a literal '+'
+ // in a path segment. A literal '+' in the input is already encoded as %2B
at this point.
+ return formEncode(toEncode).replace(FORM_ENCODED_SPACE,
PERCENT_ENCODED_SPACE);
}
/**
@@ -169,4 +170,13 @@ public class RESTUtils {
}
throw new IOException("No available port in the range: " + portRangeStart
+ ":" + portRangeEnd);
}
+
+ // Encodes in application/x-www-form-urlencoded format, in which a space
becomes '+'.
+ private static String formEncode(String toEncode) {
+ try {
+ return URLEncoder.encode(toEncode, StandardCharsets.UTF_8.name());
+ } catch (UnsupportedEncodingException e) {
+ throw new GravitinoRuntimeException("Failed to encode string: %s",
toEncode);
+ }
+ }
}
diff --git a/common/src/test/java/org/apache/gravitino/rest/TestRESTUtils.java
b/common/src/test/java/org/apache/gravitino/rest/TestRESTUtils.java
index 6402a60a5d..465a3331df 100644
--- a/common/src/test/java/org/apache/gravitino/rest/TestRESTUtils.java
+++ b/common/src/test/java/org/apache/gravitino/rest/TestRESTUtils.java
@@ -45,6 +45,11 @@ class TestRESTUtils {
String expected = "key1=value1&key2=value2";
assertEquals(expected, RESTUtils.encodeFormData(formData));
+ // Form data keeps the form encoding, in which a space becomes '+'.
+ assertEquals(
+ "scope=read+write&secret=a%2Bb",
+ RESTUtils.encodeFormData(ImmutableMap.of("scope", "read write",
"secret", "a+b")));
+
Map<Object, Object> emptyFormData = ImmutableMap.of();
assertEquals("", RESTUtils.encodeFormData(emptyFormData));
}
@@ -67,8 +72,9 @@ class TestRESTUtils {
void testEncodeString() {
assertEquals("test", RESTUtils.encodeString("test"));
assertEquals("", RESTUtils.encodeString(""));
- /* not %20 as you might expect */
- assertEquals("hello+world", RESTUtils.encodeString("hello world"));
+ assertEquals("hello%20world", RESTUtils.encodeString("hello world"));
+ assertEquals("a%2Bb", RESTUtils.encodeString("a+b"));
+ assertEquals("a+b c", RESTUtils.decodeString(RESTUtils.encodeString("a+b
c")));
assertThrows(IllegalArgumentException.class, () ->
RESTUtils.encodeString(null));
}
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTableOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTableOperations.java
index 4d11ccb61e..2c8d0fdff1 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTableOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTableOperations.java
@@ -508,6 +508,32 @@ public class TestTableOperations extends
BaseOperationsTest {
Assertions.assertArrayEquals(partitioningWithAssignments,
tableResp.getTable().partitioning());
}
+ @Test
+ public void testLoadTableWithSpaceInName() {
+ when(dispatcher.loadTable(any()))
+ .thenAnswer(
+ invocation -> {
+ NameIdentifier ident = invocation.getArgument(0);
+ return mockTable(
+ ident.name(),
+ new Column[] {mockColumn("col1", Types.StringType.get())},
+ "mock comment",
+ ImmutableMap.of(),
+ new Transform[0]);
+ });
+
+ // The client builds request paths with RESTUtils.encodeString, so the
server must decode the
+ // encoded name back to the original one.
+ try (Response resp =
+ target(tablePath(metalake, catalog, schema) +
RESTUtils.encodeString("table 1"))
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .get()) {
+ Assertions.assertEquals(Response.Status.OK.getStatusCode(),
resp.getStatus());
+ Assertions.assertEquals("table 1",
resp.readEntity(TableResponse.class).getTable().name());
+ }
+ }
+
@Test
public void testLoadTable() {
Column[] columns =
