Bug 1321637 - Fix execution of batch files with spaces in path. r=mhowell, r=aswan, a=jcristau
authorKris Maglione <maglione.k@gmail.com>
Sun, 22 Jan 2017 16:23:33 -0800
changeset 375679 44d3a67db0f5aec366b6b00dbdfec1caa577ae63
parent 375678 f5f482af2c7937f6db5a1180c7f09ba1bdc8c590
child 375680 55d70a7681c3ced07e5d5301f30a15a24b8f301e
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmhowell, aswan, jcristau
bugs1321637
milestone53.0a2
Bug 1321637 - Fix execution of batch files with spaces in path. r=mhowell, r=aswan, a=jcristau MozReview-Commit-ID: 8Q3KFLnXEkl
toolkit/components/extensions/test/xpcshell/head_native_messaging.js
toolkit/components/extensions/test/xpcshell/test_native_messaging.js
toolkit/modules/subprocess/subprocess_win.jsm
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
@@ -10,17 +10,19 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 XPCOMUtils.defineLazyModuleGetter(this, "OS",
                                   "resource://gre/modules/osfile.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "setTimeout",
                                   "resource://gre/modules/Timer.jsm");
 
 let {Subprocess, SubprocessImpl} = Cu.import("resource://gre/modules/Subprocess.jsm", {});
 
 
-let tmpDir = FileUtils.getDir("TmpD", ["NativeMessaging"]);
+// It's important that we use a space in this directory name to make sure we
+// correctly handle executing batch files with spaces in their path.
+let tmpDir = FileUtils.getDir("TmpD", ["Native Messaging"]);
 tmpDir.createUnique(Ci.nsIFile.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
 
 do_register_cleanup(() => {
   tmpDir.remove(true);
 });
 
 function getPath(filename) {
   return OS.Path.join(tmpDir.path, filename);
@@ -85,20 +87,22 @@ 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 batPath = getPath(`${script.name}.bat`);
+        // It's important that we use a space in this filename. See directory
+        // name comment above.
+        let batPath = getPath(`batch ${script.name}.bat`);
         let scriptPath = getPath(`${script.name}.py`);
 
-        let batBody = `@ECHO OFF\n${pythonPath} -u ${scriptPath} %*\n`;
+        let batBody = `@ECHO OFF\n${pythonPath} -u "${scriptPath}" %*\n`;
         yield 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)]]) {
           script.name = name;
           let manifestPath = yield writeManifest(script, scriptPath, path);
 
--- a/toolkit/components/extensions/test/xpcshell/test_native_messaging.js
+++ b/toolkit/components/extensions/test/xpcshell/test_native_messaging.js
@@ -251,17 +251,17 @@ while True:
     type: "stdio",
     allowed_extensions: [ID],
   };
 
   if (AppConstants.platform == "win") {
     yield OS.File.writeAtomic(scriptPath, SCRIPT);
 
     let batPath = OS.Path.join(userDir.path, "wontdie.bat");
-    let batBody = `@ECHO OFF\n${PYTHON} -u ${scriptPath} %*\n`;
+    let batBody = `@ECHO OFF\n${PYTHON} -u "${scriptPath}" %*\n`;
     yield OS.File.writeAtomic(batPath, batBody);
     yield OS.File.setPermissions(batPath, {unixMode: 0o755});
 
     manifest.path = batPath;
     yield writeManifest(manifestPath, manifest);
 
     registry.setValue(Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,
                       `${REGPATH}\\wontdie`, "", manifestPath);
--- a/toolkit/modules/subprocess/subprocess_win.jsm
+++ b/toolkit/modules/subprocess/subprocess_win.jsm
@@ -15,29 +15,33 @@ var {classes: Cc, interfaces: Ci, utils:
 
 var EXPORTED_SYMBOLS = ["SubprocessImpl"];
 
 Cu.import("resource://gre/modules/AppConstants.jsm");
 Cu.import("resource://gre/modules/ctypes.jsm");
 Cu.import("resource://gre/modules/osfile.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/subprocess/subprocess_common.jsm");
 
+XPCOMUtils.defineLazyServiceGetter(this, "env", "@mozilla.org/process/environment;1", "nsIEnvironment");
+
 Services.scriptloader.loadSubScript("resource://gre/modules/subprocess/subprocess_shared.js", this);
 Services.scriptloader.loadSubScript("resource://gre/modules/subprocess/subprocess_shared_win.js", this);
 
 class WinPromiseWorker extends PromiseWorker {
   constructor(...args) {
     super(...args);
 
     this.signalEvent = libc.CreateSemaphoreW(null, 0, 32, null);
 
     this.call("init", [{
       breakAwayFromJob: !AppConstants.isPlatformAndVersionAtLeast("win", "6.2"),
+      comspec: env.get("COMSPEC"),
       signalEvent: String(ctypes.cast(this.signalEvent, ctypes.uintptr_t).value),
     }]);
   }
 
   signalWorker() {
     libc.ReleaseSemaphore(this.signalEvent, 1, null);
   }
 
--- a/toolkit/modules/subprocess/subprocess_worker_win.js
+++ b/toolkit/modules/subprocess/subprocess_worker_win.js
@@ -442,16 +442,21 @@ 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)) {
+      command = io.comspec;
+      args = ["cmd.exe", "/s/c", `"${args.join(" ")}"`];
+    }
+
     let envp = this.stringList(options.environment);
 
     let handles = this.initPipes(options);
 
     let processFlags = win32.CREATE_NO_WINDOW
                      | win32.CREATE_SUSPENDED
                      | win32.CREATE_UNICODE_ENVIRONMENT;
 
@@ -593,16 +598,18 @@ io = {
 
   processes: new Map(),
 
   messageCount: 0,
 
   running: true,
 
   init(details) {
+    this.comspec = details.comspec;
+
     let signalEvent = ctypes.cast(ctypes.uintptr_t(details.signalEvent),
                                   win32.HANDLE);
     this.signal = new Signal(signalEvent);
     this.updatePollEvents();
 
     this.breakAwayFromJob = details.breakAwayFromJob;
 
     setTimeout(this.loop.bind(this), 0);