Bug 1468968 - Restrict the concept of root icons and remove orphans, …

…to avoid possible privacy leaks. r=Standard8,mikedeboer

Differential Revision: https://phabricator.services.mozilla.com/D2122

--HG--
extra : moz-landing-system : lando

toolkit/components/places/History.jsm

@@ -801,6 +801,10 @@ var clear = async function(db) {
                               EXCEPT
                               SELECT icon_id FROM moz_icons_to_pages
                             )`);
+    await db.executeCached(`DELETE FROM moz_icons
+                            WHERE root = 1
+                              AND get_host_and_port(icon_url) NOT IN (SELECT host FROM moz_origins)
+                              AND fixup_url(get_host_and_port(icon_url)) NOT IN (SELECT host FROM moz_origins)`);
 
     // Expire annotations.
     await db.execute(`DELETE FROM moz_items_annos WHERE expiration = :expire_session`,
@@ -894,6 +898,10 @@ var cleanupPages = async function(db, pages) {
                             EXCEPT
                             SELECT icon_id FROM moz_icons_to_pages
                           )`);
+  await db.executeCached(`DELETE FROM moz_icons
+                          WHERE root = 1
+                            AND get_host_and_port(icon_url) NOT IN (SELECT host FROM moz_origins)
+                            AND fixup_url(get_host_and_port(icon_url)) NOT IN (SELECT host FROM moz_origins)`);
 
   await db.execute(`DELETE FROM moz_annos
                     WHERE place_id IN ( ${ idsList } )`);

toolkit/components/places/PlacesDBUtils.jsm

@@ -536,6 +536,13 @@ var PlacesDBUtils = {
         )`
       },
 
+      { query:
+        `DELETE FROM moz_icons
+         WHERE root = 1
+           AND get_host_and_port(icon_url) NOT IN (SELECT host FROM moz_origins)
+           AND fixup_url(get_host_and_port(icon_url)) NOT IN (SELECT host FROM moz_origins)`
+      },
+
       // MOZ_HISTORYVISITS
       // F.1 remove orphan visits
       { query:

toolkit/components/places/nsFaviconService.cpp

@@ -374,11 +374,17 @@ nsFaviconService::SetAndFetchFaviconForPage(nsIURI* aPageURI,
     if (StringBeginsWith(icon.host, NS_LITERAL_CSTRING("www."))) {
       icon.host.Cut(0, 4);
     }
-    nsAutoCString path;
-    rv = aFaviconURI->GetPathQueryRef(path);
-    if (NS_SUCCEEDED(rv) && path.EqualsLiteral("/favicon.ico")) {
-      icon.rootIcon = 1;
-    }
+  }
+
+  // A root icon is when the icon and page have the same host and the path
+  // is just /favicon.ico. These icons are considered valid for the whole
+  // origin and expired with the origin through a trigger.
+  nsAutoCString path;
+  if (NS_SUCCEEDED(aFaviconURI->GetPathQueryRef(path)) &&
+      !icon.host.IsEmpty() &&
+      icon.host.Equals(page.host) &&
+      path.EqualsLiteral("/favicon.ico")) {
+    icon.rootIcon = 1;
   }
 
   // If the page url points to an image, the icon's url will be the same.
@@ -448,17 +454,15 @@ nsFaviconService::ReplaceFaviconData(nsIURI* aFaviconURI,
   iconData->fetchMode = FETCH_NEVER;
   nsresult rv = aFaviconURI->GetSpec(iconData->spec);
   NS_ENSURE_SUCCESS(rv, rv);
-  nsAutoCString path;
-  rv = aFaviconURI->GetPathQueryRef(path);
-  if (NS_SUCCEEDED(rv) && path.EqualsLiteral("/favicon.ico")) {
-    iconData->rootIcon = 1;
-  }
   // URIs can arguably lack a host.
   Unused << aFaviconURI->GetHost(iconData->host);
   if (StringBeginsWith(iconData->host, NS_LITERAL_CSTRING("www."))) {
     iconData->host.Cut(0, 4);
   }
 
+  // Note we can't set rootIcon here, because don't know the page it will be
+  // associated with. We'll do that later in SetAndFetchFaviconForPage.
+
   IconPayload payload;
   payload.mimeType = aMimeType;
   payload.data.Assign(TO_CHARBUFFER(aData), aDataLen);

toolkit/components/places/tests/favicons/test_root_icons.js

@@ -104,6 +104,11 @@ add_task(async function test_different_host() {
 
   Assert.equal(await getFaviconUrlForPage(pageURI),
                faviconURI.spec, "Should get the png icon");
+  // Check the icon is not marked as a root icon in the database.
+  let db = await PlacesUtils.promiseDBConnection();
+  let rows = await db.execute("SELECT root FROM moz_icons WHERE icon_url = :url",
+                              {url: faviconURI.spec});
+  Assert.strictEqual(rows[0].getResultByName("root"), 0);
 });
 
 add_task(async function test_same_size() {

toolkit/components/places/tests/history/test_remove.js

@@ -209,6 +209,15 @@ add_task(async function test_orphans() {
   PlacesUtils.favicons.setAndFetchFaviconForPage(
     uri, SMALLPNG_DATA_URI, true, PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE,
     null, Services.scriptSecurityManager.getSystemPrincipal());
+  // Also create a root icon.
+  let faviconURI = Services.io.newURI(uri.spec + "favicon.ico");
+  PlacesUtils.favicons.replaceFaviconDataFromDataURL(
+    faviconURI, SMALLPNG_DATA_URI.spec, 0,
+    Services.scriptSecurityManager.getSystemPrincipal());
+  PlacesUtils.favicons.setAndFetchFaviconForPage(
+    uri, faviconURI, true, PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE,
+    null, Services.scriptSecurityManager.getSystemPrincipal());
+
   PlacesUtils.annotations.setPageAnnotation(uri, "test", "restval", 0,
                                             PlacesUtils.annotations.EXPIRE_NEVER);
 

toolkit/components/places/tests/maintenance/test_preventive_maintenance.js

@@ -894,13 +894,20 @@ tests.push({
 
   setup() {
     // Insert favicon entries
-    let stmt = mDBConn.createStatement("INSERT INTO moz_icons (id, icon_url, fixed_icon_url_hash) VALUES(:favicon_id, :url, hash(fixup_url(:url)))");
+    let stmt = mDBConn.createStatement("INSERT INTO moz_icons (id, icon_url, fixed_icon_url_hash, root) VALUES(:favicon_id, :url, hash(fixup_url(:url)), :root)");
     stmt.params.favicon_id = 1;
     stmt.params.url = "http://www1.mozilla.org/favicon.ico";
+    stmt.params.root = 0;
     stmt.execute();
     stmt.reset();
     stmt.params.favicon_id = 2;
     stmt.params.url = "http://www2.mozilla.org/favicon.ico";
+    stmt.params.root = 0;
+    stmt.execute();
+    stmt.reset();
+    stmt.params.favicon_id = 3;
+    stmt.params.url = "http://www3.mozilla.org/favicon.ico";
+    stmt.params.root = 1;
     stmt.execute();
     stmt.finalize();
     // Insert orphan page.
@@ -923,6 +930,10 @@ tests.push({
     // Check that unused icon has been removed
     stmt.params.favicon_id = 2;
     Assert.ok(!stmt.executeStep());
+    stmt.reset();
+    // Check that unused icon has been removed
+    stmt.params.favicon_id = 3;
+    Assert.ok(!stmt.executeStep());
     stmt.finalize();
     // Check that the orphan page is gone.
     stmt = mDBConn.createStatement("SELECT id FROM moz_pages_w_icons WHERE id = :page_id");