Bug 1473569 - fix faulty errors in formatCommand; r=nchevobbe
authoryulia <ystartsev@mozilla.com>
Fri, 06 Jul 2018 18:03:50 +0200
changeset 425792 3ef659a44ba37b1e5d9885e008c59583d8b03d1c
parent 425791 639574c8a86390d265c65f15b90eecb66cf075b1
child 425798 03d46121ca2b67954e65d62086b0aa18563edfb0
push id66218
push userystartsev@mozilla.com
push dateWed, 11 Jul 2018 09:37:45 +0000
treeherderautoland@3ef659a44ba3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnchevobbe
bugs1473569
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1473569 - fix faulty errors in formatCommand; r=nchevobbe MozReview-Commit-ID: 9PeQySBxWyx
devtools/server/actors/webconsole/commands.js
devtools/server/tests/unit/test_format_command.js
--- a/devtools/server/actors/webconsole/commands.js
+++ b/devtools/server/actors/webconsole/commands.js
@@ -153,52 +153,63 @@ function isStringChar(testChar) {
   return stringChars.includes(testChar);
 }
 
 function checkLastChar(string, testChar) {
   const lastChar = string[string.length - 1];
   return lastChar === testChar;
 }
 
-function hasUnexpectedChar(value, char, rightOffset, leftOffset) {
+function hasUnescapedChar(value, char, rightOffset, leftOffset) {
   const lastPos = value.length - 1;
-  value.slice(rightOffset, lastPos - leftOffset).includes(char);
+  const string = value.slice(rightOffset, lastPos - leftOffset);
+  const index = string.indexOf(char);
+  if (index === -1) {
+    return false;
+  }
+  const prevChar = index > 0 ? string[index - 1] : null;
+  // return false if the unexpected character is escaped, true if it is not
+  return prevChar !== "\\";
 }
 
 function collectString(token, tokens, index) {
   const firstChar = token.value[0];
   const isString = isStringChar(firstChar);
+  const UNESCAPED_CHAR_ERROR = segment =>
+    `String has unescaped \`${firstChar}\` in [${segment}...],` +
+    " may miss a space between arguments";
   let value = token.value;
 
   // the test value is not a string, or it is a string but a complete one
   // i.e. `"test"`, as opposed to `"foo`. In either case, this we can return early
   if (!isString || checkLastChar(value, firstChar)) {
     return { value, offset: 0 };
   }
 
-  if (hasUnexpectedChar(value, firstChar, 1, 0)) {
-    throw Error(`String contains unexpected ${firstChar} character`);
+  if (hasUnescapedChar(value, firstChar, 1, 0)) {
+    throw Error(UNESCAPED_CHAR_ERROR(value));
   }
 
   let offset = null;
   for (let i = index + 1; i <= tokens.length; i++) {
     if (i === tokens.length) {
       throw Error("String does not terminate");
     }
 
     const nextToken = tokens[i];
     if (nextToken.type !== ARG) {
-      throw Error(`String does not terminate before flag ${nextToken.value}`);
-    }
-
-    if (hasUnexpectedChar(nextToken.value, firstChar, 0, 1)) {
-      throw Error(`String contains unexpected ${firstChar} character`);
+      throw Error(`String does not terminate before flag "${nextToken.value}"`);
     }
 
     value = `${value} ${nextToken.value}`;
+
+    if (hasUnescapedChar(nextToken.value, firstChar, 0, 1)) {
+      throw Error(UNESCAPED_CHAR_ERROR(value));
+    }
+
     if (checkLastChar(nextToken.value, firstChar)) {
       offset = i - index;
       break;
     }
   }
   return { value, offset };
 }
 
--- a/devtools/server/tests/unit/test_format_command.js
+++ b/devtools/server/tests/unit/test_format_command.js
@@ -65,31 +65,30 @@ const testcases = [
 
 const edgecases = [
   { input: ":", expectedError: /'' is not a valid command/ },
   { input: ":invalid", expectedError: /'invalid' is not a valid command/ },
   { input: ":screenshot :help", expectedError: /Invalid command/ },
   { input: ":screenshot --", expectedError: /invalid flag/ },
   {
     input: ":screenshot \"fo\"o bar",
-    // XXX Bug 1473569 - this should be: /String contains unexpected `\"` character/
-    expectedError: /String does not terminate/
+    expectedError:
+      /String has unescaped `"` in \["fo"o\.\.\.\], may miss a space between arguments/
   },
   {
     input: ":screenshot \"foo b\"ar",
-    // XXX Bug 1473569 - this should be: /String contains unexpected `\"` character/
-    expectedError: /String does not terminate/
+    expectedError:
+      // eslint-disable-next-line max-len
+      /String has unescaped `"` in \["foo b"ar\.\.\.\], may miss a space between arguments/
   },
   { input: ": screenshot", expectedError: /'' is not a valid command/ },
   { input: ":screenshot \"file name", expectedError: /String does not terminate/ },
   {
     input: ":screenshot \"file name --clipboard",
-    // XXX Bug 1473569 - this should be:
-    // /String does not terminate before flag \"clipboard\"/
-    expectedError: /String does not terminate before flag clipboard/
+    expectedError: /String does not terminate before flag "clipboard"/
   },
   { input: "::screenshot", expectedError: /':screenshot' is not a valid command/ }
 ];
 
 function run_test() {
   testcases.forEach(testcase => {
     Assert.equal(formatCommand(testcase.input), testcase.expectedOutput);
   });