From 6e215b33ceb6760c43d97c57f481ffa9c3aaeec4 Mon Sep 17 00:00:00 2001 From: Gao Binlong Date: Fri, 11 Aug 2023 19:00:17 +0800 Subject: [PATCH 1/5] Fix null_pointer_exception when creating or update ingest pipeline Signed-off-by: Gao Binlong --- CHANGELOG.md | 3 ++- .../java/org/opensearch/ingest/ConfigurationUtils.java | 4 +++- .../org/opensearch/ingest/ConfigurationUtilsTests.java | 10 ++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6bdbafb0fe59..ec14c8f1881ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -128,8 +128,9 @@ 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 ### Security [Unreleased 3.0]: https://github.com/opensearch-project/OpenSearch/compare/2.x...HEAD -[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.10...2.x \ No newline at end of file +[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.10...2.x diff --git a/server/src/main/java/org/opensearch/ingest/ConfigurationUtils.java b/server/src/main/java/org/opensearch/ingest/ConfigurationUtils.java index 9fd9a866ee7e3..7233c97deb38c 100644 --- a/server/src/main/java/org/opensearch/ingest/ConfigurationUtils.java +++ b/server/src/main/java/org/opensearch/ingest/ConfigurationUtils.java @@ -512,7 +512,9 @@ public static Processor readProcessor( String type, Object config ) throws Exception { - if (config instanceof Map) { + if (config == null) { + throw newConfigurationException(type, null, null, "processor [" + type + "] cannot be null"); + } else if (config instanceof Map) { return readProcessor(processorFactories, scriptService, type, (Map) config); } else if (config instanceof String && "script".equals(type)) { Map normalizedScript = new HashMap<>(1); diff --git a/server/src/test/java/org/opensearch/ingest/ConfigurationUtilsTests.java b/server/src/test/java/org/opensearch/ingest/ConfigurationUtilsTests.java index cf67c8af139fb..cc414267120ef 100644 --- 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> config3 = new ArrayList<>(); + config3.add(Collections.singletonMap("null_processor", null)); + + OpenSearchParseException ex = expectThrows( + OpenSearchParseException.class, + () -> ConfigurationUtils.readProcessorConfigs(config3, scriptService, registry) + ); + assertEquals(ex.getMessage(), "processor [null_processor] cannot be null"); } public void testReadProcessorNullDescription() throws Exception { From 15d92b546e2f5881ca36f0579721979378a7f157 Mon Sep 17 00:00:00 2001 From: Gao Binlong Date: Fri, 11 Aug 2023 19:26:41 +0800 Subject: [PATCH 2/5] Modify changelog Signed-off-by: Gao Binlong --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec14c8f1881ca..ac15c829ee3b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -128,7 +128,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 +- Fix null_pointer_exception when creating or updating ingest pipeline ([#9259](https://github.com/opensearch-project/OpenSearch/pull/9259)) ### Security From 2cea65be8d5b05a1c6ead77413f454861713ab65 Mon Sep 17 00:00:00 2001 From: Gao Binlong Date: Mon, 14 Aug 2023 10:59:54 +0800 Subject: [PATCH 3/5] Add nullable tag Signed-off-by: Gao Binlong --- .../java/org/opensearch/ingest/ConfigurationUtils.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/ingest/ConfigurationUtils.java b/server/src/main/java/org/opensearch/ingest/ConfigurationUtils.java index 7233c97deb38c..a73202ef759c7 100644 --- a/server/src/main/java/org/opensearch/ingest/ConfigurationUtils.java +++ b/server/src/main/java/org/opensearch/ingest/ConfigurationUtils.java @@ -37,6 +37,7 @@ import org.opensearch.ExceptionsHelper; import org.opensearch.OpenSearchException; import org.opensearch.OpenSearchParseException; +import org.opensearch.common.Nullable; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.core.xcontent.MediaTypeRegistry; @@ -510,7 +511,7 @@ public static Processor readProcessor( Map processorFactories, ScriptService scriptService, String type, - Object config + @Nullable Object config ) throws Exception { if (config == null) { throw newConfigurationException(type, null, null, "processor [" + type + "] cannot be null"); @@ -529,8 +530,11 @@ public static Processor readProcessor( Map processorFactories, ScriptService scriptService, String type, - Map config + @Nullable Map 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); From 4aaf9a8629ff798c5846d68c14f6c487e24d97f2 Mon Sep 17 00:00:00 2001 From: Gao Binlong Date: Mon, 14 Aug 2023 12:25:07 +0800 Subject: [PATCH 4/5] Add more test Signed-off-by: Gao Binlong --- .../java/org/opensearch/ingest/ConfigurationUtilsTests.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/server/src/test/java/org/opensearch/ingest/ConfigurationUtilsTests.java b/server/src/test/java/org/opensearch/ingest/ConfigurationUtilsTests.java index cc414267120ef..044f72d9f1846 100644 --- a/server/src/test/java/org/opensearch/ingest/ConfigurationUtilsTests.java +++ b/server/src/test/java/org/opensearch/ingest/ConfigurationUtilsTests.java @@ -245,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() { From 5b5707155c824e8875c36bdd1c41ea63f9842b9d Mon Sep 17 00:00:00 2001 From: Gao Binlong Date: Mon, 14 Aug 2023 22:25:54 +0800 Subject: [PATCH 5/5] Modify error message Signed-off-by: Gao Binlong --- .../src/main/java/org/opensearch/ingest/ConfigurationUtils.java | 2 +- .../java/org/opensearch/ingest/ConfigurationUtilsTests.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/ingest/ConfigurationUtils.java b/server/src/main/java/org/opensearch/ingest/ConfigurationUtils.java index a73202ef759c7..85bf8785e62e1 100644 --- a/server/src/main/java/org/opensearch/ingest/ConfigurationUtils.java +++ b/server/src/main/java/org/opensearch/ingest/ConfigurationUtils.java @@ -514,7 +514,7 @@ public static Processor readProcessor( @Nullable Object config ) throws Exception { if (config == null) { - throw newConfigurationException(type, null, null, "processor [" + type + "] cannot be 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) config); } else if (config instanceof String && "script".equals(type)) { diff --git a/server/src/test/java/org/opensearch/ingest/ConfigurationUtilsTests.java b/server/src/test/java/org/opensearch/ingest/ConfigurationUtilsTests.java index 044f72d9f1846..e18e5887c949e 100644 --- a/server/src/test/java/org/opensearch/ingest/ConfigurationUtilsTests.java +++ b/server/src/test/java/org/opensearch/ingest/ConfigurationUtilsTests.java @@ -188,7 +188,7 @@ public void testReadProcessors() throws Exception { OpenSearchParseException.class, () -> ConfigurationUtils.readProcessorConfigs(config3, scriptService, registry) ); - assertEquals(ex.getMessage(), "processor [null_processor] cannot be null"); + assertEquals(ex.getMessage(), "the config of processor [null_processor] cannot be null"); } public void testReadProcessorNullDescription() throws Exception {