Repository: wix/Detox
Pull request #4241 (2023-10-23 16:17:44)

fix(android): escape instrumentation args
## Description

- This pull request addresses the issue described here: #4196 

In this pull request, I have added POSIX escaping for arguments passed to `adb shell ...` in one forgotten place.


Patch:
diff --git a/detox/src/devices/common/drivers/android/tools/instrumentationArgs.js b/detox/src/devices/common/drivers/android/tools/instrumentationArgs.js
index c8d8d13e5..669b9f662 100644
--- a/detox/src/devices/common/drivers/android/tools/instrumentationArgs.js
+++ b/detox/src/devices/common/drivers/android/tools/instrumentationArgs.js
@@ -1,6 +1,7 @@
 const _ = require('lodash');
 
 const { encodeBase64 } = require('../../../../../utils/encoding');
+const { autoEscape } = require('../../../../../utils/shellUtils');
 
 const reservedInstrumentationArgs = new Set(['class', 'package', 'func', 'unit', 'size', 'perf', 'debug', 'log', 'emma', 'coverageFile']);
 const isReservedInstrumentationArg = (arg) => reservedInstrumentationArgs.has(arg);
@@ -17,7 +18,8 @@ function prepareInstrumentationArgs(args) {
       valueEncoded = encodeBase64(valueAsString);
     }
 
-    result.push('-e', key, valueEncoded);
+    const valueEscaped = hasLegacyIssues(key) ? valueEncoded : autoEscape.shell(valueEncoded);
+    result.push('-e', key, valueEscaped);
     return result;
   }, []);
 
@@ -27,6 +29,10 @@ function prepareInstrumentationArgs(args) {
   };
 }
 
+function hasLegacyIssues(key) {
+  return key === 'detoxURLBlacklistRegex';
+}
+
 module.exports = {
   prepareInstrumentationArgs,
 };


Test patch:
diff --git a/detox/src/devices/common/drivers/android/tools/instrumentationArgs.test.js b/detox/src/devices/common/drivers/android/tools/instrumentationArgs.test.js
index 01834b8e4..fba8fa6f5 100644
--- a/detox/src/devices/common/drivers/android/tools/instrumentationArgs.test.js
+++ b/detox/src/devices/common/drivers/android/tools/instrumentationArgs.test.js
@@ -1,13 +1,13 @@
+/* eslint-disable no-useless-escape */
 // @ts-nocheck
 describe('Instrumentation arguments', () => {
-  const mockEncodeBase64Fn = (arg) => `base64Mocked(${arg})`;
-
   let uut;
+  let encodeBase64;
+
   beforeEach(() => {
-    let encoding;
     jest.mock('../../../../../utils/encoding');
-    encoding = require('../../../../../utils/encoding');
-    encoding.encodeBase64.mockImplementation(mockEncodeBase64Fn);
+    const encoding = require('../../../../../utils/encoding');
+    encodeBase64 = encoding.encodeBase64;
 
     uut = require('./instrumentationArgs');
   });
@@ -49,11 +49,17 @@ describe('Instrumentation arguments', () => {
 
   // Ref: https://developer.android.com/studio/test/command-line#AMOptionsSyntax
   it('should whitelist reserved instrumentation args with respect to base64 encoding', async () => {
+    const blackListRegexps = [
+      /http:\/\/meaningless\.first\.url/,
+      /^http:\/\/localhost(?::\d{4,5})?\/[a-z]+(?:\/\d{4})?$/
+    ];
+
     const args = {
       // Free arg
       'user-arg': 'merry christ-nukah',
-
       // Reserved instrumentation args
+      'detoxURLOverride': 'https://example.com/?q=search term&ref=source',
+      'detoxURLBlacklistRegex': `(${blackListRegexps.map(r => r.source)})`,
       'class': 'class-value',
       'package': 'package-value',
       'func': 'func-value',
@@ -69,6 +75,8 @@ describe('Instrumentation arguments', () => {
     const result = uut.prepareInstrumentationArgs(args);
     expect(result.args).toEqual([
       ...expectedArgEncoded('user-arg', 'merry christ-nukah'),
+      ...expectedArgUnencoded('detoxURLOverride', "'https://example.com/?q=search term&ref=source'"),
+      ...expectedArgUnencoded('detoxURLBlacklistRegex', '(http:\\/\\/meaningless\\.first\\.url,^http:\\/\\/localhost(?::\\d{4,5})?\\/[a-z]+(?:\\/\\d{4})?$)'),
       ...expectedArgUnencoded('class', 'class-value'),
       ...expectedArgUnencoded('package', 'package-value'),
       ...expectedArgUnencoded('func', 'func-value'),
@@ -104,6 +112,6 @@ describe('Instrumentation arguments', () => {
     expect(result.args).toEqual(expectedArgs);
   });
 
-  const expectedArgEncoded = (key, value) => (['-e', key, mockEncodeBase64Fn(value)]);
+  const expectedArgEncoded = (key, value) => (['-e', key, encodeBase64(value)]);
   const expectedArgUnencoded = (key, value) => (['-e', key, value]);
 });
diff --git a/detox/test/e2e/15.urls.test.js b/detox/test/e2e/15.urls.test.js
index 2160e350b..70de0cfdf 100644
--- a/detox/test/e2e/15.urls.test.js
+++ b/detox/test/e2e/15.urls.test.js
@@ -8,7 +8,7 @@ describe('Open URLs', () => {
   });
 
   const withDefaultArgs = () => ({
-    url: 'detoxtesturlscheme://such-string',
+    url: 'detoxtesturlscheme://such-string?arg1=first&arg2=second',
     launchArgs: undefined,
   });
 
@@ -17,32 +17,23 @@ describe('Open URLs', () => {
     launchArgs: { detoxAndroidSingleInstanceActivity: true },
   });
 
-  [
-    {
-      platform: '',
-      ...withDefaultArgs(),
-    },
-    {
-      platform: 'android',
-      ...withSingleInstanceActivityArgs(),
-    }
-  ].forEach((testSpec) => {
-    const {platform, url, launchArgs} = testSpec;
-    const _platform = platform ? `:${platform}: ` : '';
-
-    it(`${_platform}device.launchApp() with a URL and a fresh app should launch app and trigger handling open url handling in app`, async () => {
+  describe.each([
+    ['(default)', withDefaultArgs()],
+    [':android: (single activity)', withSingleInstanceActivityArgs()],
+  ])('%s', (_platform, {url, launchArgs}) => {
+    it(`device.launchApp() with a URL and a fresh app should launch app and trigger handling open url handling in app`, async () => {
       await device.launchApp({newInstance: true, url, launchArgs});
       await expect(element(by.text(url))).toBeVisible();
     });
 
-    it(`${_platform}device.openURL() should trigger open url handling in app when app is in foreground`, async () => {
+    it(`device.openURL() should trigger open url handling in app when app is in foreground`, async () => {
       await device.launchApp({newInstance: true, launchArgs});
       await expect(element(by.text(url))).not.toBeVisible();
       await device.openURL({url});
       await expect(element(by.text(url))).toBeVisible();
     });
 
-    it(`${_platform}device.launchApp() with a URL should trigger url handling when app is in background`, async () => {
+    it(`device.launchApp() with a URL should trigger url handling when app is in background`, async () => {
       await device.launchApp({newInstance: true, launchArgs});
       await expect(element(by.text(url))).not.toBeVisible();
       await device.sendToHome();