nifi-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pvill...@apache.org
Subject [nifi] branch main updated: NIFI-8730: Invalidate (don't evaluate) empty script in scripting components
Date Wed, 30 Jun 2021 11:45:06 GMT
This is an automated email from the ASF dual-hosted git repository.

pvillard pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/nifi.git


The following commit(s) were added to refs/heads/main by this push:
     new 632fbcc  NIFI-8730: Invalidate (don't evaluate) empty script in scripting components
632fbcc is described below

commit 632fbcca7d7e028c4526ab2b643c6b83cfadef99
Author: Matthew Burgess <mattyb149@apache.org>
AuthorDate: Thu Jun 24 11:09:58 2021 -0400

    NIFI-8730: Invalidate (don't evaluate) empty script in scripting components
    
    Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com>
    
    This closes #5184.
---
 .../lookup/script/BaseScriptedLookupService.java   |  5 ++--
 .../processors/script/InvokeScriptedProcessor.java | 31 +++++++++++++++-------
 .../processors/script/ScriptedTransformRecord.java |  2 +-
 .../record/sink/script/ScriptedRecordSink.java     |  5 ++--
 .../rules/engine/script/ScriptedRulesEngine.java   |  5 ++--
 .../handlers/script/ScriptedActionHandler.java     |  5 ++--
 .../script/AbstractScriptedControllerService.java  |  4 +--
 .../processors/script/TestInvokeJavascript.java    | 15 ++++++++---
 .../src/test/resources/groovy/test_reader.groovy   |  2 +-
 9 files changed, 45 insertions(+), 29 deletions(-)

diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/lookup/script/BaseScriptedLookupService.java
b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/lookup/script/BaseScriptedLookupService.java
index 9c7a4b4..623796e 100644
--- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/lookup/script/BaseScriptedLookupService.java
+++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/lookup/script/BaseScriptedLookupService.java
@@ -241,11 +241,10 @@ public class BaseScriptedLookupService extends AbstractScriptedControllerService
     public void setup() {
         if (scriptNeedsReload.get() || lookupService.get() == null) {
             if (ScriptingComponentHelper.isFile(scriptingComponentHelper.getScriptPath()))
{
-                reloadScriptFile(scriptingComponentHelper.getScriptPath());
+                scriptNeedsReload.set(reloadScriptFile(scriptingComponentHelper.getScriptPath()));
             } else {
-                reloadScriptBody(scriptingComponentHelper.getScriptBody());
+                scriptNeedsReload.set(reloadScriptBody(scriptingComponentHelper.getScriptBody()));
             }
-            scriptNeedsReload.set(false);
         }
     }
 
diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/InvokeScriptedProcessor.java
b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/InvokeScriptedProcessor.java
index 681c798..48f12ce 100644
--- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/InvokeScriptedProcessor.java
+++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/InvokeScriptedProcessor.java
@@ -215,11 +215,10 @@ public class InvokeScriptedProcessor extends AbstractSessionFactoryProcessor
{
     public void setup() {
         if (scriptNeedsReload.get() || processor.get() == null) {
             if (ScriptingComponentHelper.isFile(scriptingComponentHelper.getScriptPath()))
{
-                reloadScriptFile(scriptingComponentHelper.getScriptPath());
+                scriptNeedsReload.set(reloadScriptFile(scriptingComponentHelper.getScriptPath()));
             } else {
-                reloadScriptBody(scriptingComponentHelper.getScriptBody());
+                scriptNeedsReload.set(reloadScriptBody(scriptingComponentHelper.getScriptBody()));
             }
-            scriptNeedsReload.set(false);
         }
     }
 
@@ -326,6 +325,10 @@ public class InvokeScriptedProcessor extends AbstractSessionFactoryProcessor
{
      * @return Whether the script was successfully reloaded
      */
     private boolean reloadScript(final String scriptBody) {
+        if (StringUtils.isEmpty(scriptBody)) {
+            return true;
+        }
+
         // note we are starting here with a fresh listing of validation
         // results since we are (re)loading a new/updated script. any
         // existing validation results are not relevant
@@ -431,15 +434,14 @@ public class InvokeScriptedProcessor extends AbstractSessionFactoryProcessor
{
         // store the updated validation results
         validationResults.set(results);
 
-        // return whether there was any issues loading the configured script
-        return results.isEmpty();
+        // return whether there were any issues loading the configured script
+        return !results.isEmpty();
     }
 
     /**
      * Invokes the validate() routine provided by the script, allowing for
-     * custom validation code. This method assumes there is a valid Processor
-     * defined in the script and it has been loaded by the
-     * InvokeScriptedProcessor processor
+     * custom validation code, if there is a valid Processor
+     * defined in the script and it has been loaded by the processor
      *
      * @param context The validation context to be passed into the custom
      * validate method
@@ -460,6 +462,12 @@ public class InvokeScriptedProcessor extends AbstractSessionFactoryProcessor
{
             return validationResults.get();
         }
 
+        Collection<ValidationResult> scriptingComponentHelperResults = scriptingComponentHelper.customValidate(context);
+        if (scriptingComponentHelperResults != null && !scriptingComponentHelperResults.isEmpty())
{
+            validationResults.set(scriptingComponentHelperResults);
+            return scriptingComponentHelperResults;
+        }
+
         scriptingComponentHelper.setScriptEngineName(context.getProperty(scriptingComponentHelper.SCRIPT_ENGINE).getValue());
         scriptingComponentHelper.setScriptPath(context.getProperty(ScriptingComponentUtils.SCRIPT_FILE).evaluateAttributeExpressions().getValue());
         scriptingComponentHelper.setScriptBody(context.getProperty(ScriptingComponentUtils.SCRIPT_BODY).getValue());
@@ -476,7 +484,7 @@ public class InvokeScriptedProcessor extends AbstractSessionFactoryProcessor
{
             try {
                 // defer to the underlying processor for validation, without the
                 // invokescriptedprocessor properties
-                final Set<PropertyDescriptor> innerPropertyDescriptor = new HashSet<PropertyDescriptor>(scriptingComponentHelper.getDescriptors());
+                final Set<PropertyDescriptor> innerPropertyDescriptor = new HashSet<>(scriptingComponentHelper.getDescriptors());
 
                 ValidationContext innerValidationContext = new FilteredPropertiesValidationContextAdapter(context,
innerPropertyDescriptor);
                 final Collection<ValidationResult> instanceResults = instance.validate(innerValidationContext);
@@ -568,7 +576,10 @@ public class InvokeScriptedProcessor extends AbstractSessionFactoryProcessor
{
 
     @OnStopped
     public void stop(ProcessContext context) {
-        invokeScriptedProcessorMethod("onStopped", context);
+        // If the script needs to be reloaded at this point, it is because it was empty
+        if (!scriptNeedsReload.get()) {
+            invokeScriptedProcessorMethod("onStopped", context);
+        }
         scriptingComponentHelper.stop();
         processor.set(null);
         scriptRunner = null;
diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/ScriptedTransformRecord.java
b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/ScriptedTransformRecord.java
index efe54de..370f3f2 100644
--- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/ScriptedTransformRecord.java
+++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/ScriptedTransformRecord.java
@@ -168,7 +168,7 @@ public class ScriptedTransformRecord extends AbstractProcessor implements
Search
     @OnScheduled
     public void setup(final ProcessContext context) throws IOException {
         if (!scriptingComponentHelper.isInitialized.get()) {
-            scriptingComponentHelper.createResources();
+            scriptingComponentHelper.createResources(false);
         }
 
         scriptingComponentHelper.setupVariables(context);
diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/record/sink/script/ScriptedRecordSink.java
b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/record/sink/script/ScriptedRecordSink.java
index 652965f..f17cb0e 100644
--- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/record/sink/script/ScriptedRecordSink.java
+++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/record/sink/script/ScriptedRecordSink.java
@@ -107,11 +107,10 @@ public class ScriptedRecordSink extends AbstractScriptedControllerService
implem
     public void setup() {
         if (scriptNeedsReload.get() || recordSink.get() == null) {
             if (ScriptingComponentHelper.isFile(scriptingComponentHelper.getScriptPath()))
{
-                reloadScriptFile(scriptingComponentHelper.getScriptPath());
+                scriptNeedsReload.set(reloadScriptFile(scriptingComponentHelper.getScriptPath()));
             } else {
-                reloadScriptBody(scriptingComponentHelper.getScriptBody());
+                scriptNeedsReload.set(reloadScriptBody(scriptingComponentHelper.getScriptBody()));
             }
-            scriptNeedsReload.set(false);
         }
     }
 
diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/engine/script/ScriptedRulesEngine.java
b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/engine/script/ScriptedRulesEngine.java
index 7d09509..5239761 100644
--- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/engine/script/ScriptedRulesEngine.java
+++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/engine/script/ScriptedRulesEngine.java
@@ -78,11 +78,10 @@ public class ScriptedRulesEngine extends AbstractScriptedControllerService
imple
     public void setup() {
         if (scriptNeedsReload.get() || rulesEngine.get() == null) {
             if (ScriptingComponentHelper.isFile(scriptingComponentHelper.getScriptPath()))
{
-                reloadScriptFile(scriptingComponentHelper.getScriptPath());
+                scriptNeedsReload.set(reloadScriptFile(scriptingComponentHelper.getScriptPath()));
             } else {
-                reloadScriptBody(scriptingComponentHelper.getScriptBody());
+                scriptNeedsReload.set(reloadScriptBody(scriptingComponentHelper.getScriptBody()));
             }
-            scriptNeedsReload.set(false);
         }
     }
 
diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/handlers/script/ScriptedActionHandler.java
b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/handlers/script/ScriptedActionHandler.java
index 2b48bf6..f81ade4 100644
--- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/handlers/script/ScriptedActionHandler.java
+++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/handlers/script/ScriptedActionHandler.java
@@ -80,11 +80,10 @@ public class ScriptedActionHandler extends AbstractScriptedControllerService
imp
     public void setup() {
         if (scriptNeedsReload.get() || actionHandler.get() == null) {
             if (ScriptingComponentHelper.isFile(scriptingComponentHelper.getScriptPath()))
{
-                reloadScriptFile(scriptingComponentHelper.getScriptPath());
+                scriptNeedsReload.set(reloadScriptFile(scriptingComponentHelper.getScriptPath()));
             } else {
-                reloadScriptBody(scriptingComponentHelper.getScriptBody());
+                scriptNeedsReload.set(reloadScriptBody(scriptingComponentHelper.getScriptBody()));
             }
-            scriptNeedsReload.set(false);
         }
     }
 
diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/script/AbstractScriptedControllerService.java
b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/script/AbstractScriptedControllerService.java
index 35ea374..8c8a391 100644
--- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/script/AbstractScriptedControllerService.java
+++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/script/AbstractScriptedControllerService.java
@@ -119,7 +119,7 @@ public abstract class AbstractScriptedControllerService extends AbstractControll
     @Override
     protected Collection<ValidationResult> customValidate(ValidationContext validationContext)
{
 
-        Collection<ValidationResult> commonValidationResults = super.customValidate(validationContext);
+        Collection<ValidationResult> commonValidationResults = new ArrayList<>(super.customValidate(validationContext));
         commonValidationResults.addAll(scriptingComponentHelper.customValidate(validationContext));
 
         if (!commonValidationResults.isEmpty()) {
@@ -173,7 +173,7 @@ public abstract class AbstractScriptedControllerService extends AbstractControll
         validationResults.set(results);
 
         // return whether there was any issues loading the configured script
-        return results.isEmpty();
+        return !results.isEmpty();
     }
 
     /**
diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/java/org/apache/nifi/processors/script/TestInvokeJavascript.java
b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/java/org/apache/nifi/processors/script/TestInvokeJavascript.java
index f6500c6..9118f5a 100644
--- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/java/org/apache/nifi/processors/script/TestInvokeJavascript.java
+++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/java/org/apache/nifi/processors/script/TestInvokeJavascript.java
@@ -52,7 +52,6 @@ public class TestInvokeJavascript extends BaseScriptTest {
      */
     @Test
     public void testReadFlowFileContentAndStoreInFlowFileAttribute() throws Exception {
-        runner.setValidateExpressionUsage(false);
         runner.setProperty(scriptingComponent.getScriptingComponentHelper().SCRIPT_ENGINE,
"ECMAScript");
         runner.setProperty(ScriptingComponentUtils.SCRIPT_FILE, "target/test/resources/javascript/test_reader.js");
         runner.setProperty(ScriptingComponentUtils.MODULES, "target/test/resources/javascript");
@@ -146,7 +145,6 @@ public class TestInvokeJavascript extends BaseScriptTest {
     @Test(expected = AssertionError.class)
     public void testInvokeScriptCausesException() throws Exception {
         final TestRunner runner = TestRunners.newTestRunner(new InvokeScriptedProcessor());
-        runner.setValidateExpressionUsage(false);
         runner.setProperty(scriptingComponent.getScriptingComponentHelper().SCRIPT_ENGINE,
"ECMAScript");
         runner.setProperty(ScriptingComponentUtils.SCRIPT_BODY, getFileContentsAsString(
                 TEST_RESOURCE_LOCATION + "javascript/testInvokeScriptCausesException.js")
@@ -164,7 +162,6 @@ public class TestInvokeJavascript extends BaseScriptTest {
      */
     @Test
     public void testScriptRoutesToFailure() throws Exception {
-        runner.setValidateExpressionUsage(false);
         runner.setProperty(scriptingComponent.getScriptingComponentHelper().SCRIPT_ENGINE,
"ECMAScript");
         runner.setProperty(ScriptingComponentUtils.SCRIPT_BODY, getFileContentsAsString(
                 TEST_RESOURCE_LOCATION + "javascript/testScriptRoutesToFailure.js")
@@ -177,4 +174,16 @@ public class TestInvokeJavascript extends BaseScriptTest {
         final List<MockFlowFile> result = runner.getFlowFilesForRelationship("FAILURE");
         assertFalse(result.isEmpty());
     }
+
+    /**
+     * Tests an empty script with Nashorn (which throws an NPE if it is loaded), this test
verifies an empty script is not attempted to be loaded.
+     *
+     * @throws Exception Any error encountered while testing
+     */
+    @Test
+    public void testEmptyScript() throws Exception {
+        runner.setProperty(scriptingComponent.getScriptingComponentHelper().SCRIPT_ENGINE,
"ECMAScript");
+        runner.setProperty(ScriptingComponentUtils.SCRIPT_BODY, "");
+        runner.assertNotValid();
+    }
 }
diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/resources/groovy/test_reader.groovy
b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/resources/groovy/test_reader.groovy
index 03395a2..6e3dfb1 100644
--- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/resources/groovy/test_reader.groovy
+++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/resources/groovy/test_reader.groovy
@@ -57,7 +57,7 @@ class GroovyProcessor implements Processor {
     @Override
     void onTrigger(ProcessContext context, ProcessSessionFactory sessionFactory) throws ProcessException
{
         def session = sessionFactory.createSession()
-        def flowFile = session.get();
+        def flowFile = session.get()
         if (flowFile == null) {
             return
         }

Mime
View raw message