Bug 816956 - Don't try to check all parent hosts in the Permission Manager (but only until the domain name). r=sicking
authorMounir Lamouri <mounir.lamouri@gmail.com>
Mon, 04 Feb 2013 23:41:52 +0000
changeset 130692 30b0bb7004aaade63d1777d097ddd0a98a2f66e7
parent 130691 cf0e172390b72fa0b3d89264e93b1f4b763c458d
child 130693 cfd082020dfa25856016e019b4ca68fe9ae985ee
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking
bugs816956
milestone21.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 816956 - Don't try to check all parent hosts in the Permission Manager (but only until the domain name). r=sicking
extensions/cookie/nsPermissionManager.cpp
extensions/cookie/test/unit/test_permmanager_subdomains.js
extensions/cookie/test/unit/xpcshell.ini
--- a/extensions/cookie/nsPermissionManager.cpp
+++ b/extensions/cookie/nsPermissionManager.cpp
@@ -24,16 +24,17 @@
 #include "mozilla/storage.h"
 #include "mozilla/Attributes.h"
 #include "nsXULAppAPI.h"
 #include "nsIPrincipal.h"
 #include "nsContentUtils.h"
 #include "nsIScriptSecurityManager.h"
 #include "nsIAppsService.h"
 #include "mozIApplication.h"
+#include "nsIEffectiveTLDService.h"
 
 static nsPermissionManager *gPermissionManager = nullptr;
 
 using mozilla::dom::ContentParent;
 using mozilla::dom::ContentChild;
 using mozilla::unused; // ha!
 
 static bool
@@ -121,16 +122,37 @@ GetHostForPrincipal(nsIPrincipal* aPrinc
   rv = uri->GetAsciiHost(aHost);
   if (NS_FAILED(rv) || aHost.IsEmpty()) {
     return NS_ERROR_UNEXPECTED;
   }
 
   return NS_OK;
 }
 
+nsCString
+GetNextSubDomainForHost(const nsACString& aHost)
+{
+  nsCOMPtr<nsIEffectiveTLDService> tldService =
+    do_GetService(NS_EFFECTIVETLDSERVICE_CONTRACTID);
+  if (!tldService) {
+    NS_ERROR("Should have a tld service!");
+    return EmptyCString();
+  }
+
+  nsCString subDomain;
+  nsresult rv = tldService->GetNextSubDomain(aHost, subDomain);
+  // We can fail if there is no more subdomain or if the host can't have a
+  // subdomain.
+  if (NS_FAILED(rv)) {
+    return EmptyCString();
+  }
+
+  return subDomain;
+}
+
 class AppClearDataObserver MOZ_FINAL : public nsIObserver {
 public:
   NS_DECL_ISUPPORTS
 
   // nsIObserver implementation.
   NS_IMETHODIMP
   Observe(nsISupports *aSubject, const char *aTopic, const PRUnichar *data)
   {
@@ -1054,51 +1076,46 @@ nsPermissionManager::CommonTestPermissio
 // lookup as the string doesn't contain any dots.
 nsPermissionManager::PermissionHashKey*
 nsPermissionManager::GetPermissionHashKey(const nsACString& aHost,
                                           uint32_t aAppId,
                                           bool aIsInBrowserElement,
                                           uint32_t aType,
                                           bool aExactHostMatch)
 {
-  uint32_t offset = 0;
-  PermissionHashKey* entry;
-  int64_t now = PR_Now() / 1000;
+  PermissionHashKey* entry = nullptr;
 
-  do {
-    nsRefPtr<PermissionKey> key = new PermissionKey(Substring(aHost, offset), aAppId, aIsInBrowserElement);
-    entry = mPermissionTable.GetEntry(key);
-
-    if (entry) {
-      PermissionEntry permEntry = entry->GetPermission(aType);
+  nsRefPtr<PermissionKey> key = new PermissionKey(aHost, aAppId, aIsInBrowserElement);
+  entry = mPermissionTable.GetEntry(key);
 
-      // if the entry is expired, remove and keep looking for others.
-      if (permEntry.mExpireType == nsIPermissionManager::EXPIRE_TIME &&
-          permEntry.mExpireTime <= now) {
-        nsCOMPtr<nsIPrincipal> principal;
-        if (NS_FAILED(GetPrincipal(aHost, aAppId, aIsInBrowserElement, getter_AddRefs(principal)))) {
-          return nullptr;
-        }
+  if (entry) {
+    PermissionEntry permEntry = entry->GetPermission(aType);
 
-        RemoveFromPrincipal(principal, mTypeArray[aType].get());
-      } else if (permEntry.mPermission != nsIPermissionManager::UNKNOWN_ACTION) {
-        break;
+    // if the entry is expired, remove and keep looking for others.
+    if (permEntry.mExpireType == nsIPermissionManager::EXPIRE_TIME &&
+        permEntry.mExpireTime <= (PR_Now() / 1000)) {
+      nsCOMPtr<nsIPrincipal> principal;
+      if (NS_FAILED(GetPrincipal(aHost, aAppId, aIsInBrowserElement, getter_AddRefs(principal)))) {
+        return nullptr;
       }
 
-      // reset entry, to be able to return null on failure
+      entry = nullptr;
+      RemoveFromPrincipal(principal, mTypeArray[aType].get());
+    } else if (permEntry.mPermission == nsIPermissionManager::UNKNOWN_ACTION) {
       entry = nullptr;
     }
-    if (aExactHostMatch)
-      break; // do not try super domains
+  }
 
-    offset = aHost.FindChar('.', offset) + 1;
+  if (!entry && !aExactHostMatch) {
+    nsCString domain = GetNextSubDomainForHost(aHost);
+    if (!domain.IsEmpty()) {
+      return GetPermissionHashKey(domain, aAppId, aIsInBrowserElement, aType, aExactHostMatch);
+    }
+  }
 
-  // walk up the domaintree (we stop as soon as we find a match,
-  // which will be the most specific domain we have an entry for).
-  } while (offset > 0);
   return entry;
 }
 
 // helper struct for passing arguments into hash enumeration callback.
 struct nsGetEnumeratorData
 {
   nsGetEnumeratorData(nsCOMArray<nsIPermission> *aArray, const nsTArray<nsCString> *aTypes)
    : array(aArray)
new file mode 100644
--- /dev/null
+++ b/extensions/cookie/test/unit/test_permmanager_subdomains.js
@@ -0,0 +1,56 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+function getPrincipalFromURI(uri) {
+  return Cc["@mozilla.org/scriptsecuritymanager;1"]
+           .getService(Ci.nsIScriptSecurityManager)
+           .getNoAppCodebasePrincipal(NetUtil.newURI(uri));
+}
+
+function run_test() {
+  var pm = Cc["@mozilla.org/permissionmanager;1"].
+           getService(Ci.nsIPermissionManager);
+
+  // Adds a permission to a sub-domain. Checks if it is working.
+  let sub1Principal = getPrincipalFromURI("http://sub1.example.com");
+  pm.addFromPrincipal(sub1Principal, "test/subdomains", pm.ALLOW_ACTION, 0, 0);
+  do_check_eq(pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"), pm.ALLOW_ACTION);
+
+  // A sub-sub-domain should get the permission.
+  let subsubPrincipal = getPrincipalFromURI("http://sub.sub1.example.com");
+  do_check_eq(pm.testPermissionFromPrincipal(subsubPrincipal, "test/subdomains"), pm.ALLOW_ACTION);
+
+  // Another sub-domain shouldn't get the permission.
+  let sub2Principal = getPrincipalFromURI("http://sub2.example.com");
+  do_check_eq(pm.testPermissionFromPrincipal(sub2Principal, "test/subdomains"), pm.UNKNOWN_ACTION);
+
+  // Remove current permissions.
+  pm.removeFromPrincipal(sub1Principal, "test/subdomains");
+  do_check_eq(pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"), pm.UNKNOWN_ACTION);
+
+  // Adding the permission to the main domain. Checks if it is working.
+  let mainPrincipal = getPrincipalFromURI("http://example.com");
+  pm.addFromPrincipal(mainPrincipal, "test/subdomains", pm.ALLOW_ACTION, 0, 0);
+  do_check_eq(pm.testPermissionFromPrincipal(mainPrincipal, "test/subdomains"), pm.ALLOW_ACTION);
+
+  // All sub-domains should have the permission now.
+  do_check_eq(pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"), pm.ALLOW_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(sub2Principal, "test/subdomains"), pm.ALLOW_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(subsubPrincipal, "test/subdomains"), pm.ALLOW_ACTION);
+
+  // Remove current permissions.
+  pm.removeFromPrincipal(mainPrincipal, "test/subdomains");
+  do_check_eq(pm.testPermissionFromPrincipal(mainPrincipal, "test/subdomains"), pm.UNKNOWN_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"), pm.UNKNOWN_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(sub2Principal, "test/subdomains"), pm.UNKNOWN_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(subsubPrincipal, "test/subdomains"), pm.UNKNOWN_ACTION);
+
+  // A sanity check that the previous implementation wasn't passing...
+  let crazyPrincipal = getPrincipalFromURI("http://com");
+  pm.addFromPrincipal(crazyPrincipal, "test/subdomains", pm.ALLOW_ACTION, 0, 0);
+  do_check_eq(pm.testPermissionFromPrincipal(crazyPrincipal, "test/subdomains"),  pm.ALLOW_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(mainPrincipal, "test/subdomains"),   pm.UNKNOWN_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"),   pm.UNKNOWN_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(sub2Principal, "test/subdomains"),   pm.UNKNOWN_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(subsubPrincipal, "test/subdomains"), pm.UNKNOWN_ACTION);
+}
\ No newline at end of file
--- a/extensions/cookie/test/unit/xpcshell.ini
+++ b/extensions/cookie/test/unit/xpcshell.ini
@@ -16,11 +16,12 @@ tail =
 [test_domain_eviction.js]
 [test_eviction.js]
 [test_permmanager_expiration.js]
 [test_permmanager_notifications.js]
 [test_permmanager_removeall.js]
 [test_permmanager_load_invalid_entries.js]
 skip-if = debug == true
 [test_permmanager_idn.js]
+[test_permmanager_subdomains.js]
 [test_permmanager_cleardata.js]
 [test_schema_2_migration.js]
 [test_schema_3_migration.js]