javadelight / delight-rhino-sandbox

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

Class shutter is not working as expected #17

Closed gtskaushik closed 3 years ago

gtskaushik commented 3 years ago

Analysis

  1. When the RhinoSandboxImpl.assertSafeScope(Context context) is called for the first time, we create this.safeScope and also set the classShutter on the context.
  2. But for the subsequent runs, since this.safeScope is not null, the code path does not reach the line to set the classShutter

This can be shown with the testcase below

public class ScriptEngineServiceTest {
  @Test
  public void javaTestFails() {
    var rhinoSandbox = RhinoSandboxes.create();
    String jsCode1 = "(function a() {var obj = {x:34,y:100}; return obj;})()";
    rhinoSandbox.eval(null, jsCode1);

    assertThrows(EcmaError.class, () -> {
      String jsCode2 = "(function b() {java.lang.System.out.println('hello');})()";
      rhinoSandbox.eval(null, jsCode2);  
    });
  }

  @Test
  public void javaTestPasses() {
    var rhinoSandbox = RhinoSandboxes.create();
    assertThrows(EcmaError.class, () -> {
      String jsCode2 = "(function b() {java.lang.System.out.println('hello');})()";
      System.out.println(rhinoSandbox.eval(null, jsCode2));
    });
  }
}

Proposal

In SafeContext have one more field to store the classShutter and set this for each context in SafeContext.makeContext method

gtskaushik commented 3 years ago

@mxro If the proposed fix is fine, I can create a PR for the same

mxro commented 3 years ago

@gtskaushik Thank you for reporting this issue. That sounds like an excellent idea! Please be welcome to prepare the PR!

gtskaushik commented 3 years ago

@mxro Created the PR - https://github.com/javadelight/delight-rhino-sandbox/pull/18

mxro commented 3 years ago

Can you confirm this is now all fixed and close the issue if that is the case? Thank you!

gtskaushik commented 3 years ago

It is working now. Closing the issue