Bug 1338327 Handle .cmd files with spaces in subprocess draft
authorAndrew Swan <aswan@mozilla.com>
Mon, 12 Jun 2017 14:51:32 -0700
changeset 595066 25429dd4073f97438181c1040e3e1b1d3d7762b1
parent 591816 2d20b365eee19434657f6b365d310e8b70904d2b
child 633619 e16fe7ca9a40f2deeb519b316ff1b858bcece3ce
push id64242
push useraswan@mozilla.com
push dateFri, 16 Jun 2017 00:33:20 +0000
bugs1338327
milestone55.0a1
Bug 1338327 Handle .cmd files with spaces in subprocess MozReview-Commit-ID: 9zr97CyY5in
toolkit/components/extensions/test/xpcshell/head_native_messaging.js
toolkit/components/extensions/test/xpcshell/test_ext_native_messaging.js
toolkit/modules/subprocess/subprocess_worker_win.js
--- a/toolkit/components/extensions/test/xpcshell/head_native_messaging.js
+++ b/toolkit/components/extensions/test/xpcshell/head_native_messaging.js
@@ -87,19 +87,21 @@ async function setupHosts(scripts) {
       const REGKEY = String.raw`Software\Mozilla\NativeMessagingHosts`;
 
       let registry = new MockRegistry();
       do_register_cleanup(() => {
         registry.shutdown();
       });
 
       for (let script of scripts) {
+        let {scriptExtension = "bat"} = script;
+
         // It's important that we use a space in this filename. See directory
         // name comment above.
-        let batPath = getPath(`batch ${script.name}.bat`);
+        let batPath = getPath(`batch ${script.name}.${scriptExtension}`);
         let scriptPath = getPath(`${script.name}.py`);
 
         let batBody = `@ECHO OFF\n${pythonPath} -u "${scriptPath}" %*\n`;
         await OS.File.writeAtomic(batPath, batBody);
 
         // Create absolute and relative path versions of the entry.
         for (let [name, path] of [[script.name, batPath],
                                   [`relative.${script.name}`, OS.Path.basename(batPath)]]) {
--- a/toolkit/components/extensions/test/xpcshell/test_ext_native_messaging.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_native_messaging.js
@@ -37,17 +37,17 @@ const INFO_BODY = String.raw`
 const STDERR_LINES = ["hello stderr", "this should be a separate line"];
 let STDERR_MSG = STDERR_LINES.join("\\n");
 
 const STDERR_BODY = String.raw`
   import sys
   sys.stderr.write("${STDERR_MSG}")
 `;
 
-const SCRIPTS = [
+let SCRIPTS = [
   {
     name: "echo",
     description: "a native app that echoes back messages it receives",
     script: ECHO_BODY.replace(/^ {2}/gm, ""),
   },
   {
     name: "info",
     description: "a native app that gives some info about how it was started",
@@ -55,16 +55,25 @@ const SCRIPTS = [
   },
   {
     name: "stderr",
     description: "a native app that writes to stderr and then exits",
     script: STDERR_BODY.replace(/^ {2}/gm, ""),
   },
 ];
 
+if (AppConstants.platform == "win") {
+  SCRIPTS.push({
+    name: "echocmd",
+    description: "echo but using a .cmd file",
+    scriptExtension: "cmd",
+    script: ECHO_BODY.replace(/^ {2}/gm, ""),
+  });
+}
+
 add_task(async function setup() {
   await setupHosts(SCRIPTS);
 });
 
 // Test the basic operation of native messaging with a simple
 // script that echoes back whatever message is sent to it.
 add_task(async function test_happy_path() {
   function background() {
@@ -136,45 +145,57 @@ add_task(async function test_happy_path(
 
   let procCount = await getSubprocessCount();
   equal(procCount, 1, "subprocess is still running");
   let exitPromise = waitForSubprocessExit();
   await extension.unload();
   await exitPromise;
 });
 
+// Just test that the given app (which should be the echo script above)
+// can be started.  Used to test corner cases in how the native application
+// is located/launched.
+async function simpleTest(app) {
+  function background(appname) {
+    let port = browser.runtime.connectNative(appname);
+    let MSG = "test";
+    port.onMessage.addListener(msg => {
+      browser.test.assertEq(MSG, msg, "Got expected message back");
+      browser.test.sendMessage("done");
+    });
+    port.postMessage(MSG);
+  }
+
+  let extension = ExtensionTestUtils.loadExtension({
+    background: `(${background})(${JSON.stringify(app)});`,
+    manifest: {
+      applications: {gecko: {id: ID}},
+      permissions: ["nativeMessaging"],
+    },
+  });
+
+  await extension.startup();
+  await extension.awaitMessage("done");
+
+  let procCount = await getSubprocessCount();
+  equal(procCount, 1, "subprocess is still running");
+  let exitPromise = waitForSubprocessExit();
+  await extension.unload();
+  await exitPromise;
+}
+
 if (AppConstants.platform == "win") {
   // "relative.echo" has a relative path in the host manifest.
-  add_task(async function test_relative_path() {
-    function background() {
-      let port = browser.runtime.connectNative("relative.echo");
-      let MSG = "test relative echo path";
-      port.onMessage.addListener(msg => {
-        browser.test.assertEq(MSG, msg, "Got expected message back");
-        browser.test.sendMessage("done");
-      });
-      port.postMessage(MSG);
-    }
+  add_task(function test_relative_path() {
+    return simpleTest("relative.echo");
+  });
 
-    let extension = ExtensionTestUtils.loadExtension({
-      background,
-      manifest: {
-        applications: {gecko: {id: ID}},
-        permissions: ["nativeMessaging"],
-      },
-    });
-
-    await extension.startup();
-    await extension.awaitMessage("done");
-
-    let procCount = await getSubprocessCount();
-    equal(procCount, 1, "subprocess is still running");
-    let exitPromise = waitForSubprocessExit();
-    await extension.unload();
-    await exitPromise;
+  // "echocmd" uses a .cmd file instead of a .bat file
+  add_task(function test_cmd_file() {
+    return simpleTest("echocmd");
   });
 }
 
 // Test sendNativeMessage()
 add_task(async function test_sendNativeMessage() {
   async function background() {
     let MSG = {test: "hello world"};
 
--- a/toolkit/modules/subprocess/subprocess_worker_win.js
+++ b/toolkit/modules/subprocess/subprocess_worker_win.js
@@ -444,17 +444,17 @@ class Process extends BaseProcess {
 
     if (/\\cmd\.exe$/i.test(command) && args.length == 3 && /^(\/S)?\/C$/i.test(args[1])) {
       // cmd.exe is insane and requires special treatment.
       args = [this.quoteString(args[0]), "/S/C", `"${args[2]}"`];
     } else {
       args = args.map(arg => this.quoteString(arg));
     }
 
-    if (/\.bat$/i.test(command)) {
+    if (/\.(bat|cmd)$/i.test(command)) {
       command = io.comspec;
       args = ["cmd.exe", "/s/c", `"${args.join(" ")}"`];
     }
 
     let envp = this.stringList(options.environment);
 
     let handles = this.initPipes(options);