Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

Inconsistent behaviour with vm context of Object.create(null) #508

Open
Tracked by #3087
alexlamsl opened this issue Mar 31, 2018 · 11 comments
Open
Tracked by #3087

Inconsistent behaviour with vm context of Object.create(null) #508

alexlamsl opened this issue Mar 31, 2018 · 11 comments

Comments

@alexlamsl
Copy link

  • Version: v8.10.0
  • Platform: Windows 10 x64
  • Subsystem: vm

Node.js

$ node -v
v8.11.1

$ node
> vm.runInNewContext('0 & this', Object.create(null))
0

node-chakracore

$ node -v
v8.10.0

$ node
> vm.runInNewContext('0 & this', Object.create(null))
evalmachine.<anonymous>:1
0 & this
^

TypeError: Number expected
   at Global code (evalmachine.<anonymous>:1:1)
   at Script.prototype.runInContext (vm.js:59:5)
   at Script.prototype.runInNewContext (vm.js:65:3)
   at runInNewContext (vm.js:135:3)
   at Global code (repl:1:1)
   at Script.prototype.runInThisContext (vm.js:50:5)
   at defaultEval (repl.js:240:13)
   at bound (domain.js:301:5)
   at runBound (domain.js:314:5)
   at onLine (repl.js:468:5)
@MSLaguana
Copy link
Contributor

I don't believe that this is to do with the vm module actually; just running 0 & Object.create(null) in ch (bare chakracore) gives the same error.

@MSLaguana
Copy link
Contributor

Unless you expect that, but you expect that using that as the global should have different behavior?

@alexlamsl
Copy link
Author

0 & Object.create(null) behaves consistently between node and node-chakracore (both at 8.11.1)

$ nvs use node/8
PATH = node\8.11.1\x64

$ node
> 0 & Object.create(null)
TypeError: Cannot convert object to primitive value
$ nvs use chakracore
PATH = chakracore\8.11.1\x64

$ node
> 0 & Object.create(null)
TypeError: Number expected
   at Global code (repl:1:1)

whereas the top example works for node but cause exception to be throw in node-chakracore

@MSLaguana
Copy link
Contributor

Ok, just had a look and a bare 0 & Object.create(null) in node-v8 gives an equivalent error, but in the vm context it does not. I think that this is because in the v8 case the global object keeps its prototype and so there is a valueOf, while in the node-chakracore case we replace the prototype with the sandbox and so it no longer has a valueOf.

@MSLaguana
Copy link
Contributor

I somewhat feel like this might be one of those edge-cases that may not be fully intended in node upstream? I know I asked about what happens in some strange cases when properties conflict between the sandbox and the global, and there wasn't a good answer to what was expected. I would somewhat expect that if you say "Use this sandbox object, and I've removed everything from it" that you wouldn't want it to have things like the prototype properties; just a bare minimal global object.

@alexlamsl
Copy link
Author

I want to be absolutely sure to avoid any Object.prototype properties leaking into the sandbox context, which is why it is coded this way.

Having said that, I do understand your reason for the current behaviour in node-chakracore - but then as a user I just want to be able to switch between the platforms with as few quirks as possible.

@MSLaguana
Copy link
Contributor

We definitely want to have minimal platform differences. The problem here is trying to understand what the underlying expected behavior is, given that this vm context stuff isn't strongly specced out, and "what node-v8 does" isn't necessarily right in all cases either since there are some questions about whether they do the right thing in some circumstances.

@alexlamsl
Copy link
Author

Thanks for the dicussion and clarifications - meanwhile I'll see if I can come up with an alternative test case in uglify-js which avoids this issue.

@alexlamsl
Copy link
Author

Reverted the workaround in uglify-js to confirm the difference in behaviour between node and node-chakracore still persists as of 11.0.0-nightly20180522db93156017

@alexlamsl
Copy link
Author

Still an issue as of v10.1.0

@alexlamsl
Copy link
Author

Reproducible in v10.6.0

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

No branches or pull requests

2 participants