Menu ▾ ▴

#1763 [corejs] Infinite recursion in NativeError.getStackDelegated() if object is frozen

Latest SVN
accepted
Ahmed Ashour
None
1
2016-03-31
2016-03-31
Atsushi Nakagawa
No

Bug in brief

If NativeError.getStackDelegated() is called on a frozen JS target object, the nested call to NativeError.setStackDelegated() goes into an infinite recursion and an eventual stack-overflow.

Here's a minimal test case[1]:

<html>
<head>
<script>
function test() {
    o = {}
    Error.captureStackTrace(o)
    Object.freeze(o)

    alert(o.stack)
}
</script>
</head>
<body onload="test()">
</body>
</html>

The above code results in com.gargoylesoftware.htmlunit.ScriptException: Exception invoking getStackDelegated Caused by: java.lang.StackOverflowError, if opened in HtmlUnit versions 2.17 to 2.20 (current latest).

[1] The actual bug was found on a site using the "Dojo Toolkit" (https://dojotoolkit.org/), and is caused by a get on the 'stack' field of a promise/Promise.js instance that was built through Deferred.js.

Some analysis of the problem

The problem appears to be the inability replace the stack field in NativeError.setStackDelegated() (github.com):

public void setStackDelegated(Scriptable target, Object value) {
    target.delete("stack");
    stackProvider = null;
    target.put("stack", target, value);
}

The initial target.delete("stack") appears to be settings things up for this, but the ScriptableObject.PERMANENT attribute on the stack field is preventing deletion by ScriptableObject.removeSlot(). The since slot is not removed, the subsequent target.put() ends calling NativeError.setStackDelegated() again.

Possible temporary workaround

I've used this code to avoid the problem by disabling this functionality.

wc.setJavaScriptEngine(new JavaScriptEngine(wc) {
    private final HtmlUnitContextFactory contextFactory = new HtmlUnitContextFactory(wc) {

        @Override
        protected boolean hasFeature(Context cx, int featureIndex) {
            if (featureIndex == Context.FEATURE_HTMLUNIT_ERROR_STACK) {
                return false;
            }
            return super.hasFeature(cx, featureIndex);
        }
    };

    @Override
    public HtmlUnitContextFactory getContextFactory() {
        return contextFactory;
    }
});

Discussion

  • Atsushi Nakagawa

    Atsushi Nakagawa - 2016-03-31

    Possible fix

    Since the problem seems to be inherent in attempting to switch out property slots, using a tailored variable may be less intrusive.

     final class NativeError extends IdScriptableObject {

     ...

     private RhinoException stackProvider;
+

+    private static final Object STACK_UNINITIALIZED = new Object();
+
+    private transient Object stackValue = STACK_UNINITIALIZED;

     ...

     public Object getStackDelegated(Scriptable target) {

+        if (stackValue != STACK_UNINITIALIZED) {
+            return stackValue;
+        }
         if (stackProvider == null) {
             return NOT_FOUND;
         }

         ...
         RhinoException.useMozillaStackStyle(false);

-        // We store the stack as local property both to cache it
-        // and to make the property writable
+        // We cache this value to avoid regenerating it on every lookup
         setStackDelegated(target, value);
         return value;
     }

     public void setStackDelegated(Scriptable target, Object value) {

-        target.delete("stack");
-        stackProvider = null;
-        target.put("stack", target, value);
+        stackValue = value;
     }

     ...
}
     

    Last edit: Atsushi Nakagawa 2016-03-31

  • Ahmed Ashour

    Ahmed Ashour - 2016-03-31
    • status: open --> accepted
    • assigned_to: Ahmed Ashour
     

Log in to post a comment.

mdb logo