Bug 1706501 - Make CommandLineParserWin::HandleCommandLine take nsTSubstring. r=mossop

Differential Revision: https://phabricator.services.mozilla.com/D113060

toolkit/components/remote/WinRemoteMessage.cpp

@@ -49,7 +49,7 @@ WinRemoteMessageSender::WinRemoteMessageSender(const wchar_t* aCommandLine,
 
 COPYDATASTRUCT* WinRemoteMessageSender::CopyData() { return &mData; }
 
-nsresult WinRemoteMessageReceiver::ParseV0(char* aBuffer) {
+nsresult WinRemoteMessageReceiver::ParseV0(const nsACString& aBuffer) {
   CommandLineParserWin<char> parser;
   parser.HandleCommandLine(aBuffer);
 
@@ -58,42 +58,33 @@ nsresult WinRemoteMessageReceiver::ParseV0(char* aBuffer) {
                             nsICommandLine::STATE_REMOTE_AUTO);
 }
 
-nsresult WinRemoteMessageReceiver::ParseV1(char* aBuffer) {
+nsresult WinRemoteMessageReceiver::ParseV1(const nsACString& aBuffer) {
   CommandLineParserWin<char> parser;
-  parser.HandleCommandLine(aBuffer);
-
-  // Moving |wdpath| to the working dir followed by the first null char.
-  char* wdpath = aBuffer;
-  while (*wdpath) {
-    ++wdpath;
-  }
-  ++wdpath;
+  size_t cch = parser.HandleCommandLine(aBuffer);
+  ++cch;  // skip a null char
 
   nsCOMPtr<nsIFile> workingDir;
-  NS_NewLocalFile(NS_ConvertUTF8toUTF16(wdpath), false,
-                  getter_AddRefs(workingDir));
+  if (cch < aBuffer.Length()) {
+    NS_NewLocalFile(NS_ConvertUTF8toUTF16(Substring(aBuffer, cch)), false,
+                    getter_AddRefs(workingDir));
+  }
 
   mCommandLine = new nsCommandLine();
   return mCommandLine->Init(parser.Argc(), parser.Argv(), workingDir,
                             nsICommandLine::STATE_REMOTE_AUTO);
 }
 
-nsresult WinRemoteMessageReceiver::ParseV2(char16_t* aBuffer) {
+nsresult WinRemoteMessageReceiver::ParseV2(const nsAString& aBuffer) {
   CommandLineParserWin<char16_t> parser;
-  parser.HandleCommandLine(aBuffer);
-
-  // Moving |wdpath| to the working dir followed by the first null char.
-  char16_t* wdpath = aBuffer;
-  while (*wdpath) {
-    ++wdpath;
-  }
-  ++wdpath;
+  size_t cch = parser.HandleCommandLine(aBuffer);
+  ++cch;  // skip a null char
 
   nsCOMPtr<nsIFile> workingDir;
-  NS_NewLocalFile(nsDependentString(wdpath), false, getter_AddRefs(workingDir));
+  if (cch < aBuffer.Length()) {
+    NS_NewLocalFile(Substring(aBuffer, cch), false, getter_AddRefs(workingDir));
+  }
 
   int argc = parser.Argc();
-
   Vector<nsAutoCString> utf8args;
   if (!utf8args.reserve(argc)) {
     return NS_ERROR_OUT_OF_MEMORY;
@@ -110,14 +101,18 @@ nsresult WinRemoteMessageReceiver::ParseV2(char16_t* aBuffer) {
                             nsICommandLine::STATE_REMOTE_AUTO);
 }
 
-nsresult WinRemoteMessageReceiver::Parse(COPYDATASTRUCT* aMessageData) {
+nsresult WinRemoteMessageReceiver::Parse(const COPYDATASTRUCT* aMessageData) {
   switch (static_cast<WinRemoteMessageVersion>(aMessageData->dwData)) {
     case WinRemoteMessageVersion::CommandLineOnly:
-      return ParseV0(reinterpret_cast<char*>(aMessageData->lpData));
+      return ParseV0(nsDependentCSubstring(
+          reinterpret_cast<char*>(aMessageData->lpData), aMessageData->cbData));
     case WinRemoteMessageVersion::CommandLineAndWorkingDir:
-      return ParseV1(reinterpret_cast<char*>(aMessageData->lpData));
+      return ParseV1(nsDependentCSubstring(
+          reinterpret_cast<char*>(aMessageData->lpData), aMessageData->cbData));
     case WinRemoteMessageVersion::CommandLineAndWorkingDirInUtf16:
-      return ParseV2(reinterpret_cast<char16_t*>(aMessageData->lpData));
+      return ParseV2(nsDependentSubstring(
+          reinterpret_cast<char16_t*>(aMessageData->lpData),
+          aMessageData->cbData / sizeof(char16_t)));
     default:
       MOZ_ASSERT_UNREACHABLE("Unsupported message version");
       return NS_ERROR_FAILURE;

toolkit/components/remote/WinRemoteMessage.h

@@ -51,9 +51,9 @@ class WinRemoteMessageSender final {
 class WinRemoteMessageReceiver final {
   nsCOMPtr<nsICommandLineRunner> mCommandLine;
 
-  nsresult ParseV0(char* aBuffer);
-  nsresult ParseV1(char* aBuffer);
-  nsresult ParseV2(char16_t* aBuffer);
+  nsresult ParseV0(const nsACString& aBuffer);
+  nsresult ParseV1(const nsACString& aBuffer);
+  nsresult ParseV2(const nsAString& aBuffer);
 
  public:
   WinRemoteMessageReceiver() = default;
@@ -62,7 +62,7 @@ class WinRemoteMessageReceiver final {
   WinRemoteMessageReceiver& operator=(const WinRemoteMessageReceiver&) = delete;
   WinRemoteMessageReceiver& operator=(WinRemoteMessageReceiver&&) = delete;
 
-  nsresult Parse(COPYDATASTRUCT* aMessageData);
+  nsresult Parse(const COPYDATASTRUCT* aMessageData);
   nsICommandLineRunner* CommandLineRunner();
 };
 

toolkit/xre/CmdLineAndEnvUtils.h

@@ -444,7 +444,10 @@ inline bool SetArgv0ToFullBinaryPath(wchar_t* aArgv[]) {
 // This class converts a command line string into an array of the arguments.
 // It's basically the opposite of MakeCommandLine.  However, the behavior is
 // different from ::CommandLineToArgvW in several ways, such as escaping a
-// backslash or quoting an argument containing whitespaces.
+// backslash or quoting an argument containing whitespaces.  This satisfies
+// the examples at:
+// https://docs.microsoft.com/en-us/cpp/cpp/main-function-command-line-args#results-of-parsing-command-lines
+// https://docs.microsoft.com/en-us/previous-versions/17w5ykft(v=vs.85)
 template <typename T>
 class CommandLineParserWin final {
   int mArgc;
@@ -472,37 +475,44 @@ class CommandLineParserWin final {
   int Argc() const { return mArgc; }
   const T* const* Argv() const { return mArgv; }
 
-  void HandleCommandLine(const T* aCmdLineString) {
+  // Returns the number of characters handled
+  int HandleCommandLine(const nsTSubstring<T>& aCmdLineString) {
     Release();
 
+    if (aCmdLineString.IsEmpty()) {
+      return 0;
+    }
+
     int justCounting = 1;
     // Flags, etc.
     int init = 1;
     int between, quoted, bSlashCount;
     const T* p;
+    const T* const pEnd = aCmdLineString.EndReading();
     nsTAutoString<T> arg;
 
-    // Parse command line args according to MS spec
-    // (see "Parsing C++ Command-Line Arguments" at
-    // http://msdn.microsoft.com/library/devprods/vs6/visualc/vclang/_pluslang_parsing_c.2b2b_.command.2d.line_arguments.htm).
     // We loop if we've not finished the second pass through.
     while (1) {
       // Initialize if required.
       if (init) {
-        p = aCmdLineString;
+        p = aCmdLineString.BeginReading();
         between = 1;
         mArgc = quoted = bSlashCount = 0;
 
         init = 0;
       }
+
+      const T charCurr = (p < pEnd) ? *p : 0;
+      const T charNext = (p + 1 < pEnd) ? *(p + 1) : 0;
+
       if (between) {
         // We are traversing whitespace between args.
         // Check for start of next arg.
-        if (*p != 0 && !wcschr(kCommandLineDelimiter, *p)) {
+        if (charCurr != 0 && !wcschr(kCommandLineDelimiter, charCurr)) {
           // Start of another arg.
           between = 0;
           arg.Truncate();
-          switch (*p) {
+          switch (charCurr) {
             case '\\':
               // Count the backslash.
               bSlashCount = 1;
@@ -513,7 +523,7 @@ class CommandLineParserWin final {
               break;
             default:
               // Add character to arg.
-              arg += *p;
+              arg += charCurr;
               break;
           }
         } else {
@@ -522,7 +532,8 @@ class CommandLineParserWin final {
       } else {
         // We are processing the contents of an argument.
         // Check for whitespace or end.
-        if (*p == 0 || (!quoted && wcschr(kCommandLineDelimiter, *p))) {
+        if (charCurr == 0 ||
+            (!quoted && wcschr(kCommandLineDelimiter, charCurr))) {
           // Process pending backslashes (interpret them
           // literally since they're not followed by a ").
           while (bSlashCount) {
@@ -539,7 +550,7 @@ class CommandLineParserWin final {
           between = 1;
         } else {
           // Still inside argument, process the character.
-          switch (*p) {
+          switch (charCurr) {
             case '"':
               // First, digest preceding backslashes (if any).
               while (bSlashCount > 1) {
@@ -556,7 +567,7 @@ class CommandLineParserWin final {
                 if (quoted) {
                   // Check for special case of consecutive double
                   // quotes inside a quoted section.
-                  if (*(p + 1) == '"') {
+                  if (charNext == '"') {
                     // This implies a literal double-quote.  Fake that
                     // out by causing next double-quote to look as
                     // if it was preceded by a backslash.
@@ -580,13 +591,14 @@ class CommandLineParserWin final {
                 bSlashCount--;
               }
               // Just add next char to the current arg.
-              arg += *p;
+              arg += charCurr;
               break;
           }
         }
       }
+
       // Check for end of input.
-      if (*p) {
+      if (charCurr) {
         // Go to next character.
         p++;
       } else {
@@ -604,6 +616,8 @@ class CommandLineParserWin final {
         }
       }
     }
+
+    return p - aCmdLineString.BeginReading();
   }
 };
 #  endif  // defined(MOZILLA_INTERNAL_API)

toolkit/xre/test/gtest/TestAssembleCommandLineWin.cpp

@@ -92,7 +92,7 @@ TEST(CommandLineParserWin, HandleCommandLine)
   CommandLineParserWin<char> parser;
   for (const auto& testCase : testCases) {
     NS_ConvertUTF16toUTF8 utf8(testCase.mExpected);
-    parser.HandleCommandLine(utf8.get());
+    parser.HandleCommandLine(utf8);
 
     if (utf8.Length() == 0) {
       EXPECT_EQ(parser.Argc(), 0);