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

freeze doesn't work on Array methods (shift, pop, etc) #511

Open
poef opened this issue Mar 11, 2023 · 4 comments
Open

freeze doesn't work on Array methods (shift, pop, etc) #511

poef opened this issue Mar 11, 2023 · 4 comments

Comments

@poef
Copy link

poef commented Mar 11, 2023

running node v18.15.0
if i have this code:

import {VM} from 'vm2'
const vm = new VM({
	allowAsync: false,
	wasm: false
})
let result = [1,2,3]
vm.freeze(result, 'data')

I expect that this will fail:

vm.run(`
data.pop()
`)

However, the array is changed and if I run it multiple times, I get 3, 2 then 1 as a result.

@XmiliaH
Copy link
Collaborator

XmiliaH commented Mar 11, 2023

Freeze does only stop the sandbox from manipulating the object. The pop method is from the host and is allowed to manipulate the array.

@poef
Copy link
Author

poef commented Mar 17, 2023

Hi, I understand your explanation about why the vm.freeze() call doesn't work on Array.pop() here. However, given the documentation of freeze(), I would expect the data to be immutable. Since pop() alters the array, it isn't immutable.
So the freeze() function doesn't turn arrays immutable, which I believe it should.

@XmiliaH
Copy link
Collaborator

XmiliaH commented Mar 17, 2023

If you want to make the array immutable just use Object.freeze.

@poef
Copy link
Author

poef commented Jul 21, 2023

I would politely suggest to rename 'freeze' to something else, or at least update the documentation so that it tells people that it doesn't actually freeze the data entirely. As it is now, the behaviour is rather surprising. And yes, I did use Object.freeze to fix it.

BTW, thank you for your work on VM2, I really like how simple it is to use this library.

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

No branches or pull requests

2 participants