From ce25a67070ddf3ddeb2ffcdd34ad19b41c6783c8 Mon Sep 17 00:00:00 2001 From: Adil Rakhaliyev <67043367+Bayheck@users.noreply.github.com> Date: Mon, 18 Dec 2023 17:30:17 +0600 Subject: [PATCH] Fixed method call not considering callee optional (#2985) * fix: optional method call fixed * fix: method call with optional owner * refactor: code refactored * add: new test cases added * refactor: changed the name of the call method * fix: tests updated * fix: method call and tests refactored * fix: test fixed * add: new testcase added --- .../sandbox/code-instrumentation/methods.ts | 4 +- src/processing/script/node-builder.ts | 7 +++- .../script/transformers/method-call.ts | 9 +++-- .../process-script-test.js | 38 +++++++++++++++++++ 4 files changed, 50 insertions(+), 8 deletions(-) diff --git a/src/client/sandbox/code-instrumentation/methods.ts b/src/client/sandbox/code-instrumentation/methods.ts index 632577f30..190faa87e 100644 --- a/src/client/sandbox/code-instrumentation/methods.ts +++ b/src/client/sandbox/code-instrumentation/methods.ts @@ -73,9 +73,9 @@ export default class MethodCallInstrumentation extends SandboxBase { // NOTE: In Google Chrome, iframes whose src contains html code raise the 'load' event twice. // So, we need to define code instrumentation functions as 'configurable' so that they can be redefined. nativeMethods.objectDefineProperty(window, INSTRUCTION.callMethod, { - value: (owner: any, methName: any, args: any[], optional = false) => { + value: (owner: any, methName: any, args: any[], optional = false, calleeOptional = false) => { if (isNullOrUndefined(owner)) { - if (optional) + if (optional || calleeOptional) return void 0; MethodCallInstrumentation._error(`Cannot call method '${methName}' of ${inaccessibleTypeToStr(owner)}`); diff --git a/src/processing/script/node-builder.ts b/src/processing/script/node-builder.ts index d8e6595ba..a8f31aa8f 100644 --- a/src/processing/script/node-builder.ts +++ b/src/processing/script/node-builder.ts @@ -163,14 +163,17 @@ export function createPropertySetWrapper (propertyName: string, obj: Expression, return createSimpleCallExpression(setPropertyIdentifier, [obj, createSimpleLiteral(propertyName), value]); } -export function createMethodCallWrapper (owner: Expression, method: Literal, methodArgs: (Expression | SpreadElement)[], optional = false): CallExpression { +export function createMethodCallWrapper (owner: Expression, method: Literal, methodArgs: (Expression | SpreadElement)[], optional = false, calleeOptional = false): CallExpression { const callMethodIdentifier = createIdentifier(INSTRUCTION.callMethod); const methodArgsArray = createArrayExpression(methodArgs); const args = [owner, method, methodArgsArray]; - if (optional) + if (optional || calleeOptional) args.push(createSimpleLiteral(optional)); + if (calleeOptional) + args.push(createSimpleLiteral(calleeOptional)); + return createSimpleCallExpression(callMethodIdentifier, args); } diff --git a/src/processing/script/transformers/method-call.ts b/src/processing/script/transformers/method-call.ts index 66bb719ba..a0a8c0a6d 100644 --- a/src/processing/script/transformers/method-call.ts +++ b/src/processing/script/transformers/method-call.ts @@ -46,13 +46,14 @@ const transformer: Transformer = { }, run: node => { - const callee = node.callee as MemberExpression; - const method = callee.computed + const callee = node.callee as MemberExpression; + const method = callee.computed ? callee.property as Literal : createSimpleLiteral((callee.property as Identifier).name); // eslint-disable-line no-extra-parens - const optional = (node as SimpleCallExpression).optional; + const optional = (node as SimpleCallExpression).optional; + const calleeOptional = callee.optional; - return createMethodCallWrapper(callee.object as Expression, method, node.arguments, optional); + return createMethodCallWrapper(callee.object as Expression, method, node.arguments, optional, calleeOptional); }, }; diff --git a/test/client/fixtures/sandbox/code-instrumentation/process-script-test.js b/test/client/fixtures/sandbox/code-instrumentation/process-script-test.js index 6a7cd8b5f..028027d68 100644 --- a/test/client/fixtures/sandbox/code-instrumentation/process-script-test.js +++ b/test/client/fixtures/sandbox/code-instrumentation/process-script-test.js @@ -280,6 +280,7 @@ test('optional chaining', function () { ]; var additionalCases = []; + var errorCases = []; // NOTE: Safari until iOS 13.4 don't have full support optional chaining if (!browserUtils.isIOS || browserUtils.compareVersions([browserUtils.webkitVersion, '608.2.11']) === 1) { @@ -296,6 +297,25 @@ test('optional chaining', function () { src: 'var obj = { href: "123" }; var name = "name"; window.optionChainingResult = obj?.link?.[name]?.();', expected: void 0, }, + { + src: 'var obj = { link: null }; var name = "name"; window.optionChainingResult = obj.link?.[name]();', + expected: void 0, + }, + { + src: 'var obj = {}; var name = "name"; window.optionChainingResult = obj[name]?.();', + expected: void 0, + }, + ]; + + errorCases = [ + { + src: 'var obj = { name: "123" }; var name = "name"; window.optionChainingResult = obj?.[name]();', + expected: '\'name\' is not a function', + }, + { + src: 'var obj = { link: {name: "123"} }; var name = "name"; window.optionChainingResult = obj.link?.[name]();', + expected: '\'name\' is not a function', + }, ]; for (let i = 0; i < additionalCases.length; i++) @@ -313,4 +333,22 @@ test('optional chaining', function () { delete window.optionChainingResult; delete window.obj; } + + for (var i = 0; i < errorCases.length; i++) { + var testCase = errorCases[i]; + var script = processScript(testCase.src); + var errorMessage = ''; + + try { + eval(script); + } + catch (e) { + errorMessage = e.message; + } + + strictEqual(errorMessage, testCase.expected); + + delete window.optionChainingResult; + delete window.obj; + } });