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

Circular dependency between Context and ScriptRuntime leads to a NullPointerException (emptyArgs null) #1793

Open
egueidao opened this issue Jan 7, 2025 · 6 comments

Comments

@egueidao
Copy link

egueidao commented Jan 7, 2025

Hello,

I think I have a circular dependency problem between the two classes org.mozilla.javascript.Context and org.mozilla.javascript.ScriptRuntime.
The problem is with the initialization of emptyArgs in Context class which is null and leads to a NullpointerExcption in the MemberBox code when the list of parameters is iterated.

Here is the Stacktrace :

java.lang.NullPointerException: Cannot read the array length because "args" is null
	at org.mozilla.javascript.MemberBox.invoke(MemberBox.java:205)
	at org.mozilla.javascript.JavaMembers.get(JavaMembers.java:103)
	at org.mozilla.javascript.NativeJavaObject.get(NativeJavaObject.java:96)
	at org.mozilla.javascript.ScriptableObject.getProperty(ScriptableObject.java:2037)
	at org.mozilla.javascript.ScriptRuntime.getObjectProp(ScriptRuntime.java:1741)
	at org.mozilla.javascript.ScriptRuntime.getObjectProp(ScriptRuntime.java:1736)
	at org.mozilla.javascript.gen.DynamicScript_1._c_main_1(DynamicScript:3)
	at org.mozilla.javascript.gen.DynamicScript_1.call(DynamicScript)
	at org.mozilla.javascript.ContextFactory.doTopCall(ContextFactory.java:383)
	at org.mozilla.javascript.ScriptRuntime.doTopCall(ScriptRuntime.java:3940)
	at org.mozilla.javascript.gen.DynamicScript_1.call(DynamicScript)
	at com.openpricer.common.is.DynamicJavaScriptExecution.javaScriptCodeExecutor(DynamicJavaScriptExecution.java:74)

Here are the different versions used in my application:

Rhino 1.7.15
Spring Framework 6.0.23
Java 17
Tomcat 10.1.6

Here's my problem code :


             Context context = Context.enter();
		try {
			Scriptable scope = context.initStandardObjects(null,true);

			// Inject each class into the JavaScript context
			if (classesToImport != null) {
				for (Class<?> clazz : classesToImport) {
					scope.put(clazz.getSimpleName(), scope, Context.javaToJS(clazz, scope));
				}
			}

			// Inject custom JavaScript imports
			String functionName = "main";
			String fullCode = String.format("""
					%s
					function %s(%s) {
					    %s
					}
					""", imports, functionName, parameters, code);
			
			context.evaluateString(scope, fullCode, "DynamicScript", 1, null);
			// Call the dynamically created function
			Function function = (Function) scope.get(functionName, scope);
			return function.call(context, scope, scope, args);
		} finally {
			Context.exit();
		}

When I try to execute a function by calling Function.call I get a NullPointerException in the MemberBox class on args which is null. The script tries to interpret a simple getter on my object (Script parameter).

I think this is due to the static initialization of emptyArgs in Context class :
public static final Object[] emptyArgs = ScriptRuntime.emptyArgs;

Looking in debug mode, ScriptRuntime.emptyArgs is not yet initialised with the empty array.
I then tried to force the initialization of the ScriptRuntime class first when launching my application but ScriptRuntime also depends on Context (code in ScriptRuntime.class line 102) :

 public static final Class<?> ContextClass = Kit.classOrNull(‘org.mozilla.javascript.Context’),
            ContextFactoryClass = Kit.classOrNull(‘org.mozilla.javascript.ContextFactory’),
            FunctionClass = Kit.classOrNull(‘org.mozilla.javascript.Function’),
            ScriptableObjectClass = Kit.classOrNull(‘org.mozilla.javascript.ScriptableObject’);

Is this a known problem or is something missing from my code ?
How can I ensure that Context.emptyArgs is always initialised with an empty object array ?

Thank you in advance

@rbri
Copy link
Collaborator

rbri commented Jan 7, 2025

Our code has only two places where Context.emptyArgs is used - should we deprecate that field and always use ScriptRuntime.emptyArgs?

@egueidao
Copy link
Author

egueidao commented Jan 7, 2025

It could be a good solution.

rbri added a commit to HtmlUnit/rhino-development that referenced this issue Jan 8, 2025
@rbri
Copy link
Collaborator

rbri commented Jan 8, 2025

PR provided - @gbrail, @andreabergia , @p-bakker what do you think?

@andreabergia
Copy link
Contributor

I am ok with removing the duplication and using only the ScriptRuntime one, but I am puzzled by how it could be null.
ScriptRuntime.emptyArgs is a static final field, it should always be initialized by the JVM's calling <clinit>. Same for Context.emptyArgs. So... what am I missing?

@egueidao
Copy link
Author

egueidao commented Jan 9, 2025

Maybe the circular dependency. ScriptRuntime needs Context to initialize the static field ContextClass. Maybe moving the initialization emptyArgs before ContextClass is enough.

In my call stack, <clinit> initializes ScriptRuntime first, but tries to initialize Context for the ScriptRuntime.ContextClass field before ScriptRuntime.emptyArgs

@andreabergia
Copy link
Contributor

@egueidao you are right, good point. I think the fix from @rbri is fine.

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

No branches or pull requests

3 participants