From 95ab83d2b1bebf78271395cbadb17f80ba989d4e Mon Sep 17 00:00:00 2001 From: tiagosalvado10 <9038083+tiagosalvado10@users.noreply.github.com> Date: Sun, 15 Jan 2023 13:07:12 +0000 Subject: [PATCH] [MNT-23396] [MNT-23391] Change call stack depth limit to be only applied to custom scripts / Clean scope only for custom scripts (#1592) * [MNT-23158] Change call stack depth limit to be only applied to custom scripts (as memory and time limits) * [MNT-23158] Changed how memory usage is obtained * [MNT-23391] Only clean scope if script is not considered secure (custom script) --- .../repo/jscript/AlfrescoContextFactory.java | 69 ++++++++++++------- .../repo/jscript/AlfrescoScriptContext.java | 47 ++++++++++++- .../AlfrescoScriptThreadMxBeanWrapper.java | 2 +- .../repo/jscript/RhinoScriptProcessor.java | 26 +++++-- .../resources/alfresco/repository.properties | 2 +- 5 files changed, 112 insertions(+), 34 deletions(-) diff --git a/repository/src/main/java/org/alfresco/repo/jscript/AlfrescoContextFactory.java b/repository/src/main/java/org/alfresco/repo/jscript/AlfrescoContextFactory.java index c7bbb055fb..8dc84f21cc 100644 --- a/repository/src/main/java/org/alfresco/repo/jscript/AlfrescoContextFactory.java +++ b/repository/src/main/java/org/alfresco/repo/jscript/AlfrescoContextFactory.java @@ -47,8 +47,6 @@ public class AlfrescoContextFactory extends ContextFactory private long maxMemoryUsedInBytes = -1L; private int observeInstructionCount = -1; - private AlfrescoScriptThreadMxBeanWrapper threadMxBeanWrapper; - private final int INTERPRETIVE_MODE = -1; @Override @@ -74,23 +72,13 @@ public class AlfrescoContextFactory extends ContextFactory } } - // Memory limit - if (maxMemoryUsedInBytes > 0) - { - context.setThreadId(Thread.currentThread().getId()); - } + // Memory control + context.setThreadId(Thread.currentThread().getId()); + context.setThreadMxBeanWrapper(new AlfrescoScriptThreadMxBeanWrapper()); + context.setStartMemory(); - // Max stack depth - if (maxStackDepth > 0) - { - if (optimizationLevel != INTERPRETIVE_MODE) - { - LOGGER.warn("Changing optimization level from " + optimizationLevel + " to " + INTERPRETIVE_MODE); - } - // stack depth can only be set when no optimizations are applied - context.setOptimizationLevel(INTERPRETIVE_MODE); - context.setMaximumInterpreterStackDepth(maxStackDepth); - } + // Max call stack depth + setMaxStackDepth(context, true); return context; } @@ -112,18 +100,16 @@ public class AlfrescoContextFactory extends ContextFactory } } - // Memory - if (maxMemoryUsedInBytes > 0 && threadMxBeanWrapper != null && threadMxBeanWrapper.isThreadAllocatedMemorySupported()) + // Memory limit + if (maxMemoryUsedInBytes > 0 && acx.isMemoryLimitSupported()) { - if (acx.getStartMemory() <= 0) { - acx.setStartMemory(threadMxBeanWrapper.getThreadAllocatedBytes(acx.getThreadId())); + acx.setStartMemory(); } else { - long currentAllocatedBytes = threadMxBeanWrapper.getThreadAllocatedBytes(acx.getThreadId()); - if (currentAllocatedBytes - acx.getStartMemory() >= maxMemoryUsedInBytes) + if (acx.getUsedMemory() >= maxMemoryUsedInBytes) { throw new Error("Memory limit of " + maxMemoryUsedInBytes + " bytes reached"); } @@ -137,9 +123,40 @@ public class AlfrescoContextFactory extends ContextFactory { AlfrescoScriptContext acx = (AlfrescoScriptContext) cx; acx.setStartTime(System.currentTimeMillis()); + setMaxStackDepth(acx, acx.isLimitsEnabled()); return super.doTopCall(callable, cx, scope, thisObj, args); } + private void setMaxStackDepth(AlfrescoScriptContext acx, boolean enable) + { + if (enable) + { + // Max stack depth + if (maxStackDepth > 0 && maxStackDepth != acx.getMaximumInterpreterStackDepth()) + { + LOGGER.debug("Max call stack depth limit will be enabled with value: " + maxStackDepth); + + if (optimizationLevel != INTERPRETIVE_MODE) + { + LOGGER.debug("Changing optimization level from " + optimizationLevel + " to " + INTERPRETIVE_MODE); + } + // stack depth can only be set in interpretive mode + acx.setOptimizationLevel(INTERPRETIVE_MODE); + acx.setMaximumInterpreterStackDepth(maxStackDepth); + } + } + else + { + if (acx.getMaximumInterpreterStackDepth() != Integer.MAX_VALUE) + { + LOGGER.debug("Max call stack depth limit will be set to default value: " + Integer.MAX_VALUE); + acx.setOptimizationLevel(INTERPRETIVE_MODE); + acx.setMaximumInterpreterStackDepth(Integer.MAX_VALUE); + acx.setOptimizationLevel(optimizationLevel); + } + } + } + public int getOptimizationLevel() { return optimizationLevel; @@ -180,8 +197,8 @@ public class AlfrescoContextFactory extends ContextFactory this.maxMemoryUsedInBytes = maxMemoryUsedInBytes; if (maxMemoryUsedInBytes > 0) { - this.threadMxBeanWrapper = new AlfrescoScriptThreadMxBeanWrapper(); - if (!threadMxBeanWrapper.isThreadAllocatedMemorySupported()) + AlfrescoScriptThreadMxBeanWrapper tmxw = new AlfrescoScriptThreadMxBeanWrapper(); + if (!tmxw.isThreadAllocatedMemorySupported()) { LOGGER.warn("com.sun.management.ThreadMXBean was not found on the classpath. " + "This means that the limiting the memory usage for a script will NOT work."); diff --git a/repository/src/main/java/org/alfresco/repo/jscript/AlfrescoScriptContext.java b/repository/src/main/java/org/alfresco/repo/jscript/AlfrescoScriptContext.java index cc5a4d6b20..d8942a75ac 100644 --- a/repository/src/main/java/org/alfresco/repo/jscript/AlfrescoScriptContext.java +++ b/repository/src/main/java/org/alfresco/repo/jscript/AlfrescoScriptContext.java @@ -25,6 +25,8 @@ */ package org.alfresco.repo.jscript; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.mozilla.javascript.Context; /** @@ -36,8 +38,36 @@ public class AlfrescoScriptContext extends Context { private long startTime; private long threadId; - private long startMemory; + private long startMemory = -1; private boolean limitsEnabled = false; + private AlfrescoScriptThreadMxBeanWrapper threadMxBeanWrapper = null; + + public void setStartMemory() + { + if (isMemoryLimitSupported()) + { + startMemory = threadMxBeanWrapper.getThreadAllocatedBytes(threadId); + } + } + + public long getUsedMemory() + { + long usedMemory = -1; + + if (isMemoryLimitSupported()) + { + long currentAllocatedBytes = threadMxBeanWrapper.getThreadAllocatedBytes(threadId); + usedMemory = currentAllocatedBytes - startMemory; + } + + return usedMemory; + } + + public boolean isMemoryLimitSupported() + { + AlfrescoScriptThreadMxBeanWrapper tmxw = getThreadMxBeanWrapper(); + return tmxw != null && tmxw.isThreadAllocatedMemorySupported(); + } public long getStartTime() { @@ -78,4 +108,19 @@ public class AlfrescoScriptContext extends Context { this.limitsEnabled = limitsEnabled; } + + public AlfrescoScriptThreadMxBeanWrapper getThreadMxBeanWrapper() + { + if (threadMxBeanWrapper == null) + { + threadMxBeanWrapper = new AlfrescoScriptThreadMxBeanWrapper(); + } + + return threadMxBeanWrapper; + } + + public void setThreadMxBeanWrapper(AlfrescoScriptThreadMxBeanWrapper threadMxBeanWrapper) + { + this.threadMxBeanWrapper = threadMxBeanWrapper; + } } \ No newline at end of file diff --git a/repository/src/main/java/org/alfresco/repo/jscript/AlfrescoScriptThreadMxBeanWrapper.java b/repository/src/main/java/org/alfresco/repo/jscript/AlfrescoScriptThreadMxBeanWrapper.java index 0d2bbd9321..fc2570269b 100644 --- a/repository/src/main/java/org/alfresco/repo/jscript/AlfrescoScriptThreadMxBeanWrapper.java +++ b/repository/src/main/java/org/alfresco/repo/jscript/AlfrescoScriptThreadMxBeanWrapper.java @@ -54,7 +54,7 @@ public class AlfrescoScriptThreadMxBeanWrapper return -1; } - public void checkThreadAllocatedMemory() + private void checkThreadAllocatedMemory() { try { diff --git a/repository/src/main/java/org/alfresco/repo/jscript/RhinoScriptProcessor.java b/repository/src/main/java/org/alfresco/repo/jscript/RhinoScriptProcessor.java index 59728e18b9..773860da2a 100644 --- a/repository/src/main/java/org/alfresco/repo/jscript/RhinoScriptProcessor.java +++ b/repository/src/main/java/org/alfresco/repo/jscript/RhinoScriptProcessor.java @@ -614,13 +614,29 @@ public class RhinoScriptProcessor extends BaseProcessor implements ScriptProcess } finally { - unsetScope(model, scope); + if (!secure) + { + unsetScope(model, scope); + } Context.exit(); - if (callLogger.isDebugEnabled()) - { - long endTime = System.nanoTime(); - callLogger.debug(debugScriptName+" End " + (endTime - startTime)/1000000 + " ms"); + if (callLogger.isDebugEnabled()) + { + long endTime = System.nanoTime(); + + String logMessage = debugScriptName + " End " + (endTime - startTime) / 1000000 + " ms"; + + if (cx instanceof AlfrescoScriptContext) + { + AlfrescoScriptContext acx = (AlfrescoScriptContext) cx; + long usedMemory = acx.getUsedMemory(); + if (usedMemory > 0) + { + logMessage += " - Used memory: " + usedMemory + " bytes"; + } + } + + callLogger.debug(logMessage); } } } diff --git a/repository/src/main/resources/alfresco/repository.properties b/repository/src/main/resources/alfresco/repository.properties index 6198dde5fb..de3085bc12 100644 --- a/repository/src/main/resources/alfresco/repository.properties +++ b/repository/src/main/resources/alfresco/repository.properties @@ -1365,4 +1365,4 @@ scripts.execution.maxStackDepth=-1 scripts.execution.maxMemoryUsedInBytes=-1 # Number of instructions that will trigger the observer -scripts.execution.observerInstructionCount=-1 +scripts.execution.observerInstructionCount=5000