[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)

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
+        // Memory control
-        if (maxMemoryUsedInBytes > 0)
+        context.setThreadId(Thread.currentThread().getId());
-        {
+        context.setThreadMxBeanWrapper(new AlfrescoScriptThreadMxBeanWrapper());
-            context.setThreadId(Thread.currentThread().getId());
+        context.setStartMemory();
-        }
 
-        // Max stack depth
+        // Max call stack depth
-        if (maxStackDepth > 0)
+        setMaxStackDepth(context, true);
-        {
-            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);
-        }
 
         return context;
     }
@@ -112,18 +100,16 @@ public class AlfrescoContextFactory extends ContextFactory
                 }
             }
 
-            // Memory
+            // Memory limit
-            if (maxMemoryUsedInBytes > 0 && threadMxBeanWrapper != null && threadMxBeanWrapper.isThreadAllocatedMemorySupported())
+            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 (acx.getUsedMemory() >= maxMemoryUsedInBytes)
-                    if (currentAllocatedBytes - acx.getStartMemory() >= 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();
+            AlfrescoScriptThreadMxBeanWrapper tmxw = new AlfrescoScriptThreadMxBeanWrapper();
-            if (!threadMxBeanWrapper.isThreadAllocatedMemorySupported())
+            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.");

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;
+    }
 }

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
         {

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())
+            if (callLogger.isDebugEnabled())
-            {
+            {
-                long endTime = System.nanoTime();
+                long endTime = System.nanoTime();
-                callLogger.debug(debugScriptName+" End " + (endTime - startTime)/1000000 + " ms");
+
+                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);
             }
         }
     }

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