(gravitino) branch main updated: [#8530] improvement(common): add null check to prevent NPE in PartitionStatisticsList (#8566)

[roryqi]

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 9f33a8b104 [#8530] improvement(common): add null check to prevent NPE 
in PartitionStatisticsList (#8566)
9f33a8b104 is described below

commit 9f33a8b104897007f7e72e6451352b0b7f3a6cf8
Author: LogLee <lclg...@khu.ac.kr>
AuthorDate: Tue Sep 16 18:19:42 2025 +0900

    [#8530] improvement(common): add null check to prevent NPE in 
PartitionStatisticsList (#8566)
    
    ### What changes were proposed in this pull request?
    
    Add null check precondition to
    `PartitionStatisticsListResponse.validate()` method to prevent
    NullPointerException when the partition statistics array contains null
    elements. The fix ensures that each partition statistic is validated for
    null before calling its `validate()` method.
    
    ### Why are the changes needed?
    
    * The `PartitionStatisticsListResponse.validate()` method was vulnerable
    to NPE when iterating through partition statistics array
    * If the array contained null elements, calling
    `partitionStat.validate()` would throw NullPointerException
    * The existing code only checked if the array itself was null, but not
    individual elements
    * This created a potential runtime failure that should be caught with
    proper validation
    
    Fix: #8350
    
    ### Does this PR introduce _any_ user-facing change?
    
    No, this is a defensive programming improvement. The change prevents
    potential NPE by adding proper validation, but does not change the
    external API or behavior for valid inputs.
    
    ### How was this patch tested?
    
    * Added comprehensive test case
    `testPartitionStatisticsListResponseNullElement()` to verify null
    elements trigger `IllegalArgumentException`
    * Existing unit tests continue to pass, ensuring no functional
    regression
    * Local testing completed successfully with Gradle build and Spotless
    formatting
    * All common module tests compile and run successfully
    * RAT license audit passes without issues
---
 .../dto/responses/PartitionStatisticsListResponse.java |  2 ++
 .../apache/gravitino/dto/responses/TestResponses.java  | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git 
a/common/src/main/java/org/apache/gravitino/dto/responses/PartitionStatisticsListResponse.java
 
b/common/src/main/java/org/apache/gravitino/dto/responses/PartitionStatisticsListResponse.java
index d22240bf0c..5b3be14596 100644
--- 
a/common/src/main/java/org/apache/gravitino/dto/responses/PartitionStatisticsListResponse.java
+++ 
b/common/src/main/java/org/apache/gravitino/dto/responses/PartitionStatisticsListResponse.java
@@ -63,6 +63,8 @@ public class PartitionStatisticsListResponse extends 
BaseResponse {
     Preconditions.checkArgument(
         partitionStatistics != null, "\"partitionStatistics\" must not be 
null");
     for (PartitionStatisticsDTO partitionStat : partitionStatistics) {
+      Preconditions.checkArgument(
+          partitionStat != null, "Each partition statistic must not be null");
       partitionStat.validate();
     }
   }
diff --git 
a/common/src/test/java/org/apache/gravitino/dto/responses/TestResponses.java 
b/common/src/test/java/org/apache/gravitino/dto/responses/TestResponses.java
index 781f5ef449..0c66109fca 100644
--- a/common/src/test/java/org/apache/gravitino/dto/responses/TestResponses.java
+++ b/common/src/test/java/org/apache/gravitino/dto/responses/TestResponses.java
@@ -30,6 +30,7 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
 import java.time.Instant;
 import java.util.Map;
+import java.util.Optional;
 import org.apache.gravitino.Catalog;
 import org.apache.gravitino.NameIdentifier;
 import org.apache.gravitino.authorization.Privileges;
@@ -48,6 +49,8 @@ import org.apache.gravitino.dto.model.ModelVersionDTO;
 import org.apache.gravitino.dto.rel.ColumnDTO;
 import org.apache.gravitino.dto.rel.TableDTO;
 import org.apache.gravitino.dto.rel.partitioning.Partitioning;
+import org.apache.gravitino.dto.stats.PartitionStatisticsDTO;
+import org.apache.gravitino.dto.stats.StatisticDTO;
 import org.apache.gravitino.dto.tag.TagDTO;
 import org.apache.gravitino.dto.util.DTOConverters;
 import org.apache.gravitino.json.JsonUtils;
@@ -510,4 +513,19 @@ public class TestResponses {
     ModelVersionResponse response1 = new ModelVersionResponse();
     assertThrows(IllegalArgumentException.class, response1::validate);
   }
+
+  @Test
+  void testPartitionStatisticsListResponseNullElement() {
+    AuditDTO audit = 
AuditDTO.builder().withCreator("user1").withCreateTime(Instant.now()).build();
+    StatisticDTO statistic =
+        StatisticDTO.builder()
+            .withName("stat")
+            .withAudit(audit)
+            .withValue(Optional.empty())
+            .build();
+    PartitionStatisticsDTO valid = PartitionStatisticsDTO.of("p1", new 
StatisticDTO[] {statistic});
+    PartitionStatisticsDTO[] stats = new PartitionStatisticsDTO[] {valid, 
null};
+    PartitionStatisticsListResponse response = new 
PartitionStatisticsListResponse(stats);
+    assertThrows(IllegalArgumentException.class, response::validate);
+  }
 }