RemoveComments (JsSanitizer#checkBraces) fails if a string has an unescaped double quote in it

[sebi88]

if value of js is:

var a = '"'
//some comment

RemoveComments.perform(js);

returns:

var a = '"'
//some comment

So RemoveComment's state machine loses the track.

This input works fine:

var a = '\"'
//some comment

version: delight-nashorn-sandbox-0.1.26.jar

[mxro]

Hello @sebi88 - thank you for raising this issue.

Could you provide a bit of background why you would need to remove the comments? Maybe this could help us come up with a workaround or come up with a good fix.

Otherwise, can you spot anything obvious in the code that may be causing this: https://github.com/javadelight/delight-nashorn-sandbox/blob/146398c3a85935190ec59557499b5997f0b345ee/src/main/java/delight/nashornsandbox/internal/RemoveComments.java ?

[sebi88]

hi @mxro , thanks for checking the issue, the big picture is that this code snippet:

  public static void main(String[] args) throws ScriptCPUAbuseException, ScriptException, NoSuchMethodException {
    NashornSandbox sandbox = NashornSandboxes.create();
    sandbox.setExecutor(Executors.newSingleThreadExecutor());
    String function = "function execute() {\n" +
        "var output = {};\n" +
        "var addOutput = function(name, value) {\n" +
        " output[name] = value;\n" +
        "};\n" +
        "a = '\"'\n" +
        "//something for testing\n" +
        "addOutput(\"x\", a);" +
        "return output;\n" +
        "}";
    sandbox.eval(function);
    Invocable invocable = sandbox.getSandboxedInvocable();
    Object result = invocable.invokeFunction("execute");
    System.out.println(result);
  }

throws an exception:

Exception in thread "main" delight.nashornsandbox.exceptions.BracesException: No block braces after function|for|while|do. Found [for testing
 addOutput("x", a);
 return output;
}]
	at delight.nashornsandbox.internal.JsSanitizer.checkBraces(JsSanitizer.java:211)
	at delight.nashornsandbox.internal.JsSanitizer.secureJsImpl(JsSanitizer.java:269)
	at delight.nashornsandbox.internal.JsSanitizer.secureJs(JsSanitizer.java:249)
	at delight.nashornsandbox.internal.NashornSandboxImpl.eval(NashornSandboxImpl.java:213)
	at delight.nashornsandbox.internal.NashornSandboxImpl.eval(NashornSandboxImpl.java:190)
	at xxx.operations.functions.JavaScriptFunction.main(JavaScriptFunction.java:87)

Notice, that the for word is in a comment, but this just an indicator, the real problem is the handling of a = '\"' in RemoveComments.

The workaround is to set: sandbox.allowNoBraces(true); And this is not urgent to me to be fixed, just wanted to report it, because interesting :)

[mxro]

☝️ The above is not the cleanest fix for the issue but appears to work for now. There appears to be something wrong with the logic to remove braces. If anyone would like to look into a better fix, pleas be welcome.

Otherwise this is released now in 0.1.30. Please let me know if this resolves your issue!

[sebi88]

thx!