Bug 1510281 - Use a private and isolated context for search suggestions. r=mkaply,Ehsan, a=RyanVM
authorMarco Bonardo <mbonardo@mozilla.com>
Fri, 07 Dec 2018 16:21:27 +0000
changeset 506174 66fd4c2ab08185187eda60af2f32f82c3da325bd
parent 506173 a90f08ba09140b0990750ec622931ca8bd0fba0d
child 506175 7fe309a250d8481dbe3f7238917bcc32e058486e
push id10319
push userryanvm@gmail.com
push dateTue, 11 Dec 2018 22:33:37 +0000
treeherdermozilla-beta@b1216967cdd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmkaply, Ehsan, RyanVM
bugs1510281
milestone65.0
Bug 1510281 - Use a private and isolated context for search suggestions. r=mkaply,Ehsan, a=RyanVM Differential Revision: https://phabricator.services.mozilla.com/D13329
toolkit/components/search/SearchSuggestionController.jsm
toolkit/components/search/tests/xpcshell/data/searchSuggestions.sjs
toolkit/components/search/tests/xpcshell/test_searchSuggest_cookies.js
toolkit/components/search/tests/xpcshell/xpcshell.ini
--- a/toolkit/components/search/SearchSuggestionController.jsm
+++ b/toolkit/components/search/SearchSuggestionController.jsm
@@ -14,26 +14,44 @@ XPCOMUtils.defineLazyGlobalGetters(this,
 
 const SEARCH_RESPONSE_SUGGESTION_JSON = "application/x-suggestions+json";
 const DEFAULT_FORM_HISTORY_PARAM      = "searchbar-history";
 const HTTP_OK            = 200;
 const BROWSER_SUGGEST_PREF = "browser.search.suggest.enabled";
 const REMOTE_TIMEOUT_PREF = "browser.search.suggest.timeout";
 const REMOTE_TIMEOUT_DEFAULT = 500; // maximum time (ms) to wait before giving up on a remote suggestions
 
+XPCOMUtils.defineLazyServiceGetter(this, "UUIDGenerator",
+                                   "@mozilla.org/uuid-generator;1",
+                                   "nsIUUIDGenerator");
+
 /**
  * Remote search suggestions will be shown if gRemoteSuggestionsEnabled
  * is true. Global because only one pref observer is needed for all instances.
  */
 var gRemoteSuggestionsEnabled = Services.prefs.getBoolPref(BROWSER_SUGGEST_PREF);
 Services.prefs.addObserver(BROWSER_SUGGEST_PREF, function(aSubject, aTopic, aData) {
   gRemoteSuggestionsEnabled = Services.prefs.getBoolPref(BROWSER_SUGGEST_PREF);
 });
 
 /**
+ * Generates an UUID.
+ * @returns an UUID string, without leading or trailing braces.
+ */
+function uuid() {
+  let uuid = UUIDGenerator.generateUUID().toString();
+  return uuid.slice(1, uuid.length - 1);
+}
+
+// Maps each engine name to a unique firstPartyDomain, so that requests to
+// different engines are isolated from each other and from normal browsing.
+// This is the same for all the controllers.
+var gFirstPartyDomains = new Map();
+
+/**
  * SearchSuggestionController.jsm exists as a helper module to allow multiple consumers to request and display
  * search suggestions from a given engine, regardless of the base implementation. Much of this
  * code was originally in nsSearchSuggestions.js until it was refactored to separate it from the
  * nsIAutoCompleteSearch dependency.
  * One instance of SearchSuggestionController should be used per field since form history results are cached.
  */
 
 /**
@@ -90,27 +108,35 @@ this.SearchSuggestionController.prototyp
   /**
    * The XMLHttpRequest object for remote results.
    */
   _request: null,
 
   // Public methods
 
   /**
+   * Gets the firstPartyDomains Map, useful for tests.
+   * @returns {Map} firstPartyDomains mapped by engine names.
+   */
+  get firstPartyDomains() {
+    return gFirstPartyDomains;
+  },
+
+  /**
    * Fetch search suggestions from all of the providers. Fetches in progress will be stopped and
    * results from them will not be provided.
    *
    * @param {string} searchTerm - the term to provide suggestions for
    * @param {bool} privateMode - whether the request is being made in the context of private browsing
    * @param {nsISearchEngine} engine - search engine for the suggestions.
    * @param {int} userContextId - the userContextId of the selected tab.
    *
    * @return {Promise} resolving to an object containing results or null.
    */
-  fetch(searchTerm, privateMode, engine, userContextId) {
+  fetch(searchTerm, privateMode, engine, userContextId = 0) {
     // There is no smart filtering from previous results here (as there is when looking through
     // history/form data) because the result set returned by the server is different for every typed
     // value - e.g. "ocean breathes" does not return a subset of the results returned for "ocean".
 
     this.stop();
 
     if (!Services.search.isInitialized) {
       throw new Error("Search not initialized yet (how did you get here?)");
@@ -225,19 +251,37 @@ this.SearchSuggestionController.prototyp
    */
   _fetchRemote(searchTerm, engine, privateMode, userContextId) {
     let deferredResponse = PromiseUtils.defer();
     this._request = new XMLHttpRequest();
     let submission = engine.getSubmission(searchTerm,
                                           SEARCH_RESPONSE_SUGGESTION_JSON);
     let method = (submission.postData ? "POST" : "GET");
     this._request.open(method, submission.uri.spec, true);
+    // Don't set or store cookies or on-disk cache.
+    this._request.channel.loadFlags = Ci.nsIChannel.LOAD_ANONYMOUS |
+                                      Ci.nsIChannel.INHIBIT_PERSISTENT_CACHING;
+    // Use a unique first-party domain for each engine, to isolate the
+    // suggestions requests.
+    if (!gFirstPartyDomains.has(engine.name)) {
+      // Use the engine identifier, or an uuid when not available, because the
+      // domain cannot contain invalid chars and the engine name may not be
+      // suitable. When using an uuid the firstPartyDomain of the same engine
+      // will differ across restarts, but that's acceptable for now.
+      // TODO (Bug 1511339): use a persistent unique identifier per engine.
+      gFirstPartyDomains.set(engine.name,
+        `${engine.identifier || uuid()}.search.suggestions.mozilla`);
+    }
+    let firstPartyDomain = gFirstPartyDomains.get(engine.name);
 
-    this._request.setOriginAttributes({userContextId,
-                                       privateBrowsingId: privateMode ? 1 : 0});
+    this._request.setOriginAttributes({
+      userContextId,
+      privateBrowsingId: privateMode ? 1 : 0,
+      firstPartyDomain,
+    });
 
     this._request.mozBackgroundRequest = true; // suppress dialogs and fail silently
 
     this._request.addEventListener("load", this._onRemoteLoaded.bind(this, deferredResponse));
     this._request.addEventListener("error", (evt) => deferredResponse.resolve("HTTP error"));
     // Reject for an abort assuming it's always from .stop() in which case we shouldn't return local
     // or remote results for existing searches.
     this._request.addEventListener("abort", (evt) => deferredResponse.reject("HTTP request aborted"));
--- a/toolkit/components/search/tests/xpcshell/data/searchSuggestions.sjs
+++ b/toolkit/components/search/tests/xpcshell/data/searchSuggestions.sjs
@@ -16,17 +16,20 @@ function handleRequest(request, response
     let result = [query, completions];
     response.write(JSON.stringify(result));
     return result;
   }
 
   response.setStatusLine(request.httpVersion, 200, "OK");
 
   let q = request.method == "GET" ? query.q : undefined;
-  if (q == "no remote" || q == "no results") {
+  if (q == "cookie") {
+    response.setHeader("Set-Cookie", "cookie=1");
+    writeSuggestions(q);
+  } else if (q == "no remote" || q == "no results") {
     writeSuggestions(q);
   } else if (q == "Query Mismatch") {
     writeSuggestions("This is an incorrect query string", ["some result"]);
   } else if (q == "Query Case Mismatch") {
     writeSuggestions(q.toUpperCase(), [q]);
   } else if (q == "") {
     writeSuggestions("", ["The server should never be sent an empty query"]);
   } else if (q && q.startsWith("mo")) {
new file mode 100644
--- /dev/null
+++ b/toolkit/components/search/tests/xpcshell/test_searchSuggest_cookies.js
@@ -0,0 +1,121 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Test that search suggestions from SearchSuggestionController.jsm don't store
+ * cookies.
+ */
+
+"use strict";
+
+ChromeUtils.import("resource://gre/modules/SearchSuggestionController.jsm");
+
+// We must make sure the FormHistoryStartup component is
+// initialized in order for it to respond to FormHistory
+// requests from nsFormAutoComplete.js.
+var formHistoryStartup = Cc["@mozilla.org/satchel/form-history-startup;1"].
+                         getService(Ci.nsIObserver);
+formHistoryStartup.observe(null, "profile-after-change", null);
+
+var httpServer = new HttpServer();
+
+function countCacheEntries() {
+  info("Enumerating cache entries");
+  return new Promise(resolve => {
+    let storage = Services.cache2.diskCacheStorage(Services.loadContextInfo.default, false);
+    storage.asyncVisitStorage({
+      onCacheStorageInfo(num, consumption) {
+        this._num = num;
+      },
+      onCacheEntryInfo(uri) {
+        info("Found cache entry: " + uri.asciiSpec);
+      },
+      onCacheEntryVisitCompleted() {
+        resolve(this._num || 0);
+      },
+    }, true /* Do walk entries */);
+  });
+}
+
+function countCookieEntries() {
+  info("Enumerating cookies");
+  let enumerator = Services.cookies.enumerator;
+  let cookieCount = 0;
+  for (let cookie of enumerator) {
+    info("Cookie:" + cookie.rawHost + " " + JSON.stringify(cookie.originAttributes));
+    cookieCount++;
+    break;
+  }
+  return cookieCount;
+}
+
+add_task(async function setup() {
+  Services.prefs.setBoolPref("browser.search.suggest.enabled", true);
+
+  registerCleanupFunction(async function cleanup() {
+    // Clean up all the data.
+    await new Promise(resolve =>
+      Services.clearData.deleteData(Ci.nsIClearDataService.CLEAR_ALL, resolve));
+    Services.prefs.clearUserPref("browser.search.suggest.enabled");
+  });
+
+  let server = useHttpServer();
+  server.registerContentType("sjs", "sjs");
+
+  let unicodeName = ["\u30a8", "\u30c9"].join("");
+  let engines = await addTestEngines([
+    {
+      name: unicodeName,
+      xmlFileName: "engineMaker.sjs?" + JSON.stringify({
+        baseURL: gDataUrl,
+        name: unicodeName,
+        method: "GET",
+      }),
+    },
+    {
+      name: "engine two",
+      xmlFileName: "engineMaker.sjs?" + JSON.stringify({
+        baseURL: gDataUrl,
+        name: "engine two",
+        method: "GET",
+      }),
+    },
+  ]);
+
+  // Clean up all the data.
+  await new Promise(resolve =>
+    Services.clearData.deleteData(Ci.nsIClearDataService.CLEAR_ALL, resolve));
+  Assert.equal(await countCacheEntries(), 0, "The cache should be empty");
+  Assert.equal(await countCookieEntries(), 0, "Should not find any cookie");
+
+  await test_engine(engines, true);
+  await test_engine(engines, false);
+});
+
+async function test_engine(engines, privateMode) {
+  let controller;
+  await new Promise(resolve => {
+    controller = new SearchSuggestionController((result) => {
+      Assert.equal(result.local.length, 0);
+      Assert.equal(result.remote.length, 0);
+      if (result.term == "cookie") {
+        resolve();
+      }
+    });
+    controller.fetch("test", privateMode, engines[0]);
+    controller.fetch("cookie", privateMode, engines[1]);
+  });
+  Assert.equal(await countCacheEntries(), 0, "The cache should be empty");
+  Assert.equal(await countCookieEntries(), 0, "Should not find any cookie");
+
+  let firstPartyDomain1 = controller.firstPartyDomains.get(engines[0].name);
+  Assert.ok(/^[\.a-z0-9-]+\.search\.suggestions\.mozilla/.test(firstPartyDomain1),
+            "Check firstPartyDomain1");
+
+  let firstPartyDomain2 = controller.firstPartyDomains.get(engines[1].name);
+  Assert.ok(/^[\.a-z0-9-]+\.search\.suggestions\.mozilla/.test(firstPartyDomain2),
+            "Check firstPartyDomain2");
+
+  Assert.notEqual(firstPartyDomain1, firstPartyDomain2,
+                  "Check firstPartyDomain id unique per engine");
+}
--- a/toolkit/components/search/tests/xpcshell/xpcshell.ini
+++ b/toolkit/components/search/tests/xpcshell/xpcshell.ini
@@ -61,16 +61,17 @@ support-files = data/search_ignorelist.j
 [test_defaultEngine.js]
 [test_notifications.js]
 [test_parseSubmissionURL.js]
 [test_SearchStaticData.js]
 [test_addEngine_callback.js]
 [test_multipleIcons.js]
 [test_resultDomain.js]
 [test_searchSuggest.js]
+[test_searchSuggest_cookies.js]
 [test_async.js]
 [test_async_addon.js]
 tags = addons
 [test_async_addon_no_override.js]
 tags = addons
 [test_async_distribution.js]
 [test_async_disthidden.js]
 [test_async_profile_engine.js]