(gravitino) branch main updated: [#8085] fix: NPE in CatalogCreateRequest constructor when type is null (#8121)

[jmclean]

This is an automated email from the ASF dual-hosted git repository.

jmclean 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 1c7b2de40c [#8085] fix: NPE in CatalogCreateRequest constructor when 
type is null (#8121)
1c7b2de40c is described below

commit 1c7b2de40c71f5559223e321f9e91793ab9e5801
Author: Minji Kim <kimmin7...@gmail.com>
AuthorDate: Mon Aug 18 07:41:15 2025 +0900

    [#8085] fix: NPE in CatalogCreateRequest constructor when type is null 
(#8121)
    
    ## What changes were proposed in this pull request?
    Add null safety checks in CatalogCreateRequest constructor to prevent
    NullPointerException when type is null and provider is "hadoop".
    Additionally, add a comprehensive test case to verify the fix works
    correctly and throws IllegalArgumentException instead of NPE.
    
    ## Why are the changes needed?
    - The constructor was vulnerable to NPE when
    `this.provider.equalsIgnoreCase("hadoop")` was called with a null
    provider
    - When type is null and provider is "hadoop",
    `CatalogProvider.shortNameForManagedCatalog(type)` would throw NPE
    - As mentioned by @justinmclean, relying on validate() call order is
    brittle and a simple defensive check costs nothing while hardening the
    code
    
    ## Does this PR introduce any user-facing change?
    No, this is purely a defensive programming improvement. The behavior for
    valid inputs remains unchanged, and invalid inputs now throw clear
    IllegalArgumentException with descriptive messages instead of cryptic
    NPE.
    
    ## How was this patch tested?
    - Existing unit tests continue to pass, ensuring no functional
    regression
    - Added a new test case
    `testCatalogCreateRequestNullTypeHadoopProvider()` to verify NPE
    scenario throws IllegalArgumentException instead
    - Local testing completed successfully with Gradle build and Spotless
    formatting
    - All common module tests pass
---
 .../gravitino/dto/requests/CatalogCreateRequest.java       |  9 +++++++--
 .../gravitino/dto/requests/TestCatalogCreateRequest.java   | 14 ++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git 
a/common/src/main/java/org/apache/gravitino/dto/requests/CatalogCreateRequest.java
 
b/common/src/main/java/org/apache/gravitino/dto/requests/CatalogCreateRequest.java
index dc30a7a561..3223fce6c8 100644
--- 
a/common/src/main/java/org/apache/gravitino/dto/requests/CatalogCreateRequest.java
+++ 
b/common/src/main/java/org/apache/gravitino/dto/requests/CatalogCreateRequest.java
@@ -86,11 +86,16 @@ public class CatalogCreateRequest implements RESTRequest {
               + " that doesn't support managed catalog");
     }
 
-    if (this.provider.equalsIgnoreCase("hadoop")) {
+    if (this.provider != null && this.provider.equalsIgnoreCase("hadoop")) {
       // For backward compatibility, if the provider is "hadoop" (legacy 
case), we set the
       // provider to "fileset". This is because provider "hadoop" was 
previously used as a
       // "fileset" catalog. This is a special case to maintain compatibility.
-      this.provider = CatalogProvider.shortNameForManagedCatalog(type);
+      if (type != null) {
+        this.provider = CatalogProvider.shortNameForManagedCatalog(type);
+      } else {
+        throw new IllegalArgumentException(
+            "Catalog type cannot be null when provider is \"hadoop\"");
+      }
     }
   }
 
diff --git 
a/common/src/test/java/org/apache/gravitino/dto/requests/TestCatalogCreateRequest.java
 
b/common/src/test/java/org/apache/gravitino/dto/requests/TestCatalogCreateRequest.java
index 3b5221ff1a..04544d23ee 100644
--- 
a/common/src/test/java/org/apache/gravitino/dto/requests/TestCatalogCreateRequest.java
+++ 
b/common/src/test/java/org/apache/gravitino/dto/requests/TestCatalogCreateRequest.java
@@ -77,4 +77,18 @@ public class TestCatalogCreateRequest {
         JsonMappingException.class,
         () -> JsonUtils.objectMapper().readValue(json1, 
CatalogCreateRequest.class));
   }
+
+  @Test
+  public void testCatalogCreateRequestNullTypeHadoopProvider() {
+    // Test NPE case when type is null and provider is "hadoop"
+    // This should throw IllegalArgumentException instead of NPE in constructor
+    IllegalArgumentException exception =
+        Assertions.assertThrows(
+            IllegalArgumentException.class,
+            () -> new CatalogCreateRequest("catalog_test", null, "hadoop", 
null, null));
+
+    // Verify the exception message
+    Assertions.assertEquals(
+        "Catalog type cannot be null when provider is \"hadoop\"", 
exception.getMessage());
+  }
 }