Failed to restrict the cpu and memory utilization for few scenarios

[srinivasarajuch]

Hi,

We have identified few scenarios where this library is failing to control the usage.
// Scenario 1:
function main(){
for(var i=0;i<2;i++)
logger.debug("loop cnt-"+i);
}
function main2(){
}
main();

//Scenario 2:
function main(){
logger.debug("... In fun1()....");
for(var i=0;i<2;i++)//{
logger.debug("loop cnt-"+i);
}
main();

//Scenario 3:

//simple do-while loop for demo
function loopTest(){
var i=0;
do{
logger.debug("loop cnt="+(++i));
}while(i<11)
}
loopTest();

//Scenario 4:

if(srctable.length) srctable.length = 0;__if();
else {
for(var key in srctable) {__if();
delete srctable[key];
}
}

All the above scenarios can be easily addressed if apply JSLint/JSHint/ESLint kind of JS libraries. Can you please check any other possibilities here.

[mxro]

Hi Srinivas,

Thank you for exploring different scenarios and reporting this issue.

I've created a test case for all the scenarios that you have listed. I don't see how the CPU/Memory limit would not work for these? They are all scripts that don't go into an infinite loop or do otherwise resource intensive operations.

Please confirm that my tests reflect the scenarios correctly!

I see that the syntax in some of the tests is not quite 'clean'. Please note that assuring that user scripts are provided in 'pretty' and 'clean' JS syntax is out of scope for this library. Its purpose is to create a sandbox for JS in Nashorn.

[srinivasarajuch]

Hi Maxro,

I agree, for scenario 1, 2, 3. As we are using thread monitoring and if the thread is not terminated after some time, we are forcefully stop the thread by making call as threadToMonitor.stop(); (which is deprecated in java now).

But for scenario 4

//simple do-while loop for demo
function loopTest(){
var i=0;
do{
i++;
}while(true)
}
loopTest();

Assuming below is my test java code to run the script
final NashornSandbox sandbox = NashornSandboxes.create();
sandbox.setMaxCPUTime(1000);
sandbox.allowNoBraces(false);
sandbox.setExecutor(Executors.newSingleThreadExecutor());
sandbox.eval(scriptfilecontent);

the above script fails with the below exception. As I have do-while loop for demo in javascript comment it is assuming that the for as keyword and throwing exception as there is no braces. I think this is some thing can be fixed by proper pattern matching.

java.lang.Exception: Unexpected exception, expected<delight.nashornsandbox.exceptions.ScriptCPUAbuseException> but was<delight.nashornsandbox.exceptions.BracesException>
at org.junit.internal.runners.statements.ExpectException.evaluate(ExpectException.java:28)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:76)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:678)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
Caused by: delight.nashornsandbox.exceptions.BracesException: No block braces after function|for|while|do

[mxro]

Hi Srinivas,

I've added the scenario you describe in the test case under testIssue34_Scenario5 in TestIssue34.java. I don't get a BracesException when running this. Only the ScriptCPUException. So all seems fine to me. Please check out my test case locally and compare it to your code.

Please provide me with a full example I can execute myself if things don't work as you expect!

Max

[srinivasarajuch]

Hi Maxro,

This is the updated test cases is failing. Please check

@test
public void testIssue34_Scenario3() throws ScriptCPUAbuseException, ScriptException {
String js = "//simple do-while loop for demo";
js += "function loopTest(){\n" + "var i=0;\n" + "do{\n" + "logger.debug("loop cnt="+(++i));\n"
+ "}while(i<11)\n" + "}\n" + "loopTest();";

	sandbox.eval(js);

	Assert.assertTrue(logger.getOutput().contains("loop cnt=6"));

}

[mxro]

Hi Srinivas,

Thank you for further testing this. Firstly, the code you have submitted misses a '\n' at the end of this line:

String js = "//simple do-while loop for demo";

Should be:

String js = "//simple do-while loop for demo\n";

I have fixed that and added this test case as testIssue34_Scenario3_2.

It looks like the braces exception was triggered by the 'do' included in the first name comment. I have patched the library now so that it will ignore all code in comments in checking for braces. Please note that the updated code is not yet uploaded to Maven central.

Let me know if there is anything else you find.

[srinivasarajuch]

No other issues, please merge fixes

[mxro]

Version 0.1.8 is released to Maven Central and should be available there shortly. Let us know if you have any further issues.

[Frontrider]

I'd like to get more information on this for #52. The tests do not contain any descriptions, and they look like they should either be a different type of test, or injected into the existing scenarios.