Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with coercion to boolean in legacy versions #1402

Open
andreabergia opened this issue Oct 27, 2023 · 5 comments
Open

Problem with coercion to boolean in legacy versions #1402

andreabergia opened this issue Oct 27, 2023 · 5 comments

Comments

@andreabergia
Copy link
Contributor

In legacy versions of the engine (< 1.3), Rhino does some special things for booleans and arrays; in particular:

!!new Boolean(false) => false
!![] => false

This behavior does not happen in more recent version of the engine, but as ServiceNow we still have some scripts that rely on the legacy version of the engine. In particular, we have some code that throws an error, and that can be simplified to this:

var Base = function() {};
var Extending = function() {};
Extending.prototype = Base;
var x = new Extending();
!!x

This throws an error:

org.mozilla.javascript.EcmaError: TypeError: Method "toString" called on incompatible object (org.mozilla.javascript.NativeObject is not an instance of org.mozilla.javascript.BaseFunction). (test#5)
	at app//org.mozilla.javascript.ScriptRuntime.constructError(ScriptRuntime.java:4595)
	at app//org.mozilla.javascript.ScriptRuntime.constructError(ScriptRuntime.java:4576)
	at app//org.mozilla.javascript.ScriptRuntime.typeError(ScriptRuntime.java:4608)
	at app//org.mozilla.javascript.ScriptRuntime.typeErrorById(ScriptRuntime.java:4613)
	at app//org.mozilla.javascript.IdScriptableObject.ensureType(IdScriptableObject.java:832)
	at app//org.mozilla.javascript.BaseFunction.realFunction(BaseFunction.java:367)
	at app//org.mozilla.javascript.BaseFunction.execIdCall(BaseFunction.java:313)
	at app//org.mozilla.javascript.IdFunctionObject.call(IdFunctionObject.java:84)
	at app//org.mozilla.javascript.ScriptableObject.getDefaultValue(ScriptableObject.java:794)
	at app//org.mozilla.javascript.ScriptableObject.getDefaultValue(ScriptableObject.java:769)
	at app//org.mozilla.javascript.ScriptRuntime.toBoolean(ScriptRuntime.java:409)
	at app//org.mozilla.javascript.Interpreter.stack_boolean(Interpreter.java:3507)
	at app//org.mozilla.javascript.Interpreter.interpretLoop(Interpreter.java:1495)
	at app//org.mozilla.javascript.Interpreter.interpret(Interpreter.java:1051)
	at app//org.mozilla.javascript.InterpretedFunction.call(InterpretedFunction.java:87)
	at app//org.mozilla.javascript.ContextFactory.doTopCall(ContextFactory.java:383)
	at app//org.mozilla.javascript.ScriptRuntime.doTopCall(ScriptRuntime.java:3876)
	at app//org.mozilla.javascript.InterpretedFunction.exec(InterpretedFunction.java:100)
	at app//org.mozilla.javascript.Context.evaluateString(Context.java:1170)

I'll be opening a PR to fix this.

@tonygermano
Copy link
Contributor

I don't get an error when I execute this code. !!x evaluates to true, which is the same as when I run the code in a firefox js console.

If I try to evaluate x.toString() firefox also throws a TypeError.

@tonygermano
Copy link
Contributor

Oh, I see. It only throws the TypeError for !!x when the Rhino version is 100 or 120 (but not 0.)

@tonygermano
Copy link
Contributor

tonygermano commented Nov 2, 2023

So, the actual difference in behavior appears to be that !x calls x.valueOf() in version 100-120, but it does not call that method in higher versions.

@andreabergia
Copy link
Contributor Author

Yes, it's this snippet in ScriptRuntime::toBoolean:

                if (Context.getContext().isVersionECMA1()) {
                    // pure ECMA
                    return true;
                }
                // ECMA extension
                val = ((Scriptable) val).getDefaultValue(BooleanClass);
                if ((val instanceof Scriptable) && !isSymbol(val))
                    throw errorWithClassName("msg.primitive.expected", val);
                continue;

@tonygermano
Copy link
Contributor

I do not believe this is a regression. See comments on #1403

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants