Fix null_pointer_exception when creating or updating ingest pipeline (o…

…pensearch-project#9259) * Fix null_pointer_exception when creating or update ingest pipeline Signed-off-by: Gao Binlong <[email protected]> * Modify changelog Signed-off-by: Gao Binlong <[email protected]> * Add nullable tag Signed-off-by: Gao Binlong <[email protected]> * Add more test Signed-off-by: Gao Binlong <[email protected]> * Modify error message Signed-off-by: Gao Binlong <[email protected]> --------- Signed-off-by: Gao Binlong <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>
Gaurav614 · Apr 25, 2024 · e7136b8 · e7136b8
1 parent 86ae757
commit e7136b8
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -184,6 +184,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ### Fixed
 - Fix flaky ResourceAwareTasksTests.testBasicTaskResourceTracking test ([#8993](https://github.com/opensearch-project/OpenSearch/pull/8993))
+- Fix null_pointer_exception when creating or updating ingest pipeline ([#9259](https://github.com/opensearch-project/OpenSearch/pull/9259))
 - Fix memory leak when using Zstd Dictionary ([#9403](https://github.com/opensearch-project/OpenSearch/pull/9403))
 - Fix range reads in respository-s3 ([9512](https://github.com/opensearch-project/OpenSearch/issues/9512))
 - Handle null partSize in OnDemandBlockSnapshotIndexInput ([#9291](https://github.com/opensearch-project/OpenSearch/issues/9291))

diff --git a/server/src/main/java/org/opensearch/ingest/ConfigurationUtils.java b/server/src/main/java/org/opensearch/ingest/ConfigurationUtils.java
@@ -35,6 +35,7 @@
 import org.opensearch.ExceptionsHelper;
 import org.opensearch.OpenSearchException;
 import org.opensearch.OpenSearchParseException;
+import org.opensearch.common.Nullable;
 import org.opensearch.common.xcontent.LoggingDeprecationHandler;
 import org.opensearch.common.xcontent.json.JsonXContent;
 import org.opensearch.core.common.bytes.BytesReference;
@@ -510,9 +511,11 @@ public static Processor readProcessor(
         Map<String, Processor.Factory> processorFactories,
         ScriptService scriptService,
         String type,
-        Object config
+        @Nullable Object config
     ) throws Exception {
-        if (config instanceof Map) {
+        if (config == null) {
+            throw newConfigurationException(type, null, null, "the config of processor [" + type + "] cannot be null");
+        } else if (config instanceof Map) {
             return readProcessor(processorFactories, scriptService, type, (Map<String, Object>) config);
         } else if (config instanceof String && "script".equals(type)) {
             Map<String, Object> normalizedScript = new HashMap<>(1);
@@ -527,8 +530,11 @@ public static Processor readProcessor(
         Map<String, Processor.Factory> processorFactories,
         ScriptService scriptService,
         String type,
-        Map<String, Object> config
+        @Nullable Map<String, Object> config
     ) throws Exception {
+        if (config == null) {
+            throw newConfigurationException(type, null, null, "expect the config of processor [" + type + "] to be map, but is null");
+        }
         String tag = ConfigurationUtils.readOptionalStringProperty(null, null, config, TAG_KEY);
         String description = ConfigurationUtils.readOptionalStringProperty(null, tag, config, DESCRIPTION_KEY);
         boolean ignoreFailure = ConfigurationUtils.readBooleanProperty(null, null, config, IGNORE_FAILURE_KEY, false);

diff --git a/server/src/test/java/org/opensearch/ingest/ConfigurationUtilsTests.java b/server/src/test/java/org/opensearch/ingest/ConfigurationUtilsTests.java
@@ -179,6 +179,16 @@ public void testReadProcessors() throws Exception {
         assertThat(e2.getMetadata("opensearch.processor_tag"), equalTo(Collections.singletonList("my_second_unknown")));
         assertThat(e2.getMetadata("opensearch.processor_type"), equalTo(Collections.singletonList("second_unknown_processor")));
         assertThat(e2.getMetadata("opensearch.property_name"), is(nullValue()));
+
+        // test null config
+        List<Map<String, Object>> config3 = new ArrayList<>();
+        config3.add(Collections.singletonMap("null_processor", null));
+
+        OpenSearchParseException ex = expectThrows(
+            OpenSearchParseException.class,
+            () -> ConfigurationUtils.readProcessorConfigs(config3, scriptService, registry)
+        );
+        assertEquals(ex.getMessage(), "the config of processor [null_processor] cannot be null");
     }
 
     public void testReadProcessorNullDescription() throws Exception {
@@ -235,6 +245,12 @@ public void testReadProcessorFromObjectOrMap() throws Exception {
             () -> ConfigurationUtils.readProcessor(registry, scriptService, "unknown_processor", invalidConfig)
         );
         assertThat(ex.getMessage(), equalTo("property isn't a map, but of type [" + invalidConfig.getClass().getName() + "]"));
+
+        ex = expectThrows(
+            OpenSearchParseException.class,
+            () -> ConfigurationUtils.readProcessor(registry, scriptService, "null_processor", null)
+        );
+        assertEquals(ex.getMessage(), "expect the config of processor [null_processor] to be map, but is null");
     }
 
     public void testNoScriptCompilation() {