This is an automated email from the ASF dual-hosted git repository.
roryqi 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 3b01c6ed10 [#12887] fix(server): Validate statistic name length
(#12889)
3b01c6ed10 is described below
commit 3b01c6ed10cf64f7d3a8a87607955c1d590ec4ee
Author: roryqi <[email protected]>
AuthorDate: Thu Sep 3 21:33:19 2026 +0800
[#12887] fix(server): Validate statistic name length (#12889)
### What changes were proposed in this pull request?
- Add a 128-character limit for statistic names, matching the storage
schema.
- Validate statistic names before table or partition statistics are
persisted.
- Redact overlong statistic names from error context.
- Add tests for the accepted boundary and overlong-name rejection.
### Why are the changes needed?
Overlong statistic names currently reach the storage layer and produce a
500 response. The resulting error also echoes the complete offending
name.
This change rejects such requests with a clear 400 response before
persistence.
Fix: #12887
### Does this PR introduce _any_ user-facing change?
Yes. Statistic names longer than 128 characters now return a 400 error
identifying the length limit, without echoing the complete name.
### How was this patch tested?
- `./gradlew :api:spotlessCheck :server:spotlessCheck`
- `./gradlew :server:test --tests
org.apache.gravitino.server.web.rest.TestStatisticOperations`
---
.../java/org/apache/gravitino/stats/Statistic.java | 3 ++
.../server/web/rest/StatisticOperations.java | 45 +++++++++--------
.../server/web/rest/TestStatisticOperations.java | 58 ++++++++++++++++++++++
3 files changed, 86 insertions(+), 20 deletions(-)
diff --git a/api/src/main/java/org/apache/gravitino/stats/Statistic.java
b/api/src/main/java/org/apache/gravitino/stats/Statistic.java
index e54c82a0cb..c7e5f64aba 100644
--- a/api/src/main/java/org/apache/gravitino/stats/Statistic.java
+++ b/api/src/main/java/org/apache/gravitino/stats/Statistic.java
@@ -33,6 +33,9 @@ public interface Statistic extends Auditable {
/** The prefix for custom statistics. Custom statistics are user-defined
statistics. */
String CUSTOM_PREFIX = "custom-";
+ /** The maximum number of characters allowed in a statistic name. */
+ int MAX_NAME_LENGTH = 128;
+
/**
* Get the name of the statistic.
*
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/StatisticOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/StatisticOperations.java
index 79887df7dd..4d750aa05e 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/StatisticOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/StatisticOperations.java
@@ -180,13 +180,7 @@ public class StatisticOperations {
Map<String, StatisticValue<?>> statisticMaps = Maps.newHashMap();
for (Map.Entry<String, StatisticValue<?>> entry :
request.getUpdates().entrySet()) {
- // Current we only support custom statistics
- if (!entry.getKey().startsWith(Statistic.CUSTOM_PREFIX)) {
- throw new IllegalStatisticNameException(
- "Statistic name must start with %s , but got: %s",
- Statistic.CUSTOM_PREFIX, entry.getKey());
- }
-
+ validateStatisticName(entry.getKey());
statisticMaps.put(entry.getKey(), entry.getValue());
}
@@ -392,18 +386,7 @@ public class StatisticOperations {
List<PartitionStatisticsUpdateDTO> updates = request.getUpdates();
for (PartitionStatisticsUpdateDTO update : updates) {
- update
- .statistics()
- .keySet()
- .forEach(
- statistic -> {
- if (!statistic.startsWith(Statistic.CUSTOM_PREFIX)) {
- // Current we only support custom statistics
- throw new IllegalStatisticNameException(
- "Statistic name must start with %s, but got: %s",
- Statistic.CUSTOM_PREFIX, statistic);
- }
- });
+
update.statistics().keySet().forEach(StatisticOperations::validateStatisticName);
}
MetadataObjectUtil.checkMetadataObject(metalake, object);
@@ -531,7 +514,29 @@ public class StatisticOperations {
return "";
}
- return StringUtils.join(request.getUpdates().keySet(), ",");
+ return request.getUpdates().keySet().stream()
+ .map(StatisticOperations::formatStatisticName)
+ .collect(Collectors.joining(","));
+ }
+
+ private static String formatStatisticName(String statisticName) {
+ if (statisticName != null && statisticName.length() >
Statistic.MAX_NAME_LENGTH) {
+ return String.format("<statistic name exceeds %d characters>",
Statistic.MAX_NAME_LENGTH);
+ }
+ return statisticName;
+ }
+
+ private static void validateStatisticName(String statisticName) {
+ if (statisticName.length() > Statistic.MAX_NAME_LENGTH) {
+ throw new IllegalStatisticNameException(
+ "Statistic name must not exceed %d characters",
Statistic.MAX_NAME_LENGTH);
+ }
+
+ // Currently we only support custom statistics.
+ if (!statisticName.startsWith(Statistic.CUSTOM_PREFIX)) {
+ throw new IllegalStatisticNameException(
+ "Statistic name must start with %s, but got: %s",
Statistic.CUSTOM_PREFIX, statisticName);
+ }
}
private static String getPartitionNames(PartitionStatisticsUpdateRequest
request) {
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestStatisticOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestStatisticOperations.java
index da591459de..887359c177 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestStatisticOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestStatisticOperations.java
@@ -38,6 +38,7 @@ import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.core.Application;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
+import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.reflect.FieldUtils;
import org.apache.gravitino.Config;
import org.apache.gravitino.GravitinoEnv;
@@ -370,6 +371,63 @@ public class TestStatisticOperations extends
BaseOperationsTest {
UnmodifiableStatisticException.class.getSimpleName(),
errorResp4.getType());
}
+ @Test
+ public void testUpdateTableStatisticsNameLength() {
+ String maximumLengthName =
+ Statistic.CUSTOM_PREFIX
+ + StringUtils.repeat("x", Statistic.MAX_NAME_LENGTH -
Statistic.CUSTOM_PREFIX.length());
+ StatisticsUpdateRequest validRequest =
+ new StatisticsUpdateRequest(Map.of(maximumLengthName,
StatisticValues.longValue(1L)));
+ MetadataObject tableObject =
+ MetadataObjects.parse(
+ String.format("%s.%s.%s", catalog, schema, table),
MetadataObject.Type.TABLE);
+ when(tableDispatcher.tableExists(any())).thenReturn(true);
+
+ Response validResponse =
+ target(
+ "/metalakes/"
+ + metalake
+ + "/objects/"
+ + tableObject.type()
+ + "/"
+ + tableObject.fullName()
+ + "/statistics")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(entity(validRequest, MediaType.APPLICATION_JSON_TYPE));
+ Assertions.assertEquals(Response.Status.OK.getStatusCode(),
validResponse.getStatus());
+
+ String longName = maximumLengthName + "x";
+ StatisticsUpdateRequest request =
+ new StatisticsUpdateRequest(Map.of(longName,
StatisticValues.longValue(1L)));
+
+ Response response =
+ target(
+ "/metalakes/"
+ + metalake
+ + "/objects/"
+ + tableObject.type()
+ + "/"
+ + tableObject.fullName()
+ + "/statistics")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(entity(request, MediaType.APPLICATION_JSON_TYPE));
+
+ Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
response.getStatus());
+ ErrorResponse errorResponse = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
errorResponse.getCode());
+ Assertions.assertEquals(
+ IllegalStatisticNameException.class.getSimpleName(),
errorResponse.getType());
+ Assertions.assertTrue(
+ errorResponse
+ .getMessage()
+ .contains(
+ String.format(
+ "Statistic name must not exceed %d characters",
Statistic.MAX_NAME_LENGTH)));
+ Assertions.assertFalse(errorResponse.getMessage().contains(longName));
+ }
+
@Test
public void testUpdateTableStatisticsWithNullRequestBody() {
when(tableDispatcher.tableExists(any())).thenReturn(true);