(gravitino) branch main updated: [#8195] fix(flink-jdbc): validate required 'jdbc-url' in toFlinkCatalogProperties to prevent NPE (#8263)

[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 d476393215 [#8195] fix(flink-jdbc): validate required 'jdbc-url' in 
toFlinkCatalogProperties to prevent NPE (#8263)
d476393215 is described below

commit d476393215e0f0fa5cbb10c4deec2f9916360cd3
Author: Minji Ryu <77240765+mx...@users.noreply.github.com>
AuthorDate: Mon Aug 25 18:56:04 2025 +0900

    [#8195] fix(flink-jdbc): validate required 'jdbc-url' in 
toFlinkCatalogProperties to prevent NPE (#8263)
    
    ### What changes were proposed in this pull request?
    
    - Validate required property GRAVITINO_JDBC_URL in
    JdbcPropertiesConverter#toFlinkCatalogProperties to prevent NPE.
    - Throw IllegalArgumentException with a clear message when jdbc-url is
    missing or blank, instead of letting an NPE occur.
    - `Tests` : add the test `testToJdbcCatalogPropertiesWithoutUrl()` in
    AbstractJdbcPropertiesConverterTestSuite
    
    ### Why are the changes needed?
    
    - When jdbc-url is missing, the current implementation can throw a
    NullPointerException during URL processing. Failing fast with a
    meaningful validation error improves debuggability and avoids leaking
    internal details.
    
    Fix: #8195
    
    ### Does this PR introduce *any* user-facing change?
    - APIs / property keys: No changes.
    - Behavior: If jdbc-url is missing, the code now throws
    IllegalArgumentException with a descriptive message (previously resulted
    in an NPE).
    
    ### How was this patch tested?
    
    Local runs:
    ```
    ./gradlew spotlessApply
    ./gradlew test -PskipITs
    ```
    
    ---------
    
    Co-authored-by: Justin Mclean <jus...@classsoftware.com>
---
 .../flink/connector/jdbc/JdbcPropertiesConverter.java     |  6 ++++++
 .../jdbc/AbstractJdbcPropertiesConverterTestSuite.java    | 15 +++++++++++++++
 2 files changed, 21 insertions(+)

diff --git 
a/flink-connector/flink/src/main/java/org/apache/gravitino/flink/connector/jdbc/JdbcPropertiesConverter.java
 
b/flink-connector/flink/src/main/java/org/apache/gravitino/flink/connector/jdbc/JdbcPropertiesConverter.java
index 45b999ffde..b46d2cc1bf 100644
--- 
a/flink-connector/flink/src/main/java/org/apache/gravitino/flink/connector/jdbc/JdbcPropertiesConverter.java
+++ 
b/flink-connector/flink/src/main/java/org/apache/gravitino/flink/connector/jdbc/JdbcPropertiesConverter.java
@@ -48,6 +48,12 @@ public abstract class JdbcPropertiesConverter implements 
PropertiesConverter {
     Map<String, String> flinkCatalogProperties =
         
PropertiesConverter.super.toFlinkCatalogProperties(gravitinoProperties);
     String gravitinoJdbcUrl = 
gravitinoProperties.get(JdbcPropertiesConstants.GRAVITINO_JDBC_URL);
+    Preconditions.checkArgument(
+        gravitinoJdbcUrl != null,
+        "Cannot create catalog properties: missing '"
+            + JdbcPropertiesConstants.GRAVITINO_JDBC_URL
+            + "'.");
+
     // The URL in FlinkJdbcCatalog does not support database and other 
parameters.
     flinkCatalogProperties.put(
         JdbcPropertiesConstants.FLINK_JDBC_URL, 
getBaseUrlFromJdbcUrl(gravitinoJdbcUrl));
diff --git 
a/flink-connector/flink/src/test/java/org/apache/gravitino/flink/connector/jdbc/AbstractJdbcPropertiesConverterTestSuite.java
 
b/flink-connector/flink/src/test/java/org/apache/gravitino/flink/connector/jdbc/AbstractJdbcPropertiesConverterTestSuite.java
index 80f5886461..d89fbf5401 100644
--- 
a/flink-connector/flink/src/test/java/org/apache/gravitino/flink/connector/jdbc/AbstractJdbcPropertiesConverterTestSuite.java
+++ 
b/flink-connector/flink/src/test/java/org/apache/gravitino/flink/connector/jdbc/AbstractJdbcPropertiesConverterTestSuite.java
@@ -153,4 +153,19 @@ public abstract class 
AbstractJdbcPropertiesConverterTestSuite {
     Assertions.assertEquals(
         testDriver, 
properties.get(JdbcPropertiesConstants.GRAVITINO_JDBC_DRIVER));
   }
+
+  @Test
+  public void testToJdbcCatalogPropertiesWithoutUrl() {
+    Map<String, String> catalogPropertiesWithoutUrl = new 
HashMap<>(catalogProperties);
+    
catalogPropertiesWithoutUrl.remove(JdbcPropertiesConstants.GRAVITINO_JDBC_URL);
+
+    IllegalArgumentException ex =
+        Assertions.assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                getConverter(catalogPropertiesWithoutUrl)
+                    .toFlinkCatalogProperties(catalogPropertiesWithoutUrl));
+
+    
Assertions.assertTrue(ex.getMessage().contains(JdbcPropertiesConstants.GRAVITINO_JDBC_URL));
+  }
 }