Clean up arguments after executing Script Transformation (#4524)

Signed-off-by: Jimmy Tanagra <[email protected]>
openhab · Dec 28, 2024 · 8b68aa7 · 8b68aa7
1 parent 1f9ba2b
commit 8b68aa7
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 6 deletions.
diff --git a/.../src/main/java/org/openhab/core/automation/module/script/ScriptTransformationService.java b/.../src/main/java/org/openhab/core/automation/module/script/ScriptTransformationService.java
@@ -17,6 +17,7 @@
 import java.net.URI;
 import java.net.URLDecoder;
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Locale;
@@ -185,8 +186,10 @@ public void deactivate() {
                         : scriptEngineContainer.getScriptEngine();
                 ScriptContext executionContext = engine.getContext();
                 executionContext.setAttribute("input", source, ScriptContext.ENGINE_SCOPE);
+                ArrayList<String> injectedParams = null;
 
                 if (params != null) {
+                    injectedParams = new ArrayList<>();
                     for (String param : params.split("&")) {
                         String[] splitString = param.split("=");
                         if (splitString.length != 2) {
@@ -197,6 +200,7 @@ public void deactivate() {
                             param = URLDecoder.decode(splitString[0], StandardCharsets.UTF_8);
                             String value = URLDecoder.decode(splitString[1], StandardCharsets.UTF_8);
                             executionContext.setAttribute(param, value, ScriptContext.ENGINE_SCOPE);
+                            injectedParams.add(param);
                         }
                     }
                 }
@@ -210,8 +214,15 @@ public void deactivate() {
                     scriptRecord.compiledScript = compiledScript;
                 }
 
-                Object result = compiledScript != null ? compiledScript.eval() : engine.eval(scriptRecord.script);
-                return result == null ? null : result.toString();
+                try {
+                    Object result = compiledScript != null ? compiledScript.eval() : engine.eval(scriptRecord.script);
+                    return result == null ? null : result.toString();
+                } finally {
+                    if (injectedParams != null) {
+                        injectedParams
+                                .forEach(param -> executionContext.removeAttribute(param, ScriptContext.ENGINE_SCOPE));
+                    }
+                }
             } catch (ScriptException e) {
                 throw new TransformationException("Failed to execute script.", e);
             } catch (IllegalStateException e) {

diff --git a/.../test/java/org/openhab/core/automation/module/script/ScriptTransformationServiceTest.java b/.../test/java/org/openhab/core/automation/module/script/ScriptTransformationServiceTest.java
@@ -112,7 +112,6 @@ public void scriptExecutionParametersAreInjectedIntoEngineContext() throws Trans
         verify(scriptContext).setAttribute(eq("input"), eq("input"), eq(ScriptContext.ENGINE_SCOPE));
         verify(scriptContext).setAttribute(eq("param1"), eq("value1"), eq(ScriptContext.ENGINE_SCOPE));
         verify(scriptContext).setAttribute(eq("param2"), eq("value2"), eq(ScriptContext.ENGINE_SCOPE));
-        verifyNoMoreInteractions(scriptContext);
     }
 
     @Test
@@ -122,7 +121,6 @@ public void scriptExecutionParametersAreDecoded() throws TransformationException
         verify(scriptContext).setAttribute(eq("input"), eq("input"), eq(ScriptContext.ENGINE_SCOPE));
         verify(scriptContext).setAttribute(eq("param1"), eq("&amp;"), eq(ScriptContext.ENGINE_SCOPE));
         verify(scriptContext).setAttribute(eq("param2"), eq("=value"), eq(ScriptContext.ENGINE_SCOPE));
-        verifyNoMoreInteractions(scriptContext);
     }
 
     @Test
@@ -141,7 +139,23 @@ abstract class CompilableScriptEngine implements ScriptEngine, Compilable {
         inOrder.verify(scriptContext, times(2)).setAttribute(anyString(), anyString(), eq(ScriptContext.ENGINE_SCOPE));
         inOrder.verify((Compilable) scriptEngine).compile(SCRIPT);
         inOrder.verify(scriptEngine).eval(SCRIPT);
-        inOrder.verifyNoMoreInteractions();
+    }
+
+    @Test
+    public void scriptAttributesRemovedAfterExecution() throws TransformationException, ScriptException {
+        abstract class CompilableScriptEngine implements ScriptEngine, Compilable {
+        }
+        scriptEngine = mock(CompilableScriptEngine.class);
+
+        when(scriptEngineContainer.getScriptEngine()).thenReturn(scriptEngine);
+        when(scriptEngine.getContext()).thenReturn(scriptContext);
+
+        InOrder inOrder = inOrder(scriptContext, scriptEngine);
+
+        service.transform(SCRIPT_UID + "?param1=value1", "input");
+
+        inOrder.verify(scriptEngine).eval(SCRIPT);
+        inOrder.verify(scriptContext).removeAttribute(eq("param1"), eq(ScriptContext.ENGINE_SCOPE));
     }
 
     @Test
@@ -150,7 +164,7 @@ public void invalidScriptExecutionParametersAreDiscarded() throws Transformation
 
         verify(scriptContext).setAttribute(eq("input"), eq("input"), eq(ScriptContext.ENGINE_SCOPE));
         verify(scriptContext).setAttribute(eq("param1"), eq("value1"), eq(ScriptContext.ENGINE_SCOPE));
-        verifyNoMoreInteractions(scriptContext);
+        verify(scriptContext, times(0)).setAttribute(eq("invalid"), any(), eq(ScriptContext.ENGINE_SCOPE));
     }
 
     @Test