You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by pv...@apache.org on 2021/06/30 11:45:06 UTC
[nifi] branch main updated: NIFI-8730: Invalidate (don't evaluate)
empty script in scripting components
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 <ma...@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 <pi...@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
}