Bug 135322 – REGRESSION: JSObjectSetPrototype() does not work on result of JSGetGlobalObject()

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Jay Freeman (saurik)]

Created attachment 235573 [details]
the C++ code I provided in my comment, as an attachment

Hello. I ran into this issue at WWDC while testing my software against Xcode 6. I provided a demonstration of this functionality difference in JavaScript to a couple engineers at Apple, and was encouraged to file two bug reports (#133531 and #133532). I also ran the behavior of the snippet of JavaScript I provided, which does not match the seemingly-correct behavior of both Firefox and Chrome, past someone I know on the ECMAScript standardization committee, and their response was "lol" ;P. However, I was told that at the JavaScript level, this behavior (which Safari used to simulate :/) was "intended", and the bug was marked as "won't fix" :(.

I was, however and despite, also encouraged in the response to provide a test case demonstrating the change in behavior at the level of the C API as part of a (new?) bug report. I honestly have found it difficult to work up the energy to put this together, as I thought I had already described the behavior change decently, and it sounds like even non-compliance at the level of JavaScript visible to a browser is "intentional" :(. However, due to increasing numbers of bug reports against my software that are making me start to look into annoying or even "drastic" workarounds (including grubbing around in my JSObject memory), I am giving this a try.

Attached is some C++ code that uses JavaScriptCore. What it does is define a new JSClass, create a JSGlobalContext using that class for the global object, and then use JSContextGetGlobalObject to get a reference to this new object. It then attempts to set the prototype of this object, which is futile, as JSContextGetGlobalObject no longer actually returns the global object (despite the name of the function :/): instead, it returns a useless-to-me JSProxy that sort of almost sometimes wraps the behavior of the actual global object, and which for reasons I do not understand has its own separate prototype chain (I could see JSProxy proxying __proxy__).

The new prototype that I add has a single property, "test", which I then try to access as a variable reference in the global scope (to simplify this code, the name of the property, the value of the property, and the code that I evaluate, are all the single string "test"). On older, working, versions of JavaScriptCore, JSContextGetGlobalObject returns the global object, the prototype is correctly set, and this program returns "test". On newer, broken, versions of JavaScriptCore, JSContextGetGlobalObject returns a JSProxy, the prototype that is set does not get forwarded, and you get the JavaScript exception "ReferenceError: Can't find variable: test"

Sadly, as one of the main use cases for my software is as a library "add-on", where the JSGlobalContext has already been created, I am not in a position to fix this by adding "proxy-like" forwarding logic to my own global object (which would have slightly incorrect semantics anyway unless I go to irritating lengths to attempt to simulate the behavior of properties being overridden from JavaScript prototypes), or to attempt to get a reference to the "real" prototype by using a constructor function (as I can't call JSObjectMakeConstructor without a reference to the JSClass, and I don't know of any way to get a JSClassRef given only a JSObjectRef). 

Regardless, thank you very much for looking at this bug report. Despite feeling a little demotivated with relation to this one issue, I have generally been a very happy user of JavaScriptCore for the last five years. I had a great time talking to Oliver about this issue at WWDC, and showing him the awkward behavior it was causing at the JavaScript level (where "this.test" and "test" give different results, despite "this" being a reference to the same global context object that "test" is supposed to be resolved from). I also found Edward (whom I met in person, and who wanted to be CC'd to the bug report) very friendly to work with. Thank you both again!

#include <JavaScriptCore/JSBase.h>
#include <JavaScriptCore/JSContextRef.h>
#include <JavaScriptCore/JSStringRef.h>
#include <JavaScriptCore/JSObjectRef.h>
#include <JavaScriptCore/JSValueRef.h>

#include <cassert>
#include <cstdio>

int main() {
    JSValueRef error(NULL);

    JSClassDefinition definition(kJSClassDefinitionEmpty);
    definition.className = "Global";
    JSClassRef global(JSClassCreate(&definition));
    JSGlobalContextRef context(JSGlobalContextCreate(global));
    JSObjectRef object(JSContextGetGlobalObject(context));

    JSObjectRef above(JSObjectMake(context, NULL, NULL));
    JSStringRef test(JSStringCreateWithUTF8CString("test"));
    JSValueRef value(JSValueMakeString(context, test));
    JSObjectSetProperty(context, above, test, value, kJSPropertyAttributeDontEnum, &error);
    assert(error == NULL);

    JSObjectSetPrototype(context, object, above);
    JSValueRef result(JSEvaluateScript(context, test, NULL, NULL, 0, &error));
    if (error != NULL) {
        assert(result == NULL);
        result = error;
        error = NULL;
    }

    JSStringRef string(JSValueToStringCopy(context, result, &error));
    assert(error == NULL);
    char buffer[JSStringGetMaximumUTF8CStringSize(string)];
    JSStringGetUTF8CString(string, buffer, sizeof(buffer));
    printf("%s\n", buffer);
    return 0;
}

[Mark Hahnenberg]

Thank you for the very detailed bug report! 

The prototype chain of the JSProxy object should match that of the JSGlobalObject. Until recently, there was a bug in JSGlobalContextCreate that caused the two to have different prototype chains (see bug 135250).

I think you've identified a separate but related issue with JSObjectSetPrototype which doesn't correctly account for JSProxies. It should be an easy fix. We should also audit the rest of the C API to check that we correctly handle JSProxies in all other situations.

[Mark Hahnenberg]

Created attachment 235608 [details]
Patch

[Mark Hahnenberg]

<rdar://problem/17784938>

[Mark Hahnenberg]

Committed r171691: <http://trac.webkit.org/changeset/171691>