Bug 874197 - Change nsIPermissionManager.TYPE_SESSION to also respect expiration times if they are specified. r=jdm sr=mounir

extensions/cookie/nsPermissionManager.cpp

@@ -618,8 +618,10 @@ nsPermissionManager::AddFromPrincipal(nsIPrincipal* aPrincipal,
                  aExpireType == nsIPermissionManager::EXPIRE_SESSION,
                  NS_ERROR_INVALID_ARG);
 
-  // Skip addition if the permission is already expired.
-  if (aExpireType == nsIPermissionManager::EXPIRE_TIME &&
+  // Skip addition if the permission is already expired. Note that EXPIRE_SESSION only
+  // honors expireTime if it is nonzero.
+  if ((aExpireType == nsIPermissionManager::EXPIRE_TIME ||
+       (aExpireType == nsIPermissionManager::EXPIRE_SESSION && aExpireTime != 0)) &&
       aExpireTime <= (PR_Now() / 1000)) {
     return NS_OK;
   }
@@ -702,7 +704,7 @@ nsPermissionManager::AddInternal(nsIPrincipal* aPrincipal,
     // only thing changed is the expire time.
     if (aPermission == oldPermissionEntry.mPermission &&
         aExpireType == oldPermissionEntry.mExpireType &&
-        (aExpireType != nsIPermissionManager::EXPIRE_TIME ||
+        (aExpireType == nsIPermissionManager::EXPIRE_NEVER ||
          aExpireTime == oldPermissionEntry.mExpireTime))
       op = eOperationNone;
     else if (aPermission == nsIPermissionManager::UNKNOWN_ACTION)
@@ -801,14 +803,16 @@ nsPermissionManager::AddInternal(nsIPrincipal* aPrincipal,
           aExpireType == nsIPermissionManager::EXPIRE_SESSION) {
         entry->GetPermissions()[index].mNonSessionPermission = entry->GetPermissions()[index].mPermission;
         entry->GetPermissions()[index].mNonSessionExpireType = entry->GetPermissions()[index].mExpireType;
+        entry->GetPermissions()[index].mNonSessionExpireTime = entry->GetPermissions()[index].mExpireTime;
       } else if (aExpireType != nsIPermissionManager::EXPIRE_SESSION) {
         entry->GetPermissions()[index].mNonSessionPermission = aPermission;
         entry->GetPermissions()[index].mNonSessionExpireType = aExpireType;
-        entry->GetPermissions()[index].mExpireTime = aExpireTime;
+        entry->GetPermissions()[index].mNonSessionExpireTime = aExpireTime;
       }
 
       entry->GetPermissions()[index].mPermission = aPermission;
       entry->GetPermissions()[index].mExpireType = aExpireType;
+      entry->GetPermissions()[index].mExpireTime = aExpireTime;
 
       if (aDBOperation == eWriteToDB && aExpireType != nsIPermissionManager::EXPIRE_SESSION)
         // We care only about the id, the permission and expireType/expireTime here.
@@ -1143,7 +1147,10 @@ nsPermissionManager::GetPermissionHashKey(const nsACString& aHost,
     PermissionEntry permEntry = entry->GetPermission(aType);
 
     // if the entry is expired, remove and keep looking for others.
-    if (permEntry.mExpireType == nsIPermissionManager::EXPIRE_TIME &&
+    // Note that EXPIRE_SESSION only honors expireTime if it is nonzero.
+    if ((permEntry.mExpireType == nsIPermissionManager::EXPIRE_TIME ||
+         (permEntry.mExpireType == nsIPermissionManager::EXPIRE_SESSION &&
+          permEntry.mExpireTime != 0)) &&
         permEntry.mExpireTime <= (PR_Now() / 1000)) {
       nsCOMPtr<nsIPrincipal> principal;
       if (NS_FAILED(GetPrincipal(aHost, aAppId, aIsInBrowserElement, getter_AddRefs(principal)))) {
@@ -1375,6 +1382,7 @@ nsPermissionManager::RemoveExpiredPermissionsForAppEnumerator(
 
     permEntry.mPermission = permEntry.mNonSessionPermission;
     permEntry.mExpireType = permEntry.mNonSessionExpireType;
+    permEntry.mExpireTime = permEntry.mNonSessionExpireTime;
 
     gPermissionManager->NotifyObserversWithPermission(entry->GetKey()->mHost,
                                                       entry->GetKey()->mAppId,

extensions/cookie/nsPermissionManager.h

@@ -1,4 +1,4 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
@@ -45,6 +45,7 @@ public:
      , mExpireTime(aExpireTime)
      , mNonSessionPermission(aPermission)
      , mNonSessionExpireType(aExpireType)
+     , mNonSessionExpireTime(aExpireTime)
     {}
 
     int64_t  mID;
@@ -54,6 +55,7 @@ public:
     int64_t  mExpireTime;
     uint32_t mNonSessionPermission;
     uint32_t mNonSessionExpireType;
+    uint32_t mNonSessionExpireTime;
   };
 
   /**

extensions/cookie/test/unit/test_permmanager_expiration.js

@@ -27,29 +27,41 @@ function do_run_test() {
 
   // add a permission with *now* expiration
   pm.addFromPrincipal(principal, "test/expiration-perm-exp", 1, pm.EXPIRE_TIME, now);
+  pm.addFromPrincipal(principal, "test/expiration-session-exp", 1, pm.EXPIRE_SESSION, now);
 
   // add a permission with future expiration (100 milliseconds)
   pm.addFromPrincipal(principal, "test/expiration-perm-exp2", 1, pm.EXPIRE_TIME, now + 100);
+  pm.addFromPrincipal(principal, "test/expiration-session-exp2", 1, pm.EXPIRE_SESSION, now + 100);
 
   // add a permission with future expiration (1000 seconds)
   pm.addFromPrincipal(principal, "test/expiration-perm-exp3", 1, pm.EXPIRE_TIME, now + 1e6);
+  pm.addFromPrincipal(principal, "test/expiration-session-exp3", 1, pm.EXPIRE_SESSION, now + 1e6);
 
   // add a permission without expiration
   pm.addFromPrincipal(principal, "test/expiration-perm-nexp", 1, pm.EXPIRE_NEVER, 0);
 
   // check that the second two haven't expired yet
   do_check_eq(1, pm.testPermissionFromPrincipal(principal, "test/expiration-perm-exp3"));
+  do_check_eq(1, pm.testPermissionFromPrincipal(principal, "test/expiration-session-exp3"));
   do_check_eq(1, pm.testPermissionFromPrincipal(principal, "test/expiration-perm-nexp"));
 
   // ... and the first one has
   do_timeout(10, continue_test);
   yield;
   do_check_eq(0, pm.testPermissionFromPrincipal(principal, "test/expiration-perm-exp"));
+  do_check_eq(0, pm.testPermissionFromPrincipal(principal, "test/expiration-session-exp"));
 
   // ... and that the short-term one will
   do_timeout(200, continue_test);
   yield;
-  do_check_eq(0, pm.testPermissionFromPrincipal(principal, "test/expiration-perm-exp2")); 
+  do_check_eq(0, pm.testPermissionFromPrincipal(principal, "test/expiration-perm-exp2"));
+  do_check_eq(0, pm.testPermissionFromPrincipal(principal, "test/expiration-session-exp2"));
+
+  // Check that .getPermission returns a matching result
+  do_check_null(pm.getPermissionObject(principal, "test/expiration-perm-exp", false));
+  do_check_null(pm.getPermissionObject(principal, "test/expiration-session-exp", false));
+  do_check_null(pm.getPermissionObject(principal, "test/expiration-perm-exp2", false));
+  do_check_null(pm.getPermissionObject(principal, "test/expiration-session-exp2", false));
 
   do_finish_generator_test(test_generator);
 }

netwerk/base/public/nsIPermissionManager.idl

@@ -55,7 +55,8 @@ interface nsIPermissionManager : nsISupports
   /**
    * Predefined expiration types for permissions.  Permissions can be permanent
    * (never expire), expire at the end of the session, or expire at a specified
-   * time.
+   * time. Permissions that expire at the end of a session may also have a
+   * specified expiration time.
    */
   const uint32_t EXPIRE_NEVER = 0;
   const uint32_t EXPIRE_SESSION = 1;