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);

Reply via email to