GeezerVison is showing github.com — vm: properly handle defining symbol props · nodejs/node@ed45142
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: src/node_contextify.cc +1Lines changed: 1 addition & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -524,6 +524,7 @@ void ContextifyContext::PropertySetterCallback( | 524 | 524 | !is_function) | 525 | 525 | return; | 526 | 526 |
| | 527 | + if (!is_declared && property->IsSymbol()) return; | 527 | 528 | if (ctx->sandbox()->Set(context, property, value).IsNothing()) return; | 528 | 529 |
| 529 | 530 | Local<Value> desc; | |
Collapse file Copy file name to clipboard +105Lines changed: 105 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +'use strict'; | | 2 | +require('../common'); | | 3 | +const assert = require('assert'); | | 4 | +const vm = require('vm'); | | 5 | + | | 6 | +// These assertions check that we can set new keys to the global context, | | 7 | +// get them back and also list them via getOwnProperty* or in. | | 8 | +// | | 9 | +// Related to: | | 10 | +// - https://github.com/nodejs/node/issues/45983 | | 11 | + | | 12 | +const global = vm.runInContext('this', vm.createContext()); | | 13 | + | | 14 | +function runAssertions(data, property, viaDefine, value1, value2, value3) { | | 15 | + // Define the property for the first time | | 16 | + setPropertyAndAssert(data, property, viaDefine, value1); | | 17 | + // Update the property | | 18 | + setPropertyAndAssert(data, property, viaDefine, value2); | | 19 | + // Delete the property | | 20 | + deletePropertyAndAssert(data, property); | | 21 | + // Re-define the property | | 22 | + setPropertyAndAssert(data, property, viaDefine, value3); | | 23 | + // Delete the property again | | 24 | + deletePropertyAndAssert(data, property); | | 25 | +} | | 26 | + | | 27 | +const fun1 = () => 1; | | 28 | +const fun2 = () => 2; | | 29 | +const fun3 = () => 3; | | 30 | + | | 31 | +function runAssertionsOnSandbox(builder) { | | 32 | + const sandboxContext = vm.createContext({ runAssertions, fun1, fun2, fun3 }); | | 33 | + vm.runInContext(builder('this'), sandboxContext); | | 34 | + vm.runInContext(builder('{}'), sandboxContext); | | 35 | +} | | 36 | + | | 37 | +// Assertions on: define property | | 38 | +runAssertions(global, 'toto', true, 1, 2, 3); | | 39 | +runAssertions(global, Symbol.for('toto'), true, 1, 2, 3); | | 40 | +runAssertions(global, 'tutu', true, fun1, fun2, fun3); | | 41 | +runAssertions(global, Symbol.for('tutu'), true, fun1, fun2, fun3); | | 42 | +runAssertions(global, 'tyty', true, fun1, 2, 3); | | 43 | +runAssertions(global, Symbol.for('tyty'), true, fun1, 2, 3); | | 44 | + | | 45 | +// Assertions on: direct assignment | | 46 | +runAssertions(global, 'titi', false, 1, 2, 3); | | 47 | +runAssertions(global, Symbol.for('titi'), false, 1, 2, 3); | | 48 | +runAssertions(global, 'tata', false, fun1, fun2, fun3); | | 49 | +runAssertions(global, Symbol.for('tata'), false, fun1, fun2, fun3); | | 50 | +runAssertions(global, 'tztz', false, fun1, 2, 3); | | 51 | +runAssertions(global, Symbol.for('tztz'), false, fun1, 2, 3); | | 52 | + | | 53 | +// Assertions on: define property from sandbox | | 54 | +runAssertionsOnSandbox( | | 55 | + (variable) => ` | | 56 | + runAssertions(${variable}, 'toto', true, 1, 2, 3); | | 57 | + runAssertions(${variable}, Symbol.for('toto'), true, 1, 2, 3); | | 58 | + runAssertions(${variable}, 'tutu', true, fun1, fun2, fun3); | | 59 | + runAssertions(${variable}, Symbol.for('tutu'), true, fun1, fun2, fun3); | | 60 | + runAssertions(${variable}, 'tyty', true, fun1, 2, 3); | | 61 | + runAssertions(${variable}, Symbol.for('tyty'), true, fun1, 2, 3);` | | 62 | +); | | 63 | + | | 64 | +// Assertions on: direct assignment from sandbox | | 65 | +runAssertionsOnSandbox( | | 66 | + (variable) => ` | | 67 | + runAssertions(${variable}, 'titi', false, 1, 2, 3); | | 68 | + runAssertions(${variable}, Symbol.for('titi'), false, 1, 2, 3); | | 69 | + runAssertions(${variable}, 'tata', false, fun1, fun2, fun3); | | 70 | + runAssertions(${variable}, Symbol.for('tata'), false, fun1, fun2, fun3); | | 71 | + runAssertions(${variable}, 'tztz', false, fun1, 2, 3); | | 72 | + runAssertions(${variable}, Symbol.for('tztz'), false, fun1, 2, 3);` | | 73 | +); | | 74 | + | | 75 | +// Helpers | | 76 | + | | 77 | +// Set the property on data and assert it worked | | 78 | +function setPropertyAndAssert(data, property, viaDefine, value) { | | 79 | + if (viaDefine) { | | 80 | + Object.defineProperty(data, property, { | | 81 | + enumerable: true, | | 82 | + writable: true, | | 83 | + value: value, | | 84 | + configurable: true, | | 85 | + }); | | 86 | + } else { | | 87 | + data[property] = value; | | 88 | + } | | 89 | + assert.strictEqual(data[property], value); | | 90 | + assert.ok(property in data); | | 91 | + if (typeof property === 'string') { | | 92 | + assert.ok(Object.getOwnPropertyNames(data).includes(property)); | | 93 | + } else { | | 94 | + assert.ok(Object.getOwnPropertySymbols(data).includes(property)); | | 95 | + } | | 96 | +} | | 97 | + | | 98 | +// Delete the property from data and assert it worked | | 99 | +function deletePropertyAndAssert(data, property) { | | 100 | + delete data[property]; | | 101 | + assert.strictEqual(data[property], undefined); | | 102 | + assert.ok(!(property in data)); | | 103 | + assert.ok(!Object.getOwnPropertyNames(data).includes(property)); | | 104 | + assert.ok(!Object.getOwnPropertySymbols(data).includes(property)); | | 105 | +} |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-vm-global-symbol.js -26Lines changed: 0 additions & 26 deletions
|