Add a unit test for CommandLine that makes sure that GetProgram will not retu...

base/command_line.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/command_line.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #include <shellapi.h>
 #elif defined(OS_POSIX)
@@ skipping 36 matching lines @@
 #endif
 
 #if defined(OS_WIN)
 // Lowercase a string.  This is used to lowercase switch names.
 // Is this what we really want?  It seems crazy to me.  I've left it in
 // for backwards compatibility on Windows.
 static void Lowercase(std::string* parameter) {
   transform(parameter->begin(), parameter->end(), parameter->begin(),
             tolower);
 }
-#endif
+
+// Trims the quotes from the beginning and end of a path.
+CommandLine::StringType TrimQuotes(const FilePath::StringType& path) {
+  if (!path.empty() && path[0] == '"' && path[path.length() - 1] == '"')
+    return path.substr(1, path.length() - 2);
+  return path;
+}
+#endif  // defined(OS_WIN)
 
 CommandLine::~CommandLine() {
 }
 
 #if defined(OS_WIN)
 CommandLine::CommandLine(NoProgram no_program) {
 }
 
 void CommandLine::ParseFromString(const std::wstring& command_line) {
   TrimWhitespace(command_line, TRIM_ALL, &command_line_string_);
@@ skipping 39 matching lines @@
 
 // static
 CommandLine CommandLine::FromString(const std::wstring& command_line) {
   CommandLine cmd;
   cmd.ParseFromString(command_line);
   return cmd;
 }
 
 CommandLine::CommandLine(const FilePath& program) {
   if (!program.empty()) {
-    program_ = program.value();
+    program_ = TrimQuotes(program.value());

[Evan Martin, 2010/11/15 22:21:57]

To be clear, if there are quotes here something has gone horribly, wrong, right?

Can you have it test for quotes and hit a NOTREACHED() in that case?

I guess you can also remove that TODO then.
(But what if program.value() ends with a backslash?  I don't really understand
your threat model here...)

[tommi (sloooow) - chröme, 2010/11/15 22:41:23]

I think so, yes.  However I cleaned up quite a bit of code last week that was
using it's own command line parsing with wstring but maybe there still is code
out there that uses its own command line parsing and somehow could give
CommandLine a quoted FilePath :-/  Unlikely though.
Yes.
I don't have a threat model, but that's an interesting point.  In that case the
file path could only represent a directory...

Do you want me to change it to:

program_ = WindowsStyleQuote(program, true);

where:

std::wstring WindowsStyleQuote(const std::wstring& arg, bool always_quote) {
  if (!always_quote && arg.find_first_of(L" \\\"") == std::wstring::npos) {
    // No quoting necessary.
    return arg;
  }

  // .. rest same as it currently is
}

     // TODO(evanm): proper quoting here.
-    command_line_string_ = L'"' + program.value() + L'"';
+    command_line_string_ = L'"' + program_ + L'"';
   }
 }
 
 #elif defined(OS_POSIX)
 CommandLine::CommandLine(NoProgram no_program) {
   // Push an empty argument, because we always assume argv_[0] is a program.
   argv_.push_back("");
 }
 
 CommandLine::CommandLine(int argc, const char* const* argv) {
@@ skipping 405 matching lines @@
 
 // private
 CommandLine::CommandLine() {
 }
 
 // static
 CommandLine* CommandLine::ForCurrentProcessMutable() {
   DCHECK(current_process_commandline_);
   return current_process_commandline_;
 }