From 00f999006b37f2d2279cc2c637f72c819e2e51c9 Mon Sep 17 00:00:00 2001 From: Jack Works Date: Tue, 2 Jul 2019 12:33:09 +0800 Subject: [PATCH 1/4] Fix `this` binding in Function --- src/evaluators.js | 19 ++++++++++++++++--- src/realm.js | 2 +- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/evaluators.js b/src/evaluators.js index d7fd73b..2cf68cf 100644 --- a/src/evaluators.js +++ b/src/evaluators.js @@ -190,7 +190,7 @@ export function createSafeEvaluatorWhichTakesEndowments(safeEvaluatorFactory) { * A safe version of the native Function which relies on * the safety of evalEvaluator for confinement. */ -export function createFunctionEvaluator(unsafeRec, safeEval) { +export function createFunctionEvaluator(unsafeRec, safeEval, realmGlobal) { const { unsafeFunction, unsafeGlobal } = unsafeRec; const safeFunction = function Function(...params) { @@ -247,8 +247,21 @@ export function createFunctionEvaluator(unsafeRec, safeEval) { } const src = `(function(${functionParams}){\n${functionBody}\n})`; - - return safeEval(src); + const isStrict = !!/^\s*['|"]use strict['|"]/.exec(functionBody); + const fn = safeEval(src); + if (isStrict) { + return fn; + } + // we fix the `this` binding in Function(). + const bindThis = `(function (globalThis, f) { + function f2() { + return Reflect.apply(f, this || globalThis, arguments); + } + f2.toString = () => f.toString(); + return f2; +})`; + const fnWithThis = safeEval(bindThis)(realmGlobal, fn); + return fnWithThis; }; // Ensure that Function from any compartment in a root realm can be used diff --git a/src/realm.js b/src/realm.js index 6f03266..182df44 100644 --- a/src/realm.js +++ b/src/realm.js @@ -65,7 +65,7 @@ function createRealmRec(unsafeRec, transforms, sloppyGlobals) { const safeEvalWhichTakesEndowments = createSafeEvaluatorWhichTakesEndowments( safeEvaluatorFactory ); - const safeFunction = createFunctionEvaluator(unsafeRec, safeEval); + const safeFunction = createFunctionEvaluator(unsafeRec, safeEval, safeGlobal); setDefaultBindings(safeGlobal, safeEval, safeFunction); From dc3e2cb5d1c1bbf6ababab12a15609887cf40359 Mon Sep 17 00:00:00 2001 From: Jack Works Date: Tue, 2 Jul 2019 13:01:20 +0800 Subject: [PATCH 2/4] Add test for new behavior of inner Function constructor --- test/realm/test-confinement.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/realm/test-confinement.js b/test/realm/test-confinement.js index 9ddf5a2..28328bf 100644 --- a/test/realm/test-confinement.js +++ b/test/realm/test-confinement.js @@ -2,12 +2,16 @@ import test from 'tape'; import Realm from '../../src/realm'; test('confinement evaluation strict mode', t => { - t.plan(2); + t.plan(3); const r = Realm.makeRootRealm(); t.equal(r.evaluate('(function() { return this })()'), undefined); - t.equal(r.evaluate('(new Function("return this"))()'), undefined); + t.equal(r.evaluate('(new Function("return this"))()'), r.global); + t.equal( + r.evaluate('(new Function("\\"use strict\\";return this"))()'), + undefined + ); }); test('constructor this binding', t => { From 6c3ca12f749201fd26cf9f22d3d5e3b452bde577 Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Sun, 7 Jul 2019 17:51:11 -0700 Subject: [PATCH 3/4] add testcase for other Function this bindings --- test/realm/test-confinement.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/test/realm/test-confinement.js b/test/realm/test-confinement.js index 28328bf..db0c038 100644 --- a/test/realm/test-confinement.js +++ b/test/realm/test-confinement.js @@ -1,15 +1,21 @@ import test from 'tape'; import Realm from '../../src/realm'; +test('non-strict mode this binding in Function constructor', t => { + t.plan(1); + + const r = Realm.makeRootRealm(); + t.equal(r.evaluate('(new Function("return this"))()'), r.global); +}); + test('confinement evaluation strict mode', t => { - t.plan(3); + t.plan(2); const r = Realm.makeRootRealm(); t.equal(r.evaluate('(function() { return this })()'), undefined); - t.equal(r.evaluate('(new Function("return this"))()'), r.global); t.equal( - r.evaluate('(new Function("\\"use strict\\";return this"))()'), + r.evaluate(`(new Function('"use strict"; return this'))()`), undefined ); }); @@ -18,9 +24,9 @@ test('constructor this binding', t => { const r = Realm.makeRootRealm(); const F = r.evaluate('(new Function("return this"))'); - t.equal(F(), undefined); + t.equal(F(), r.global); t.equal(F.call(8), 8); - t.equal(F.call(undefined), undefined); + t.equal(F.call(undefined), r.global); t.equal(Reflect.apply(F, 8, []), 8); const x = { F }; From d66910ebde571a3670b62745a6424f1207aac2fa Mon Sep 17 00:00:00 2001 From: Jack Works Date: Sat, 28 Sep 2019 11:33:01 +0800 Subject: [PATCH 4/4] fix: use stricter compare to compare undefined --- src/evaluators.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/evaluators.js b/src/evaluators.js index 2cf68cf..b24f641 100644 --- a/src/evaluators.js +++ b/src/evaluators.js @@ -253,9 +253,12 @@ export function createFunctionEvaluator(unsafeRec, safeEval, realmGlobal) { return fn; } // we fix the `this` binding in Function(). + // note: In non-strict mode, if `this` is not object + // it will be wrapped by Object(). Like 1 becomes Object(1) which is Number(1) + // Should we simulate this? It seems like no one will rely on this. const bindThis = `(function (globalThis, f) { function f2() { - return Reflect.apply(f, this || globalThis, arguments); + return Reflect.apply(f, this === undefined ? globalThis : this, arguments); } f2.toString = () => f.toString(); return f2;