Bug 620931 part 2.5 - Properly quote arguments on Windows when starting child processes. r=rstrong
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 12 May 2011 15:44:35 +0200
changeset 69651 fdd5cd8f391f4955307bd3b34021ca2f9dbe9a15
parent 69650 676006c214ad329a115a7e5fc7c1a9315b0ae901
child 69652 88caff1a09d06c002a9a2458902d861e65e39181
push idunknown
push userunknown
push dateunknown
reviewersrstrong
bugs620931
milestone6.0a1
Bug 620931 part 2.5 - Properly quote arguments on Windows when starting child processes. r=rstrong
dom/plugins/ipc/PluginMessageUtils.cpp
ipc/chromium/src/base/command_line.cc
ipc/glue/GeckoChildProcessHost.cpp
--- a/dom/plugins/ipc/PluginMessageUtils.cpp
+++ b/dom/plugins/ipc/PluginMessageUtils.cpp
@@ -110,19 +110,17 @@ ReplaceAll(const string& haystack, const
   }
 
   return munged;
 }
 
 string
 MungePluginDsoPath(const string& path)
 {
-#if defined(XP_WIN)
-  return "\""+ path +"\"";
-#elif defined(OS_LINUX)
+#if defined(OS_LINUX)
   // https://bugzilla.mozilla.org/show_bug.cgi?id=519601
   return ReplaceAll(path, "netscape", "netsc@pe");
 #else
   return path;
 #endif
 }
 
 string
--- a/ipc/chromium/src/base/command_line.cc
+++ b/ipc/chromium/src/base/command_line.cc
@@ -259,45 +259,75 @@ std::wstring CommandLine::PrefixedSwitch
 #if defined(OS_WIN)
 void CommandLine::AppendSwitch(const std::wstring& switch_string) {
   std::wstring prefixed_switch_string = PrefixedSwitchString(switch_string);
   command_line_string_.append(L" ");
   command_line_string_.append(prefixed_switch_string);
   switches_[WideToASCII(switch_string)] = L"";
 }
 
+// Quote a string if necessary, such that CommandLineToArgvW() will
+// always process it as a single argument.
+static std::wstring WindowsStyleQuote(const std::wstring& arg) {
+  // We follow the quoting rules of CommandLineToArgvW.
+  // http://msdn.microsoft.com/en-us/library/17w5ykft.aspx
+  if (arg.find_first_of(L" \\\"\t") == std::wstring::npos) {
+    // No quoting necessary.
+    return arg;
+  }
+
+  std::wstring out;
+  out.push_back(L'"');
+  for (size_t i = 0; i < arg.size(); ++i) {
+    if (arg[i] == '\\') {
+      // Find the extent of this run of backslashes.
+      size_t start = i, end = start + 1;
+      for (; end < arg.size() && arg[end] == '\\'; ++end)
+        /* empty */;
+      size_t backslash_count = end - start;
+
+      // Backslashes are escapes only if the run is followed by a double quote.
+      // Since we also will end the string with a double quote, we escape for
+      // either a double quote or the end of the string.
+      if (end == arg.size() || arg[end] == '"') {
+        // To quote, we need to output 2x as many backslashes.
+        backslash_count *= 2;
+      }
+      for (size_t j = 0; j < backslash_count; ++j)
+        out.push_back('\\');
+
+      // Advance i to one before the end to balance i++ in loop.
+      i = end - 1;
+    } else if (arg[i] == '"') {
+      out.push_back('\\');
+      out.push_back('"');
+    } else {
+      out.push_back(arg[i]);
+    }
+  }
+  out.push_back('"');
+
+  return out;
+}
+
 void CommandLine::AppendSwitchWithValue(const std::wstring& switch_string,
                                         const std::wstring& value_string) {
-  std::wstring value_string_edit;
-
-  // NOTE(jhughes): If the value contains a quotation mark at one
-  //                end but not both, you may get unusable output.
-  if (!value_string.empty() &&
-      (value_string.find(L" ") != std::wstring::npos) &&
-      (value_string[0] != L'"') &&
-      (value_string[value_string.length() - 1] != L'"')) {
-    // need to provide quotes
-    value_string_edit = StringPrintf(L"\"%ls\"", value_string.c_str());
-  } else {
-    value_string_edit = value_string;
-  }
-
+  std::wstring quoted_value_string = WindowsStyleQuote(value_string);
   std::wstring combined_switch_string =
-    PrefixedSwitchStringWithValue(switch_string, value_string_edit);
+    PrefixedSwitchStringWithValue(switch_string, quoted_value_string);
 
   command_line_string_.append(L" ");
   command_line_string_.append(combined_switch_string);
 
   switches_[WideToASCII(switch_string)] = value_string;
 }
 
 void CommandLine::AppendLooseValue(const std::wstring& value) {
-  // TODO(evan): quoting?
   command_line_string_.append(L" ");
-  command_line_string_.append(value);
+  command_line_string_.append(WindowsStyleQuote(value));
 }
 
 void CommandLine::AppendArguments(const CommandLine& other,
                                   bool include_program) {
   // Verify include_program is used correctly.
   // Logic could be shorter but this is clearer.
   DCHECK(include_program ? !other.program().empty() : other.program().empty());
   command_line_string_ += L" " + other.command_line_string_;
--- a/ipc/glue/GeckoChildProcessHost.cpp
+++ b/ipc/glue/GeckoChildProcessHost.cpp
@@ -256,19 +256,17 @@ void GeckoChildProcessHost::InitWindowsG
   // properly group with the parent app on the Win7 taskbar.
   nsCOMPtr<nsIWinTaskbar> taskbarInfo =
     do_GetService(NS_TASKBAR_CONTRACTID);
   if (taskbarInfo) {
     PRBool isSupported = PR_FALSE;
     taskbarInfo->GetAvailable(&isSupported);
     nsAutoString appId;
     if (isSupported && NS_SUCCEEDED(taskbarInfo->GetDefaultGroupId(appId))) {
-      mGroupId.Assign(PRUnichar('\"'));
       mGroupId.Append(appId);
-      mGroupId.Append(PRUnichar('\"'));
     } else {
       mGroupId.AssignLiteral("-");
     }
   }
 }
 #endif
 
 bool