Bug 1473569 - fix faulty errors in formatCommand; r=nchevobbe draft
authoryulia <ystartsev@mozilla.com>
Fri, 06 Jul 2018 18:03:50 +0200
changeset 816059 09ea09170531c0f7fa14e37ecb1e9dd2423ca4b3
parent 814904 fa376bf17cc95539f5e37186977d760296fb5093
push id115734
push userbmo:ystartsev@mozilla.com
push dateTue, 10 Jul 2018 14:40:09 +0000
reviewersnchevobbe
bugs1473569
milestone63.0a1
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);
   });