Bug 1556068 - Track JsPropertyProvider's analyzeInput string and bail if it's taking too long. r=Honza.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Thu, 06 Jun 2019 13:08:21 +0000
changeset 477603 0a7fc2e413adbcb44d40e2a9f559fbe7a2b6c985
parent 477602 f795f0184e3e6d3bd2bd7fd1d862e2160ed07613
child 477604 745cae0780eee5e4d2fe6211ffa98ef4d18cb33c
push id36119
push userncsoregi@mozilla.com
push dateThu, 06 Jun 2019 21:52:09 +0000
treeherdermozilla-central@6a81efd823db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1556068
milestone69.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 1556068 - Track JsPropertyProvider's analyzeInput string and bail if it's taking too long. r=Honza. It can happen that the string we receive is quite large and as a result takes a long time to analyze, freezing the process. This patch simply tracks for how long the for loop is running, and bail out if it's greater than a given time (set to 2500 ms for now). This is mostly a safeguard, a future patch should try to improve the performance of the function itself. Differential Revision: https://phabricator.services.mozilla.com/D33791
devtools/shared/webconsole/js-property-provider.js
--- a/devtools/shared/webconsole/js-property-provider.js
+++ b/devtools/shared/webconsole/js-property-provider.js
@@ -29,16 +29,17 @@ const OPEN_BODY = "{[(".split("");
 const CLOSE_BODY = "}])".split("");
 const OPEN_CLOSE_BODY = {
   "{": "}",
   "[": "]",
   "(": ")",
 };
 
 const NO_AUTOCOMPLETE_PREFIXES = ["var", "const", "let", "function", "class"];
+const OPERATOR_CHARS_SET = new Set(";,:=<>+-*/%|&^~?!".split(""));
 
 function hasArrayIndex(str) {
   return /\[\d+\]$/.test(str);
 }
 
 /**
  * Analyses a given string to find the last statement that is interesting for
  * later completion.
@@ -83,29 +84,41 @@ function analyzeInputString(str) {
 
     return {
       state,
       lastStatement: characters.slice(start).join(""),
       isElementAccess,
     };
   };
 
+  const TIMEOUT = 2500;
+  const startingTime = Date.now();
+
   for (let i = 0; i < characters.length; i++) {
+    // We are possibly dealing with a very large string that would take a long time to
+    // analyze (and freeze the process). If the function has been running for more than
+    // a given time, we stop the analysis (this isn't too bad because the only
+    // consequence is that we won't provide autocompletion items).
+    if (Date.now() - startingTime > TIMEOUT) {
+      return {
+        err: "timeout",
+      };
+    }
+
     c = characters[i];
-
     switch (state) {
       // Normal JS state.
       case STATE_NORMAL:
         if (c == '"') {
           state = STATE_DQUOTE;
         } else if (c == "'") {
           state = STATE_QUOTE;
         } else if (c == "`") {
           state = STATE_TEMPLATE_LITERAL;
-        } else if (";,:=<>+-*/%|&^~?!".split("").includes(c)) {
+        } else if (OPERATOR_CHARS_SET.has(c)) {
           // If the character is an operator, we need to update the start position.
           start = i + 1;
         } else if (c == " ") {
           const currentLastStatement = characters.slice(start, i).join("");
           const before = characters.slice(0, i);
           const after = characters.slice(i + 1);
           const trimmedBefore = Array.from(before.join("").trimRight());
           const trimmedAfter = Array.from(after.join("").trimLeft());
@@ -271,16 +284,17 @@ function JSPropertyProvider({
     err,
     state,
     lastStatement,
     isElementAccess,
   } = analyzeInputString(inputValue);
 
   // There was an error analysing the string.
   if (err) {
+    console.error("Failed to analyze input string", err);
     return null;
   }
 
   // If the current state is not STATE_NORMAL, then we are inside of an string
   // which means that no completion is possible.
   if (state != STATE_NORMAL) {
     return null;
   }