Object.defineProperty nodejs bug as test262 test

[dckc]

I gather SES on node is somewhat blocked by...

• VM context object silently fails on Object.defineProperty #5679

I'm trying to turn this nodejs bug into a test262 test. I gather this might have some impact on its priority.

At first I thought it was out of scope of ECMA 262 because it depends on node's vm API. But then I noticed an API in test262 that reifies the abstract CreateRealm function as $.createRealm.

So I'm trying to supply $.createRealm to the test instead of vm.createContext, but it's not working: when I run node vmbug.js, I get 30 etc. as Kris reports in #5679, but when I run node vmbug2.js I get 20 with a different exception:

R1:  ReferenceError: x is not defined
    at realm1:59:11
    at realm1:65:3
    at ContextifyScript.Script.runInContext (vm.js:37:29)
...
    at tryModuleLoad (module.js:449:12)
R1:  20
R1:  { value: 20,
  writable: false,
  enumerable: false,
  configurable: false }

Any clues what I'm doing wrong?

• vmbug.js
• vmbug2.js

cc @erights @kriskowal @metaweta @jfparadis

[erights]

At first I thought it was out of scope of ECMA 262 because it depends on node's vm API.

That does not make it out of scope. There's a trick to write a test262 test that would seem to depend on a platform API. Probe to detect if the API is present. If not, the test succeeds. There is one precedent -- I wrote a test demonstrating a way that some browser failed to conform. To write the test I had to probe for the existence of "document". If there was none, that test succeeded.

I think the test itself is now long retired. But the precedent it set remains!

But yes, it is better to avoid the need to do that when we can.

[erights]

Attn @bterlson @bmeck

[bmeck]

@erights yes, this relates to what we discovered in September. To avoid breaking jsdom the largest consumer of the vm module, we must wait until v8 5.7 lands in Node to fix the bug. As such, I highly recommend avoiding the use of the vm module for other reasons; the Context it creates is not a pure Realm and does not mimic the behaviors of a global proxy Object correctly.

[erights]

What's the ETA for v8 5.7 landing in Node?

[kriskowal]

@bmeck Do you recommend an interim alternative? I presume none exists.

[bmeck]

@erights Node@9 stable release (April) so Node@10 LTS release (October)

@kriskowal none exist but it is a minimal C++ addon to write if you are willing to manage a membrane yourself.

[erights]

We definitely cannot wait that long. Neither would it be sensible for us to write C++, and thereby run on something other than stock Node. Instead, we need to figure out how SES, Frozen Realms, and Dan's test262 test can successfully use what Node has in the meantime.

For SES and Frozen Realms, I think we can just copy everything we would need from the real global onto the pseudo-global. We should then shadow the real global completely, even for those global variable names whitelisted in whitelist.js

[bmeck]

@erights to be clear the "global" in the vm module on Node does not behave like a normal object and in some cases breaks the invariants set forth by Ecma262.

[erights]

Yes. Since the result is that Node is presently non-spec-compliant in this regard, we should be able to write a test262 test that visibly fails. Dan is asking for advice on how to do so.

[bmeck]

the script in vmbug2.js doesn't appear to always use the fake global:

// test case starts on line 47
var script = '(' + function () {
    'use strict';

    $.global.x = 10;
    Object.defineProperty($.global, 'x', {
        writable: false,
        enumerable: false,
        configurable: false,
        value: 20
    });
    try {

       // shouldn't this be $.global.x = 30?

        x = 30;
    } catch (e) {
        print(e);
    }

    // shouldn't this be print($.global.x)

    print(x); // should be 20
    print(Object.getOwnPropertyDescriptor($.global, 'x'));
} + ')()';

[dckc]

In this respect, vmbug2 just mimics vmbug, which does exhibit the buggy behavior.

I presume a purpose of the test is to demonstrate an interaction with the usual way of assigning to globals, i.e. without any qualification.

@kriskowal?

[dckc]

I see a comment from kriskowal on Dec 12, 2017 about the node situation:

Confirmed that this is fully fixed in 9.2.1.

so I'm taking this off my todo list, or at least demoting it: test excess authority via prototype chain (WIP)