This is an automated email from the ASF dual-hosted git repository. github-actions[bot] pushed a commit to branch cherry-pick-de25ce3b-to-branch-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit d767645a5a54f9067834f859b4433a7aee00f5f9 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 =
