base::CommandLine: Added optional quoting of placeholder arguments.

Added an optional parameter |quote_placeholder| to GetCommandLineString
and GetArgumentsString. If true, placeholder command-line arguments such
as "%1" will also be quoted. These need quoting in case the substituted
argument has a space in it. It was previously not possible to build a
CommandLine that would contain a quoted "%1" when converted to a string.

Only affects Windows.

BUG=416785

Review URL: https://codereview.chromium.org/595803002

Cr-Commit-Position: refs/heads/master@{#297617}

3 files changed, 118 insertions, 49 deletions

diff --git a/base/command_line.cc b/base/command_line.cc
index 9bad539..1f5edce 100644
--- a/base/command_line.cc
+++ b/base/command_line.cc
@@ -105,10 +105,16 @@ std::string LowerASCIIOnWindows(const std::string& string) {
 
 #if defined(OS_WIN)
 // Quote a string as necessary for CommandLineToArgvW compatiblity *on Windows*.
-base::string16 QuoteForCommandLineToArgvW(const base::string16& arg) {
+base::string16 QuoteForCommandLineToArgvW(const base::string16& arg,
+                                          bool quote_placeholders) {
   // We follow the quoting rules of CommandLineToArgvW.
   // http://msdn.microsoft.com/en-us/library/17w5ykft.aspx
-  if (arg.find_first_of(L" \\\"") == base::string16::npos) {
+  base::string16 quotable_chars(L" \\\"");
+  // We may also be required to quote '%', which is commonly used in a command
+  // line as a placeholder. (It may be substituted for a string with spaces.)
+  if (quote_placeholders)
+    quotable_chars.push_back(L'%');
+  if (arg.find_first_of(quotable_chars) == base::string16::npos) {
     // No quoting necessary.
     return arg;
   }
@@ -247,49 +253,6 @@ void CommandLine::InitFromArgv(const StringVector& argv) {
   AppendSwitchesAndArguments(*this, argv);
 }
 
-CommandLine::StringType CommandLine::GetCommandLineString() const {
-  StringType string(argv_[0]);
-#if defined(OS_WIN)
-  string = QuoteForCommandLineToArgvW(string);
-#endif
-  StringType params(GetArgumentsString());
-  if (!params.empty()) {
-    string.append(StringType(FILE_PATH_LITERAL(" ")));
-    string.append(params);
-  }
-  return string;
-}
-
-CommandLine::StringType CommandLine::GetArgumentsString() const {
-  StringType params;
-  // Append switches and arguments.
-  bool parse_switches = true;
-  for (size_t i = 1; i < argv_.size(); ++i) {
-    StringType arg = argv_[i];
-    StringType switch_string;
-    StringType switch_value;
-    parse_switches &= arg != kSwitchTerminator;
-    if (i > 1)
-      params.append(StringType(FILE_PATH_LITERAL(" ")));
-    if (parse_switches && IsSwitch(arg, &switch_string, &switch_value)) {
-      params.append(switch_string);
-      if (!switch_value.empty()) {
-#if defined(OS_WIN)
-        switch_value = QuoteForCommandLineToArgvW(switch_value);
-#endif
-        params.append(kSwitchValueSeparator + switch_value);
-      }
-    }
-    else {
-#if defined(OS_WIN)
-      arg = QuoteForCommandLineToArgvW(arg);
-#endif
-      params.append(arg);
-    }
-  }
-  return params;
-}
-
 FilePath CommandLine::GetProgram() const {
   return FilePath(argv_[0]);
 }
@@ -439,4 +402,50 @@ void CommandLine::ParseFromString(const base::string16& command_line) {
 }
 #endif
 
+CommandLine::StringType CommandLine::GetCommandLineStringInternal(
+    bool quote_placeholders) const {
+  StringType string(argv_[0]);
+#if defined(OS_WIN)
+  string = QuoteForCommandLineToArgvW(string, quote_placeholders);
+#endif
+  StringType params(GetArgumentsStringInternal(quote_placeholders));
+  if (!params.empty()) {
+    string.append(StringType(FILE_PATH_LITERAL(" ")));
+    string.append(params);
+  }
+  return string;
+}
+
+CommandLine::StringType CommandLine::GetArgumentsStringInternal(
+    bool quote_placeholders) const {
+  StringType params;
+  // Append switches and arguments.
+  bool parse_switches = true;
+  for (size_t i = 1; i < argv_.size(); ++i) {
+    StringType arg = argv_[i];
+    StringType switch_string;
+    StringType switch_value;
+    parse_switches &= arg != kSwitchTerminator;
+    if (i > 1)
+      params.append(StringType(FILE_PATH_LITERAL(" ")));
+    if (parse_switches && IsSwitch(arg, &switch_string, &switch_value)) {
+      params.append(switch_string);
+      if (!switch_value.empty()) {
+#if defined(OS_WIN)
+        switch_value =
+            QuoteForCommandLineToArgvW(switch_value, quote_placeholders);
+#endif
+        params.append(kSwitchValueSeparator + switch_value);
+      }
+    }
+    else {
+#if defined(OS_WIN)
+      arg = QuoteForCommandLineToArgvW(arg, quote_placeholders);
+#endif
+      params.append(arg);
+    }
+  }
+  return params;
+}
+
 }  // namespace base
diff --git a/base/command_line.h b/base/command_line.h
index 84bca28..1829e63 100644
--- a/base/command_line.h
+++ b/base/command_line.h
@@ -98,12 +98,41 @@ class BASE_EXPORT CommandLine {
   // Constructs and returns the represented command line string.
   // CAUTION! This should be avoided on POSIX because quoting behavior is
   // unclear.
-  StringType GetCommandLineString() const;
+  StringType GetCommandLineString() const {
+    return GetCommandLineStringInternal(false);
+  }
+
+#if defined(OS_WIN)
+  // Constructs and returns the represented command line string. Assumes the
+  // command line contains placeholders (eg, %1) and quotes any program or
+  // argument with a '%' in it. This should be avoided unless the placeholder is
+  // required by an external interface (eg, the Windows registry), because it is
+  // not generally safe to replace it with an arbitrary string. If possible,
+  // placeholders should be replaced *before* converting the command line to a
+  // string.
+  StringType GetCommandLineStringWithPlaceholders() const {
+    return GetCommandLineStringInternal(true);
+  }
+#endif
 
   // Constructs and returns the represented arguments string.
   // CAUTION! This should be avoided on POSIX because quoting behavior is
   // unclear.
-  StringType GetArgumentsString() const;
+  StringType GetArgumentsString() const {
+    return GetArgumentsStringInternal(false);
+  }
+
+#if defined(OS_WIN)
+  // Constructs and returns the represented arguments string. Assumes the
+  // command line contains placeholders (eg, %1) and quotes any argument with a
+  // '%' in it. This should be avoided unless the placeholder is required by an
+  // external interface (eg, the Windows registry), because it is not generally
+  // safe to replace it with an arbitrary string. If possible, placeholders
+  // should be replaced *before* converting the arguments to a string.
+  StringType GetArgumentsStringWithPlaceholders() const {
+    return GetArgumentsStringInternal(true);
+  }
+#endif
 
   // Returns the original command line string as a vector of strings.
   const StringVector& argv() const { return argv_; }
@@ -174,6 +203,14 @@ class BASE_EXPORT CommandLine {
   //   CommandLine cl(*CommandLine::ForCurrentProcess());
   //   cl.AppendSwitch(...);
 
+  // Internal version of GetCommandLineString. If |quote_placeholders| is true,
+  // also quotes parts with '%' in them.
+  StringType GetCommandLineStringInternal(bool quote_placeholders) const;
+
+  // Internal version of GetArgumentsString. If |quote_placeholders| is true,
+  // also quotes parts with '%' in them.
+  StringType GetArgumentsStringInternal(bool quote_placeholders) const;
+
   // The singleton CommandLine representing the current process's command line.
   static CommandLine* current_process_commandline_;
 
diff --git a/base/command_line_unittest.cc b/base/command_line_unittest.cc
index 8aed618..8fdd605 100644
--- a/base/command_line_unittest.cc
+++ b/base/command_line_unittest.cc
@@ -190,12 +190,14 @@ TEST(CommandLineTest, GetArgumentsString) {
   static const char kSecondArgName[] = "arg2";
   static const char kThirdArgName[] = "arg with space";
   static const char kFourthArgName[] = "nospace";
+  static const char kFifthArgName[] = "%1";
 
   CommandLine cl(CommandLine::NO_PROGRAM);
   cl.AppendSwitchPath(kFirstArgName, FilePath(kPath1));
   cl.AppendSwitchPath(kSecondArgName, FilePath(kPath2));
   cl.AppendArg(kThirdArgName);
   cl.AppendArg(kFourthArgName);
+  cl.AppendArg(kFifthArgName);
 
 #if defined(OS_WIN)
   CommandLine::StringType expected_first_arg(
@@ -206,11 +208,13 @@ TEST(CommandLineTest, GetArgumentsString) {
       base::UTF8ToUTF16(kThirdArgName));
   CommandLine::StringType expected_fourth_arg(
       base::UTF8ToUTF16(kFourthArgName));
+  CommandLine::StringType expected_fifth_arg(base::UTF8ToUTF16(kFifthArgName));
 #elif defined(OS_POSIX)
   CommandLine::StringType expected_first_arg(kFirstArgName);
   CommandLine::StringType expected_second_arg(kSecondArgName);
   CommandLine::StringType expected_third_arg(kThirdArgName);
   CommandLine::StringType expected_fourth_arg(kFourthArgName);
+  CommandLine::StringType expected_fifth_arg(kFifthArgName);
 #endif
 
 #if defined(OS_WIN)
@@ -238,8 +242,21 @@ TEST(CommandLineTest, GetArgumentsString) {
               .append(expected_third_arg)
               .append(QUOTE_ON_WIN)
               .append(FILE_PATH_LITERAL(" "))
-              .append(expected_fourth_arg);
-  EXPECT_EQ(expected_str, cl.GetArgumentsString());
+              .append(expected_fourth_arg)
+              .append(FILE_PATH_LITERAL(" "));
+
+  CommandLine::StringType expected_str_no_quote_placeholders(expected_str);
+  expected_str_no_quote_placeholders.append(expected_fifth_arg);
+  EXPECT_EQ(expected_str_no_quote_placeholders, cl.GetArgumentsString());
+
+#if defined(OS_WIN)
+  CommandLine::StringType expected_str_quote_placeholders(expected_str);
+  expected_str_quote_placeholders.append(QUOTE_ON_WIN)
+                                 .append(expected_fifth_arg)
+                                 .append(QUOTE_ON_WIN);
+  EXPECT_EQ(expected_str_quote_placeholders,
+            cl.GetArgumentsStringWithPlaceholders());
+#endif
 }
 
 // Test methods for appending switches to a command line.
@@ -349,6 +366,12 @@ TEST(CommandLineTest, ProgramQuotes) {
   // Check that quotes are added to command line string paths containing spaces.
   CommandLine::StringType cmd_string(cl_program_path.GetCommandLineString());
   EXPECT_EQ(L"\"Program Path\"", cmd_string);
+
+  // Check the optional quoting of placeholders in programs.
+  CommandLine cl_quote_placeholder(base::FilePath(L"%1"));
+  EXPECT_EQ(L"%1", cl_quote_placeholder.GetCommandLineString());
+  EXPECT_EQ(L"\"%1\"",
+            cl_quote_placeholder.GetCommandLineStringWithPlaceholders());
 }
 #endif