diff --git a/application/src/main/java/org/thingsboard/server/service/script/AbstractJsInvokeService.java b/application/src/main/java/org/thingsboard/server/service/script/AbstractJsInvokeService.java index eef49ae518..0af1f4846b 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/AbstractJsInvokeService.java +++ b/application/src/main/java/org/thingsboard/server/service/script/AbstractJsInvokeService.java @@ -17,7 +17,10 @@ package org.thingsboard.server.service.script; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; +import lombok.Getter; import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Value; import org.thingsboard.common.util.ThingsBoardThreadFactory; import org.thingsboard.server.common.data.ApiUsageRecordKey; import org.thingsboard.server.common.data.id.CustomerId; @@ -30,9 +33,10 @@ import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; +import static java.lang.String.format; + /** * Created by ashvayka on 26.09.18. */ @@ -45,6 +49,16 @@ public abstract class AbstractJsInvokeService implements JsInvokeService { protected Map scriptIdToNameMap = new ConcurrentHashMap<>(); protected Map disabledFunctions = new ConcurrentHashMap<>(); + @Getter + @Value("${js.max_total_args_size:100000}") + private long maxTotalArgsSize; + @Getter + @Value("${js.max_result_size:300000}") + private long maxResultSize; + @Getter + @Value("${js.max_script_body_size:50000}") + private long maxScriptBodySize; + protected AbstractJsInvokeService(TbApiUsageStateService apiUsageStateService, TbApiUsageClient apiUsageClient) { this.apiUsageStateService = apiUsageStateService; this.apiUsageClient = apiUsageClient; @@ -65,33 +79,45 @@ public abstract class AbstractJsInvokeService implements JsInvokeService { @Override public ListenableFuture eval(TenantId tenantId, JsScriptType scriptType, String scriptBody, String... argNames) { if (apiUsageStateService.getApiUsageState(tenantId).isJsExecEnabled()) { + if (scriptBodySizeExceeded(scriptBody)) { + return error(format("Script body exceeds maximum allowed size of %s symbols", getMaxScriptBodySize())); + } UUID scriptId = UUID.randomUUID(); String functionName = "invokeInternal_" + scriptId.toString().replace('-', '_'); String jsScript = generateJsScript(scriptType, functionName, scriptBody, argNames); return doEval(scriptId, functionName, jsScript); } else { - return Futures.immediateFailedFuture(new RuntimeException("JS Execution is disabled due to API limits!")); + return error("JS Execution is disabled due to API limits!"); } } @Override - public ListenableFuture invokeFunction(TenantId tenantId, CustomerId customerId, UUID scriptId, Object... args) { + public ListenableFuture invokeFunction(TenantId tenantId, CustomerId customerId, UUID scriptId, Object... args) { if (apiUsageStateService.getApiUsageState(tenantId).isJsExecEnabled()) { String functionName = scriptIdToNameMap.get(scriptId); if (functionName == null) { - return Futures.immediateFailedFuture(new RuntimeException("No compiled script found for scriptId: [" + scriptId + "]!")); + return error("No compiled script found for scriptId: [" + scriptId + "]!"); } if (!isDisabled(scriptId)) { + if (argsSizeExceeded(args)) { + return scriptExecutionError(scriptId, format("Script input arguments exceed maximum allowed total args size of %s symbols", getMaxTotalArgsSize())); + } apiUsageClient.report(tenantId, customerId, ApiUsageRecordKey.JS_EXEC_COUNT, 1); - return doInvokeFunction(scriptId, functionName, args); + return Futures.transformAsync(doInvokeFunction(scriptId, functionName, args), output -> { + String result = output.toString(); + if (resultSizeExceeded(result)) { + return scriptExecutionError(scriptId, format("Script invocation result exceeds maximum allowed size of %s symbols", getMaxResultSize())); + } + return Futures.immediateFuture(result); + }, MoreExecutors.directExecutor()); } else { String message = "Script invocation is blocked due to maximum error count " + getMaxErrors() + ", scriptId " + scriptId + "!"; log.warn(message); - return Futures.immediateFailedFuture(new RuntimeException(message)); + return error(message); } } else { - return Futures.immediateFailedFuture(new RuntimeException("JS Execution is disabled due to API limits!")); + return error("JS Execution is disabled due to API limits!"); } } @@ -127,6 +153,27 @@ public abstract class AbstractJsInvokeService implements JsInvokeService { disableListInfo.incrementAndGet(); } + private boolean scriptBodySizeExceeded(String scriptBody) { + if (getMaxScriptBodySize() <= 0) return false; + return scriptBody.length() > getMaxScriptBodySize(); + } + + private boolean argsSizeExceeded(Object[] args) { + if (getMaxTotalArgsSize() <= 0) return false; + long totalArgsSize = 0; + for (Object arg : args) { + if (arg instanceof CharSequence) { + totalArgsSize += ((CharSequence) arg).length(); + } + } + return totalArgsSize > getMaxTotalArgsSize(); + } + + private boolean resultSizeExceeded(String result) { + if (getMaxResultSize() <= 0) return false; + return result.length() > getMaxResultSize(); + } + private String generateJsScript(JsScriptType scriptType, String functionName, String scriptBody, String... argNames) { if (scriptType == JsScriptType.RULE_NODE_SCRIPT) { return RuleNodeScriptFactory.generateRuleNodeScript(functionName, scriptBody, argNames); @@ -148,6 +195,16 @@ public abstract class AbstractJsInvokeService implements JsInvokeService { } } + private ListenableFuture error(String message) { + return Futures.immediateFailedFuture(new RuntimeException(message)); + } + + private ListenableFuture scriptExecutionError(UUID scriptId, String errorMsg) { + RuntimeException error = new RuntimeException(errorMsg); + onScriptExecutionError(scriptId, error, null); + return Futures.immediateFailedFuture(error); + } + private class DisableListInfo { private final AtomicInteger counter; private long expirationTime; diff --git a/application/src/main/java/org/thingsboard/server/service/script/JsInvokeService.java b/application/src/main/java/org/thingsboard/server/service/script/JsInvokeService.java index 3e5d014bef..f9af2f83e4 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/JsInvokeService.java +++ b/application/src/main/java/org/thingsboard/server/service/script/JsInvokeService.java @@ -25,7 +25,7 @@ public interface JsInvokeService { ListenableFuture eval(TenantId tenantId, JsScriptType scriptType, String scriptBody, String... argNames); - ListenableFuture invokeFunction(TenantId tenantId, CustomerId customerId, UUID scriptId, Object... args); + ListenableFuture invokeFunction(TenantId tenantId, CustomerId customerId, UUID scriptId, Object... args); ListenableFuture release(UUID scriptId); diff --git a/application/src/main/java/org/thingsboard/server/service/script/NashornJsInvokeService.java b/application/src/main/java/org/thingsboard/server/service/script/NashornJsInvokeService.java index 5d64017f33..4b9e5dea57 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/NashornJsInvokeService.java +++ b/application/src/main/java/org/thingsboard/server/service/script/NashornJsInvokeService.java @@ -15,6 +15,7 @@ */ package org.thingsboard.server.service.script; +import lombok.Getter; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @@ -32,12 +33,15 @@ public class NashornJsInvokeService extends AbstractNashornJsInvokeService { @Value("${js.local.use_js_sandbox}") private boolean useJsSandbox; + @Getter @Value("${js.local.monitor_thread_pool_size}") private int monitorThreadPoolSize; + @Getter @Value("${js.local.max_cpu_time}") private long maxCpuTime; + @Getter @Value("${js.local.max_errors}") private int maxErrors; @@ -53,21 +57,6 @@ public class NashornJsInvokeService extends AbstractNashornJsInvokeService { return useJsSandbox; } - @Override - protected int getMonitorThreadPoolSize() { - return monitorThreadPoolSize; - } - - @Override - protected long getMaxCpuTime() { - return maxCpuTime; - } - - @Override - protected int getMaxErrors() { - return maxErrors; - } - @Override protected long getMaxBlacklistDuration() { return TimeUnit.SECONDS.toMillis(maxBlackListDurationSec); diff --git a/application/src/main/java/org/thingsboard/server/service/script/RuleNodeJsScriptEngine.java b/application/src/main/java/org/thingsboard/server/service/script/RuleNodeJsScriptEngine.java index c90e104ff4..f1374711d5 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/RuleNodeJsScriptEngine.java +++ b/application/src/main/java/org/thingsboard/server/service/script/RuleNodeJsScriptEngine.java @@ -210,11 +210,11 @@ public class RuleNodeJsScriptEngine implements org.thingsboard.rule.engine.api.S return executeScriptAsync(msg.getCustomerId(), inArgs[0], inArgs[1], inArgs[2]); } - ListenableFuture executeScriptAsync(CustomerId customerId, Object... args) { + ListenableFuture executeScriptAsync(CustomerId customerId, String... args) { return Futures.transformAsync(sandboxService.invokeFunction(tenantId, customerId, this.scriptId, args), o -> { try { - return Futures.immediateFuture(mapper.readTree(o.toString())); + return Futures.immediateFuture(mapper.readTree(o)); } catch (Exception e) { if (e.getCause() instanceof ScriptException) { return Futures.immediateFailedFuture(e.getCause()); diff --git a/application/src/main/resources/thingsboard.yml b/application/src/main/resources/thingsboard.yml index 3e85852a20..57e06b6563 100644 --- a/application/src/main/resources/thingsboard.yml +++ b/application/src/main/resources/thingsboard.yml @@ -595,6 +595,9 @@ state: js: evaluator: "${JS_EVALUATOR:local}" # local/remote + max_total_args_size: "${JS_MAX_TOTAL_ARGS_SIZE:100000}" + max_result_size: "${JS_MAX_RESULT_SIZE:300000}" + max_script_body_size: "${JS_MAX_SCRIPT_BODY_SIZE:50000}" # Built-in JVM JavaScript environment properties local: # Use Sandboxed (secured) JVM JavaScript environment diff --git a/application/src/test/java/org/thingsboard/server/service/script/JsInvokeServiceTest.java b/application/src/test/java/org/thingsboard/server/service/script/JsInvokeServiceTest.java new file mode 100644 index 0000000000..8f0edd71c6 --- /dev/null +++ b/application/src/test/java/org/thingsboard/server/service/script/JsInvokeServiceTest.java @@ -0,0 +1,96 @@ +/** + * Copyright © 2016-2022 The Thingsboard Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.thingsboard.server.service.script; + +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.test.context.TestPropertySource; +import org.thingsboard.server.common.data.id.TenantId; +import org.thingsboard.server.controller.AbstractControllerTest; +import org.thingsboard.server.dao.service.DaoSqlTest; + +import java.util.UUID; +import java.util.concurrent.ExecutionException; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +@DaoSqlTest +@TestPropertySource(properties = { + "js.max_script_body_size=50", + "js.max_total_args_size=50", + "js.max_result_size=50", + "js.local.max_errors=2" +}) +class JsInvokeServiceTest extends AbstractControllerTest { + + @Autowired + private NashornJsInvokeService jsInvokeService; + + @Value("${js.local.max_errors}") + private int maxJsErrors; + + @Test + void givenTooBigScriptForEval_thenReturnError() { + String hugeScript = "var a = 'qwertyqwertywertyqwabababer'; return {a: a};"; + + assertThatThrownBy(() -> { + evalScript(hugeScript); + }).hasMessageContaining("body exceeds maximum allowed size"); + } + + @Test + void givenTooBigScriptInputArgs_thenReturnErrorAndReportScriptExecutionError() throws Exception { + String script = "return { msg: msg };"; + String hugeMsg = "{\"input\":\"123456781234349\"}"; + UUID scriptId = evalScript(script); + + for (int i = 0; i < maxJsErrors; i++) { + assertThatThrownBy(() -> { + invokeScript(scriptId, hugeMsg); + }).hasMessageContaining("input arguments exceed maximum"); + } + assertThatScriptIsBlocked(scriptId); + } + + @Test + void whenScriptInvocationResultIsTooBig_thenReturnErrorAndReportScriptExecutionError() throws Exception { + String script = "var s = new Array(50).join('a'); return { s: s};"; + UUID scriptId = evalScript(script); + + for (int i = 0; i < maxJsErrors; i++) { + assertThatThrownBy(() -> { + invokeScript(scriptId, "{}"); + }).hasMessageContaining("result exceeds maximum allowed size"); + } + assertThatScriptIsBlocked(scriptId); + } + + private void assertThatScriptIsBlocked(UUID scriptId) { + assertThatThrownBy(() -> { + invokeScript(scriptId, "{}"); + }).hasMessageContaining("invocation is blocked due to maximum error"); + } + + private UUID evalScript(String script) throws ExecutionException, InterruptedException { + return jsInvokeService.eval(TenantId.SYS_TENANT_ID, JsScriptType.RULE_NODE_SCRIPT, script).get(); + } + + private String invokeScript(UUID scriptId, String msg) throws ExecutionException, InterruptedException { + return jsInvokeService.invokeFunction(TenantId.SYS_TENANT_ID, null, scriptId, msg, "{}", "POST_TELEMETRY_REQUEST").get(); + } + +} diff --git a/application/src/test/java/org/thingsboard/server/service/script/MockJsInvokeService.java b/application/src/test/java/org/thingsboard/server/service/script/MockJsInvokeService.java index 17803bac96..1d91f50131 100644 --- a/application/src/test/java/org/thingsboard/server/service/script/MockJsInvokeService.java +++ b/application/src/test/java/org/thingsboard/server/service/script/MockJsInvokeService.java @@ -37,7 +37,7 @@ public class MockJsInvokeService implements JsInvokeService { } @Override - public ListenableFuture invokeFunction(TenantId tenantId, CustomerId customerId, UUID scriptId, Object... args) { + public ListenableFuture invokeFunction(TenantId tenantId, CustomerId customerId, UUID scriptId, Object... args) { log.warn("invokeFunction {} {} {} {}", tenantId, customerId, scriptId, args); return Futures.immediateFuture("{}"); } diff --git a/msa/js-executor/api/jsInvokeMessageProcessor.ts b/msa/js-executor/api/jsInvokeMessageProcessor.ts index 0a101775e4..f1b60b6e07 100644 --- a/msa/js-executor/api/jsInvokeMessageProcessor.ts +++ b/msa/js-executor/api/jsInvokeMessageProcessor.ts @@ -43,6 +43,7 @@ const useSandbox = config.get('script.use_sandbox') === 'true'; const maxActiveScripts = Number(config.get('script.max_active_scripts')); const slowQueryLogMs = Number(config.get('script.slow_query_log_ms')); const slowQueryLogBody = config.get('script.slow_query_log_body') === 'true'; +const maxResultSize = Number(config.get('js.max_result_size')); export class JsInvokeMessageProcessor { @@ -164,9 +165,19 @@ export class JsInvokeMessageProcessor { (script) => { this.executor.executeScript(script, invokeRequest.args, invokeRequest.timeout).then( (result) => { - const invokeResponse = JsInvokeMessageProcessor.createInvokeResponse(result, true); - this.logger.debug('[%s] Sending success invoke response, scriptId: [%s]', requestId, scriptId); - this.sendResponse(requestId, responseTopic, headers, scriptId, undefined, invokeResponse); + if (result.length <= maxResultSize) { + const invokeResponse = JsInvokeMessageProcessor.createInvokeResponse(result, true); + this.logger.debug('[%s] Sending success invoke response, scriptId: [%s]', requestId, scriptId); + this.sendResponse(requestId, responseTopic, headers, scriptId, undefined, invokeResponse); + } else { + let err = { + name: 'Error', + message: 'script invocation result exceeds maximum allowed size of ' + maxResultSize + ' symbols' + } + const invokeResponse = JsInvokeMessageProcessor.createInvokeResponse("", false, RUNTIME_ERROR, err); + this.logger.debug('[%s] Script invocation result exceeds maximum allowed size of %s symbols, scriptId: [%s]', requestId, maxResultSize, scriptId); + this.sendResponse(requestId, responseTopic, headers, scriptId, undefined, invokeResponse); + } }, (err: any) => { let errorCode; diff --git a/msa/js-executor/config/custom-environment-variables.yml b/msa/js-executor/config/custom-environment-variables.yml index d408b14136..b9c24c8d8d 100644 --- a/msa/js-executor/config/custom-environment-variables.yml +++ b/msa/js-executor/config/custom-environment-variables.yml @@ -20,6 +20,7 @@ http_port: "HTTP_PORT" # /livenessProbe js: response_poll_interval: "REMOTE_JS_RESPONSE_POLL_INTERVAL_MS" + max_result_size: "JS_MAX_RESULT_SIZE" kafka: bootstrap: diff --git a/msa/js-executor/config/default.yml b/msa/js-executor/config/default.yml index 32163ea01e..64829ef792 100644 --- a/msa/js-executor/config/default.yml +++ b/msa/js-executor/config/default.yml @@ -20,6 +20,7 @@ http_port: "8888" # /livenessProbe js: response_poll_interval: "25" + max_result_size: "300000" kafka: bootstrap: