Bug 1134615 - Refactor the blocklist matching code in _getPluginBlocklistState into _getPluginBlocklistEntry so that getPluginBlocklistURL and getPluginInfoURL return the correct block entry. r=Mossop

--HG--
extra : rebase_source : ca6e2fe51eb28e8ddd9203d22a57f68096a4b405
extra : amend_source : 62ec06e414a88562bfe1163be2ab8f02602a083e

toolkit/mozapps/extensions/nsBlocklistService.js

@@ -280,26 +280,6 @@ function parseRegExp(aStr) {
   return new RegExp(pattern, flags);
 }
 
-/**
- * Helper function to test if the blockEntry matches with the plugin.
- *
- * @param   blockEntry
- *          The plugin blocklist entries to compare against.
- * @param   plugin
- *          The nsIPluginTag to get the blocklist state for.
- * @returns True if the blockEntry matches the plugin, false otherwise.
- */
-function matchesAllPluginNames(blockEntry, plugin) {
-  for (let name in blockEntry.matches) {
-    if (!(name in plugin) ||
-        typeof(plugin[name]) != "string" ||
-        !blockEntry.matches[name].test(plugin[name])) {
-      return false;
-    }
-  }
-  return true;
-}
-
 /**
  * Manages the Blocklist. The Blocklist is a representation of the contents of
  * blocklist.xml and allows us to remotely disable / re-enable blocklisted
@@ -1046,8 +1026,8 @@ Blocklist.prototype = {
   },
 
   /**
-   * Private version of getPluginBlocklistState that allows the caller to pass in
+   * Private helper to get the blocklist entry for a plugin given a set of
-   * the plugin blocklist entries.
+   * blocklist entries and versions.
    *
    * @param   plugin
    *          The nsIPluginTag to get the blocklist state for.
@@ -1059,13 +1039,13 @@ Blocklist.prototype = {
    * @param   toolkitVersion
    *          The toolkit version to compare to, will use the current version if
    *          null.
-   * @returns The blocklist state for the item, one of the STATE constants as
+   * @returns {entry: blocklistEntry, version: blocklistEntryVersion},
-   *          defined in nsIBlocklistService.
+   *          or null if there is no matching entry.
    */
-  _getPluginBlocklistState: function Blocklist_getPluginBlocklistState(plugin,
+  _getPluginBlocklistEntry: function Blocklist_getPluginBlocklistEntry(plugin,
                             pluginEntries, appVersion, toolkitVersion) {
     if (!gBlocklistEnabled)
-      return Ci.nsIBlocklistService.STATE_NOT_BLOCKED;
+      return null;
 
     // Not all applications implement nsIXULAppInfo (e.g. xpcshell doesn't).
     if (!appVersion && !gApp.version)
@@ -1092,52 +1072,67 @@ Blocklist.prototype = {
 
       for (let blockEntryVersion of blockEntry.versions) {
         if (blockEntryVersion.includesItem(plugin.version, appVersion,
-                                                toolkitVersion)) {
+                                           toolkitVersion)) {
-          if (blockEntryVersion.severity >= gBlocklistLevel)
+          return {entry: blockEntry, version: blockEntryVersion};
-            return Ci.nsIBlocklistService.STATE_BLOCKED;
-          if (blockEntryVersion.severity == SEVERITY_OUTDATED) {
-            let vulnerabilityStatus = blockEntryVersion.vulnerabilityStatus;
-            if (vulnerabilityStatus == VULNERABILITYSTATUS_UPDATE_AVAILABLE)
-              return Ci.nsIBlocklistService.STATE_VULNERABLE_UPDATE_AVAILABLE;
-            if (vulnerabilityStatus == VULNERABILITYSTATUS_NO_UPDATE)
-              return Ci.nsIBlocklistService.STATE_VULNERABLE_NO_UPDATE;
-            return Ci.nsIBlocklistService.STATE_OUTDATED;
-          }
-          return Ci.nsIBlocklistService.STATE_SOFTBLOCKED;
         }
       }
     }
 
-    return Ci.nsIBlocklistService.STATE_NOT_BLOCKED;
+    return null;
   },
 
   /**
-   * Get the matching blocklist entry for the passed plugin, if
+   * Private version of getPluginBlocklistState that allows the caller to pass in
-   * available.
+   * the plugin blocklist entries.
-   * @param plugin The plugin to find the block entry for.
+   *
-   * @returns The block entry which matches the passed plugin, null
+   * @param   plugin
-   *          otherwise.
+   *          The nsIPluginTag to get the blocklist state for.
+   * @param   pluginEntries
+   *          The plugin blocklist entries to compare against.
+   * @param   appVersion
+   *          The application version to compare to, will use the current
+   *          version if null.
+   * @param   toolkitVersion
+   *          The toolkit version to compare to, will use the current version if
+   *          null.
+   * @returns The blocklist state for the item, one of the STATE constants as
+   *          defined in nsIBlocklistService.
    */
-  _getPluginBlockEntry: function (plugin) {
+  _getPluginBlocklistState: function Blocklist_getPluginBlocklistState(plugin,
-    if (!gBlocklistEnabled)
+                            pluginEntries, appVersion, toolkitVersion) {
-      return null;
 
-    if (!this._isBlocklistLoaded())
+    let r = this._getPluginBlocklistEntry(plugin, pluginEntries,
-      this._loadBlocklist();
+                                          appVersion, toolkitVersion);
-
+    if (!r) {
-    for each (let blockEntry in this._pluginEntries) {
+      return Ci.nsIBlocklistService.STATE_NOT_BLOCKED;
-      if (matchesAllPluginNames(blockEntry, plugin)) {
-        return blockEntry;
-      }
     }
 
-	  return null;
+    let {entry: blockEntry, version: blockEntryVersion} = r;
+
+    if (blockEntryVersion.severity >= gBlocklistLevel)
+      return Ci.nsIBlocklistService.STATE_BLOCKED;
+    if (blockEntryVersion.severity == SEVERITY_OUTDATED) {
+      let vulnerabilityStatus = blockEntryVersion.vulnerabilityStatus;
+      if (vulnerabilityStatus == VULNERABILITYSTATUS_UPDATE_AVAILABLE)
+        return Ci.nsIBlocklistService.STATE_VULNERABLE_UPDATE_AVAILABLE;
+      if (vulnerabilityStatus == VULNERABILITYSTATUS_NO_UPDATE)
+        return Ci.nsIBlocklistService.STATE_VULNERABLE_NO_UPDATE;
+      return Ci.nsIBlocklistService.STATE_OUTDATED;
+    }
+    return Ci.nsIBlocklistService.STATE_SOFTBLOCKED;
   },
 
   /* See nsIBlocklistService */
   getPluginBlocklistURL: function Blocklist_getPluginBlocklistURL(plugin) {
-    let blockEntry = this._getPluginBlockEntry(plugin);
+    if (!this._isBlocklistLoaded())
-    if (!blockEntry || !blockEntry.blockID) {
+      this._loadBlocklist();
+
+    let r = this._getPluginBlocklistEntry(plugin, this._pluginEntries);
+    if (!r) {
+      return null;
+    }
+    let {entry: blockEntry, version: blockEntryVersion} = r;
+    if (!blockEntry.blockID) {
       return null;
     }
 
@@ -1146,8 +1141,15 @@ Blocklist.prototype = {
 
   /* See nsIBlocklistService */
   getPluginInfoURL: function (plugin) {
-    let blockEntry = this._getPluginBlockEntry(plugin);
+    if (!this._isBlocklistLoaded())
-    if (!blockEntry || !blockEntry.blockID) {
+      this._loadBlocklist();
+
+    let r = this._getPluginBlocklistEntry(plugin, this._pluginEntries);
+    if (!r) {
+      return null;
+    }
+    let {entry: blockEntry, version: blockEntryVersion} = r;
+    if (!blockEntry.blockID) {
       return null;
     }
 

toolkit/mozapps/extensions/test/xpcshell/data/pluginInfoURL_block.xml

@@ -7,15 +7,39 @@
     <pluginItem blockID="test_plugin_wInfoURL">
       <match name="name" exp="^test_with_infoURL"/>
       <match name="version" exp="^5"/>
+      <versionRange>
+        <targetApplication id="xpcshell@tests.mozilla.org">
+          <versionRange minVersion="1" maxVersion="*"/>
+        </targetApplication>
+      </versionRange>
       <infoURL>http://test.url.com/</infoURL>
     </pluginItem>
     <pluginItem blockID="test_plugin_wAltInfoURL">
       <match name="name" exp="^test_with_altInfoURL"/>
       <match name="version" exp="^5"/>
+      <versionRange>
+        <targetApplication id="xpcshell@tests.mozilla.org">
+          <versionRange minVersion="1" maxVersion="*"/>
+        </targetApplication>
+      </versionRange>
       <infoURL>http://alt.test.url.com/</infoURL>
     </pluginItem>
     <pluginItem blockID="test_plugin_noInfoURL">
       <match name="name" exp="^test_no_infoURL"/>
+      <versionRange>
+        <targetApplication id="xpcshell@tests.mozilla.org">
+          <versionRange minVersion="1" maxVersion="*"/>
+        </targetApplication>
+      </versionRange>
+    </pluginItem>
+    <pluginItem blockID="test_plugin_newVersion">
+      <match name="name" exp="^test_newVersion"/>
+      <infoURL>http://test.url2.com/</infoURL>
+      <versionRange minVersion="1" maxVersion="2">
+        <targetApplication id="xpcshell@tests.mozilla.org">
+          <versionRange minVersion="1" maxVersion="*"/>
+        </targetApplication>
+      </versionRange>
     </pluginItem>
   </pluginItems>
 </blocklist>

toolkit/mozapps/extensions/test/xpcshell/test_pluginInfoURL.js

@@ -30,7 +30,9 @@ MockPlugin.prototype = {
 const PLUGINS = [
   new MockPlugin('test_with_infoURL', '5', Ci.nsIPluginTag.STATE_ENABLED),
   new MockPlugin('test_with_altInfoURL', '5', Ci.nsIPluginTag.STATE_ENABLED),
-  new MockPlugin('test_no_infoURL', '5', Ci.nsIPluginTag.STATE_ENABLED)
+  new MockPlugin('test_no_infoURL', '5', Ci.nsIPluginTag.STATE_ENABLED),
+  new MockPlugin('test_newVersion', '1', Ci.nsIPluginTag.STATE_ENABLED),
+  new MockPlugin('test_newVersion', '3', Ci.nsIPluginTag.STATE_ENABLED)
 ];
 
 /**
@@ -78,3 +80,11 @@ add_task(function* test_infoURL_missing() {
   Assert.strictEqual(Services.blocklist.getPluginInfoURL(PLUGINS[2]), null,
     'Should be null when no infoURL tag is available.');
 });
+
+add_task(function* test_intoURL_newVersion() {
+  let testInfoURL = 'http://test.url2.com/';
+  Assert.strictEqual(Services.blocklist.getPluginInfoURL(PLUGINS[3]),
+    testInfoURL, 'Old plugin should match');
+  Assert.strictEqual(Services.blocklist.getPluginInfoURL(PLUGINS[4]),
+    null, 'New plugin should not match');
+});