javadelight / delight-rhino-sandbox

A sandbox to execute JavaScript code with Rhino in Java.
Other
38 stars 12 forks source link

InstructionLimit calculation #22

Open liebig opened 3 years ago

liebig commented 3 years ago

In what unit or in what measure is InstructionLimit calculated? Unfortunately, I don't really understand how the value is measured.

Because with sandbox.eval("app.js", "var i = 0; while (true) { logger.info(i++) }"); and sandbox.setInstructionLimit(1); or sandbox.setInstructionLimit(19000); the console looks like

...
22:32:25.996 [main] INFO  TestApp - 86
22:32:25.997 [main] INFO  TestApp - 87
Exception in thread "main" delight.rhinosandox.exceptions.ScriptCPUAbuseException

But with sandbox.setInstructionLimit(19001); the console looks like

...
22:34:29.718 [main] INFO  TestApp - 174
22:34:29.719 [main] INFO  TestApp - 175
Exception in thread "main" delight.rhinosandox.exceptions.ScriptCPUAbuseException
mxro commented 3 years ago

Thank you for your question.

This is how the instruction limit is calculated https://github.com/javadelight/delight-rhino-sandbox/blob/master/src/main/java/delight/rhinosandox/internal/SafeContext.java#L68 (by hooking into observeInstructionCount from ContextFactory). Would it work better here to calculate this in a different way?

liebig commented 3 years ago

Thanks for the quick reply. I took a closer look at the implementation and now I understand it. I had wondered when the observeInstructionCount was called. However, I would like the INSTRUCTION_STEPS variable to be configurable. Would that make sense?

mxro commented 3 years ago

So if I understand it correctly, we can set the interval in which Rhino will call observeInstructionCount using Context.setInstructionObserverThreshold (see https://github.com/javadelight/delight-rhino-sandbox/blob/master/src/main/java/delight/rhinosandox/internal/SafeContext.java#L39). This is set in SafeContext to the same value that the instruction count is incremented by. That would explain why it is called in larger intervals.

I think the idea behind limiting instruction count is more around trying to prevent abusive scripts from running. So with that in mind, there shouldn't be a big difference between 10,000 and 100,000 steps etc because these should still execute very quickly. Only usecase I could think of for a smaller INSTRUCTIONS_STEPS setting would be if the sandbox is used to execute a very large number of scripts that should all be very small - but I think setup times etc may be a much larger factor here again than the 10,000 bytes instructions limit. What do you think? Do you have a particular usecase where we need to consider the no of instructions in a more fine grained way? If so, we can start with that and see how we can accommodate for this in the best possible way!

liebig commented 3 years ago

Yes, in itself it also really makes sense not to pause after each JavaScript step. My use case is to simply try to pause the JavaScript code at a certain point. Another use case would be to implement a simple debugger in Java that pauses the JavaScript code step by step.

mxro commented 3 years ago

Okay, let's maybe let us leave this issue open and see if there are further requirements for something like this, and we can take it from there?

When you say pause the JavaScript at a certain point, is that for debugging purposes?

liebig commented 3 years ago

Yes exactly, among other things for debugging.