From 7d70a97b77c1d79ae63271703368adccb588cc46 Mon Sep 17 00:00:00 2001 From: Christoph Weitkamp Date: Mon, 19 Oct 2020 13:41:58 +0200 Subject: [PATCH] Fixed IndexOutOfBoundsException in ScriptModuleTypeProvider (#1730) Signed-off-by: Christoph Weitkamp --- .../internal/ScriptEngineManagerImpl.java | 35 ++++++++-------- .../provider/ScriptModuleTypeProvider.java | 42 +++++++++++-------- .../core/automation/type/ActionType.java | 8 ++-- .../core/automation/type/ConditionType.java | 7 ++-- 4 files changed, 50 insertions(+), 42 deletions(-) diff --git a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java index 1c406105795..ed5bc0a76fe 100644 --- a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java +++ b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java @@ -44,9 +44,9 @@ public class ScriptEngineManagerImpl implements ScriptEngineManager { private final Logger logger = LoggerFactory.getLogger(ScriptEngineManagerImpl.class); - private final Map loadedScriptEngineInstances = new HashMap<>(); - private final Map customSupport = new HashMap<>(); - private final Map genericSupport = new HashMap<>(); + private final Map loadedScriptEngineInstances = new HashMap<>(); + private final Map customSupport = new HashMap<>(); + private final Map genericSupport = new HashMap<>(); private final ScriptExtensionManager scriptExtensionManager; @Activate @@ -57,7 +57,6 @@ public ScriptEngineManagerImpl(final @Reference ScriptExtensionManager scriptExt @Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC) public void addScriptEngineFactory(ScriptEngineFactory engineFactory) { List scriptTypes = engineFactory.getScriptTypes(); - logger.trace("{}.getScriptTypes(): {}", engineFactory.getClass().getSimpleName(), scriptTypes); for (String scriptType : scriptTypes) { if (isCustomFactory(engineFactory)) { @@ -66,26 +65,27 @@ public void addScriptEngineFactory(ScriptEngineFactory engineFactory) { this.genericSupport.put(scriptType, engineFactory); } } - if (!engineFactory.getScriptTypes().isEmpty()) { - ScriptEngine scriptEngine = engineFactory.createScriptEngine(engineFactory.getScriptTypes().get(0)); - if (scriptEngine != null) { - javax.script.ScriptEngineFactory factory = scriptEngine.getFactory(); - logger.debug( - "Initialized a {} ScriptEngineFactory for {} ({}): supports {} ({}) with file extensions {}, names {}, and mimetypes {}", - (isCustomFactory(engineFactory)) ? "custom" : "generic", factory.getEngineName(), - factory.getEngineVersion(), factory.getLanguageName(), factory.getLanguageVersion(), - factory.getExtensions(), factory.getNames(), factory.getMimeTypes()); + if (logger.isDebugEnabled()) { + if (!scriptTypes.isEmpty()) { + ScriptEngine scriptEngine = engineFactory.createScriptEngine(scriptTypes.get(0)); + if (scriptEngine != null) { + javax.script.ScriptEngineFactory factory = scriptEngine.getFactory(); + logger.debug( + "Initialized a {} ScriptEngineFactory for {} ({}): supports {} ({}) with file extensions {}, names {}, and mimetypes {}", + (isCustomFactory(engineFactory)) ? "custom" : "generic", factory.getEngineName(), + factory.getEngineVersion(), factory.getLanguageName(), factory.getLanguageVersion(), + factory.getExtensions(), factory.getNames(), factory.getMimeTypes()); + } else { + logger.trace("addScriptEngineFactory: engine was null"); + } } else { - logger.trace("addScriptEngineFactory: engine was null"); + logger.trace("addScriptEngineFactory: scriptTypes was empty"); } - } else { - logger.trace("addScriptEngineFactory: scriptTypes was empty"); } } public void removeScriptEngineFactory(ScriptEngineFactory engineFactory) { List scriptTypes = engineFactory.getScriptTypes(); - logger.trace("{}.getScriptTypes(): {}", engineFactory.getClass().getSimpleName(), scriptTypes); for (String scriptType : scriptTypes) { if (isCustomFactory(engineFactory)) { @@ -150,7 +150,6 @@ public void loadScript(String engineIdentifier, InputStreamReader scriptData) { ScriptEngine engine = container.getScriptEngine(); try { engine.eval(scriptData); - if (engine instanceof Invocable) { Invocable inv = (Invocable) engine; try { diff --git a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/provider/ScriptModuleTypeProvider.java b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/provider/ScriptModuleTypeProvider.java index 396ff0c492d..52733ca4a98 100755 --- a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/provider/ScriptModuleTypeProvider.java +++ b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/provider/ScriptModuleTypeProvider.java @@ -18,8 +18,6 @@ import java.util.Locale; import java.util.Map; import java.util.TreeMap; -import java.util.stream.Collectors; -import java.util.stream.Stream; import javax.script.ScriptEngine; @@ -77,7 +75,6 @@ public class ScriptModuleTypeProvider implements ModuleTypeProvider { return null; } else { List outputs = new ArrayList<>(); - Output result = new Output("result", "java.lang.Object", "result", "the script result", null, null, null); outputs.add(result); return new ActionType(ScriptActionHandler.TYPE_ID, getConfigDescriptions(locale), "execute a given script", @@ -113,13 +110,12 @@ private List getConfigDescriptions(@Nullable Locale final ConfigDescriptionParameter script = ConfigDescriptionParameterBuilder.create("script", Type.TEXT) .withRequired(true).withReadOnly(false).withMultiple(false).withLabel("Script").withContext("script") .withDescription("the script to execute").build(); - return Stream.of(scriptType, script).collect(Collectors.toList()); + return List.of(scriptType, script); } @Override public Collection getModuleTypes(@Nullable Locale locale) { - return (Collection) Stream.of(getScriptActionType(locale), getScriptConditionType(locale)) - .collect(Collectors.toList()); + return (Collection) List.of(getScriptActionType(locale), getScriptConditionType(locale)); } @Override @@ -143,24 +139,34 @@ public void removeProviderChangeListener(ProviderChangeListener list */ @Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC) public void setScriptEngineFactory(ScriptEngineFactory engineFactory) { - ScriptEngine scriptEngine = engineFactory.createScriptEngine(engineFactory.getScriptTypes().get(0)); - if (scriptEngine != null) { - javax.script.ScriptEngineFactory factory = scriptEngine.getFactory(); - parameterOptions.put(getPreferredMimeType(factory), getLanguageName(factory)); - logger.trace("ParameterOptions: {}", parameterOptions); + List scriptTypes = engineFactory.getScriptTypes(); + if (!scriptTypes.isEmpty()) { + ScriptEngine scriptEngine = engineFactory.createScriptEngine(scriptTypes.get(0)); + if (scriptEngine != null) { + javax.script.ScriptEngineFactory factory = scriptEngine.getFactory(); + parameterOptions.put(getPreferredMimeType(factory), getLanguageName(factory)); + logger.trace("ParameterOptions: {}", parameterOptions); + } else { + logger.trace("setScriptEngineFactory: engine was null"); + } } else { - logger.trace("setScriptEngineFactory: engine was null"); + logger.trace("addScriptEngineFactory: scriptTypes was empty"); } } public void unsetScriptEngineFactory(ScriptEngineFactory engineFactory) { - ScriptEngine scriptEngine = engineFactory.createScriptEngine(engineFactory.getScriptTypes().get(0)); - if (scriptEngine != null) { - javax.script.ScriptEngineFactory factory = scriptEngine.getFactory(); - parameterOptions.remove(getPreferredMimeType(factory)); - logger.trace("ParameterOptions: {}", parameterOptions); + List scriptTypes = engineFactory.getScriptTypes(); + if (!scriptTypes.isEmpty()) { + ScriptEngine scriptEngine = engineFactory.createScriptEngine(scriptTypes.get(0)); + if (scriptEngine != null) { + javax.script.ScriptEngineFactory factory = scriptEngine.getFactory(); + parameterOptions.remove(getPreferredMimeType(factory)); + logger.trace("ParameterOptions: {}", parameterOptions); + } else { + logger.trace("unsetScriptEngineFactory: engine was null"); + } } else { - logger.trace("unsetScriptEngineFactory: engine was null"); + logger.trace("unsetScriptEngineFactory: scriptTypes was empty"); } } diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/type/ActionType.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/type/ActionType.java index e7062663e28..b99564e8c59 100644 --- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/type/ActionType.java +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/type/ActionType.java @@ -16,6 +16,7 @@ import java.util.List; import java.util.Set; +import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.automation.Action; import org.openhab.core.automation.Module; @@ -32,6 +33,7 @@ * @author Ana Dimova - Initial contribution * @author Vasil Ilchev - Initial contribution */ +@NonNullByDefault public class ActionType extends ModuleType { /** @@ -57,7 +59,7 @@ public class ActionType extends ModuleType { * {@link Action} instances. */ public ActionType(@Nullable String UID, @Nullable List configDescriptions, - List inputs) { + @Nullable List inputs) { this(UID, configDescriptions, inputs, null); } @@ -101,8 +103,8 @@ public ActionType(@Nullable String UID, @Nullable List tags, @Nullable Visibility visibility, @Nullable List inputs, @Nullable List outputs) { super(UID, configDescriptions, label, description, tags, visibility); - this.inputs = inputs != null ? Collections.unmodifiableList(inputs) : Collections.emptyList(); - this.outputs = outputs != null ? Collections.unmodifiableList(outputs) : Collections.emptyList(); + this.inputs = inputs != null ? Collections.unmodifiableList(inputs) : List.of(); + this.outputs = outputs != null ? Collections.unmodifiableList(outputs) : List.of(); } /** diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/type/ConditionType.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/type/ConditionType.java index 01883c5a2c7..fba662102f5 100644 --- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/type/ConditionType.java +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/type/ConditionType.java @@ -16,6 +16,7 @@ import java.util.List; import java.util.Set; +import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.automation.Condition; import org.openhab.core.automation.Visibility; @@ -31,6 +32,7 @@ * @author Ana Dimova - Initial contribution * @author Vasil Ilchev - Initial contribution */ +@NonNullByDefault public class ConditionType extends ModuleType { private final List inputs; @@ -47,8 +49,7 @@ public class ConditionType extends ModuleType { */ public ConditionType(@Nullable String UID, @Nullable List configDescriptions, @Nullable List inputs) { - super(UID, configDescriptions); - this.inputs = inputs != null ? Collections.unmodifiableList(inputs) : Collections.emptyList(); + this(UID, configDescriptions, null, null, null, null, inputs); } /** @@ -72,7 +73,7 @@ public ConditionType(@Nullable String UID, @Nullable List tags, @Nullable Visibility visibility, @Nullable List inputs) { super(UID, configDescriptions, label, description, tags, visibility); - this.inputs = inputs != null ? Collections.unmodifiableList(inputs) : Collections.emptyList(); + this.inputs = inputs != null ? Collections.unmodifiableList(inputs) : List.of(); } /**