Bug 1473841 - Don't return dot-notation invalid properties; r=Honza.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Tue, 27 Nov 2018 13:46:46 +0000
changeset 504730 22794547ae9ad0014c4aec7653d5a66dffdfc505
parent 504703 8be20508c9d5577ec80c7e181fae203a7f736bc1
child 504731 46d8dde1fc3083c9ea7bf6f78e9c94735df71b20
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1473841
milestone65.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 1473841 - Don't return dot-notation invalid properties; r=Honza. In JsPropertyProvider, if doing a property access (with a dot), check that the results are suited for a dot notation (e.g. `data` is, while `data-test` is not). In case, of an element access, we can return everything. This implies making some changes to some tests which were using invalid dot notation access in some case, which revealed a bug with bracket autocomplete and spaces. So the bracket autocomplete with spaces is now also fixed, and a test case was added for that as well. Differential Revision: https://phabricator.services.mozilla.com/D12726
devtools/shared/webconsole/js-property-provider.js
devtools/shared/webconsole/test/test_jsterm_autocomplete.html
devtools/shared/webconsole/test/unit/test_js_property_provider.js
--- a/devtools/shared/webconsole/js-property-provider.js
+++ b/devtools/shared/webconsole/js-property-provider.js
@@ -6,16 +6,17 @@
 
 "use strict";
 
 const DevToolsUtils = require("devtools/shared/DevToolsUtils");
 
 if (!isWorker) {
   loader.lazyImporter(this, "Parser", "resource://devtools/shared/Parser.jsm");
 }
+loader.lazyRequireGetter(this, "Reflect", "resource://gre/modules/reflect.jsm", true);
 
 // Provide an easy way to bail out of even attempting an autocompletion
 // if an object has way too many properties. Protects against large objects
 // with numeric values that wouldn't be tallied towards MAX_AUTOCOMPLETIONS.
 const MAX_AUTOCOMPLETE_ATTEMPTS = exports.MAX_AUTOCOMPLETE_ATTEMPTS = 100000;
 // Prevent iterating over too many properties during autocomplete suggestions.
 const MAX_AUTOCOMPLETIONS = exports.MAX_AUTOCOMPLETIONS = 1500;
 
@@ -112,22 +113,23 @@ function analyzeInputString(str) {
           const nextNonSpaceCharIndex = after.indexOf(nextNonSpaceChar);
           const previousNonSpaceChar = trimmedBefore[trimmedBefore.length - 1];
 
           // There's only spaces after that, so we can return.
           if (!nextNonSpaceChar) {
             return buildReturnObject();
           }
 
-          // If the previous char in't a dot, and the next one isn't a dot either,
-          // and the current computed statement is not a variable/function/class
-          // declaration, update the start position.
+          // If the previous char isn't a dot or opening bracket, and the next one isn't
+          // one either, and the current computed statement is not a
+          // variable/function/class declaration, update the start position.
           if (
-            previousNonSpaceChar !== "." && nextNonSpaceChar !== "."
-            && !NO_AUTOCOMPLETE_PREFIXES.includes(currentLastStatement)
+            previousNonSpaceChar !== "." && nextNonSpaceChar !== "." &&
+            previousNonSpaceChar !== "[" && nextNonSpaceChar !== "[" &&
+            !NO_AUTOCOMPLETE_PREFIXES.includes(currentLastStatement)
           ) {
             start = i + nextNonSpaceCharIndex;
           }
 
           // Let's jump to handle the next non-space char.
           i = i + nextNonSpaceCharIndex;
         } else if (OPEN_BODY.includes(c)) {
           bodyStack.push({
@@ -474,17 +476,34 @@ function JSPropertyProvider({
     }
   }
 
   const prepareReturnedObject = matches => {
     if (isElementAccess) {
       // If it's an element access, we need to wrap properties in quotes (either the one
       // the user already typed, or `"`).
       matches = wrapMatchesInQuotes(matches, elementAccessQuote);
+    } else if (!isWorker) {
+      // If we're not performing an element access, we need to check that the property
+      // are suited for a dot access. (Reflect.jsm is not available in worker context yet,
+      // see Bug 1507181).
+      matches = new Set([...matches].filter(propertyName => {
+        let valid = true;
+        try {
+          // In order to know if the property is suited for dot notation, we use Reflect
+          // to parse an expression where we try to access the property with a dot. If it
+          // throws, this means that we need to do an element access instead.
+          Reflect.parse(`({${propertyName}: true})`);
+        } catch (e) {
+          valid = false;
+        }
+        return valid;
+      }));
     }
+
     return {isElementAccess, matchProp, matches};
   };
 
   // If the final property is a primitive
   if (typeof obj != "object") {
     return prepareReturnedObject(getMatchedProps(obj, search));
   }
 
--- a/devtools/shared/webconsole/test/test_jsterm_autocomplete.html
+++ b/devtools/shared/webconsole/test/test_jsterm_autocomplete.html
@@ -106,21 +106,23 @@
       doAutocompleteAfterOperator,
       dontAutocompleteAfterDeclaration,
       doKeywordsAutocomplete,
     ];
 
     if (!isWorker) {
       // `Cu` is not defined in workers, then we can't test `Cu.Sandbox`
       tests.push(doAutocompleteSandbox);
-      // Array literal, string and commands completion aren't handled in Workers yet.
+      // Some cases are handled in worker context because we can't use Parser.jsm.
+      // See Bug 1507181.
       tests.push(
         doAutocompleteArray,
         doAutocompleteString,
         doAutocompleteCommands,
+        doAutocompleteBracketSurroundedBySpaces,
       );
     }
 
     for (const test of tests) {
       await test(state.client);
     }
 
     await closeDebugger(state);
@@ -294,21 +296,41 @@
     is(matches.length, 7);
     checkObject(matches,
       ["foo", "foobar", "foobaz", "omg", "omgfoo", "omgstr", "strfoo"]);
 
     matches =
       (await client.autocomplete("window.foobarObject.  foo ; window.foo")).matches;
     is(matches.length, 1);
     checkObject(matches, ["foobarObject"]);
+  }
 
-    matches =
-      (await client.autocomplete("window.emojiObject  .  ")).matches;
+  async function doAutocompleteBracketSurroundedBySpaces(client) {
+    const wrap = (arr, quote = `"`) => arr.map(x => `${quote}${x}${quote}`);
+    let matches = await getAutocompleteMatches(client, "window.foobarObject\n  [")
+    is(matches.length, 7);
+    checkObject(matches,
+      wrap(["foo", "foobar", "foobaz", "omg", "omgfoo", "omgstr", "strfoo"]));
+
+    matches = await getAutocompleteMatches(client, "window.foobarObject\n  ['o")
+    is(matches.length, 3);
+    checkObject(matches, wrap(["omg", "omgfoo", "omgstr"], "'"));
+
+    matches = await getAutocompleteMatches(client, "window.foobarObject\n  [\n  s");
     is(matches.length, 1);
-    checkObject(matches, ["😎"]);
+    checkObject(matches, [`"strfoo"`]);
+
+    matches = await getAutocompleteMatches(client, "window.foobarObject\n  [  ");
+    is(matches.length, 7);
+    checkObject(matches,
+      wrap(["foo", "foobar", "foobaz", "omg", "omgfoo", "omgstr", "strfoo"]));
+
+    matches = await getAutocompleteMatches(client, "window.emojiObject  [   '");
+    is(matches.length, 1);
+    checkObject(matches, [`'😎'`]);
   }
 
   async function doAutocompleteAfterOr(client) {
     info("test autocomplete for 'true || foo'");
     const {matches} = await client.autocomplete("true || foobar");
     is(matches.length, 1, "autocomplete returns expected results");
     is(matches.join("-"), "foobarObject");
   }
@@ -537,11 +559,17 @@ async function doKeywordsAutocomplete(cl
   ok(!matches.includes("function"),
     "'function' is not returned when doing a property access");
 
   info("test autocomplete for 'window[func'");
   matches = (await client.autocomplete("window[func")).matches;
   ok(!matches.includes("function"),
     "'function' is not returned when doing an element access");
   }
+
+  async function getAutocompleteMatches(client, input) {
+    info(`test autocomplete for "${input}"`);
+    const res = (await client.autocomplete(input));
+    return res.matches;
+  }
 </script>
 </body>
 </html>
--- a/devtools/shared/webconsole/test/unit/test_js_property_provider.js
+++ b/devtools/shared/webconsole/test/unit/test_js_property_provider.js
@@ -343,16 +343,24 @@ function runChecks(dbgObject, environmen
   results = propertyProvider("(1.1)[");
   test_has_result(results, `"toFixed"`);
 
   results = propertyProvider("(1)[toFixed");
   test_has_exact_results(results, [`"toFixed"`]);
 
   results = propertyProvider("(1)['toFixed");
   test_has_exact_results(results, ["'toFixed'"]);
+
+  info("Test access on dot-notation invalid property name");
+  results = propertyProvider("testHyphenated.prop");
+  Assert.ok(!results.matches.has("prop-A"),
+    "Does not return invalid property name on dot access");
+
+  results = propertyProvider("testHyphenated['prop");
+  test_has_result(results, `'prop-A'`);
 }
 
 /**
  * A helper that ensures an empty array of results were found.
  * @param Object results
  *        The results returned by JSPropertyProvider.
  */
 function test_has_no_results(results) {