Bug 1245184 - nsICookieManager.remove should use the OriginAttributes, r=sicking
authorAndrea Marchesini <amarchesini@mozilla.com>
Thu, 25 Feb 2016 16:41:13 +0100
changeset 285599 ab3a305b3e4147cf8ea728ceebf218dcea36053f
parent 285598 c815269c99c8e79455d0258ffb08fb7b480caba7
child 285600 ff0797af7e1ef53172282d6ba36672596549136b
push id30033
push userkwierso@gmail.com
push dateFri, 26 Feb 2016 19:21:05 +0000
treeherdermozilla-central@07438c9bfc83 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking
bugs1245184
milestone47.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 1245184 - nsICookieManager.remove should use the OriginAttributes, r=sicking
browser/base/content/sanitize.js
browser/components/preferences/cookies.js
dom/base/ChromeUtils.cpp
dom/base/ChromeUtils.h
dom/webidl/ChromeUtils.webidl
netwerk/cookie/nsCookieService.cpp
netwerk/cookie/nsICookieManager.idl
netwerk/test/TestCookie.cpp
netwerk/test/moz.build
--- a/browser/base/content/sanitize.js
+++ b/browser/base/content/sanitize.js
@@ -237,17 +237,18 @@ Sanitizer.prototype = {
           if (range) {
             // Iterate through the cookies and delete any created after our cutoff.
             let cookiesEnum = cookieMgr.enumerator;
             while (cookiesEnum.hasMoreElements()) {
               let cookie = cookiesEnum.getNext().QueryInterface(Ci.nsICookie2);
 
               if (cookie.creationTime > range[0]) {
                 // This cookie was created after our cutoff, clear it
-                cookieMgr.remove(cookie.host, cookie.name, cookie.path, false);
+                cookieMgr.remove(cookie.host, cookie.name, cookie.path,
+                                 cookie.originAttributes, false);
 
                 if (++yieldCounter % YIELD_PERIOD == 0) {
                   yield new Promise(resolve => setTimeout(resolve, 0)); // Don't block the main thread too long
                 }
               }
             }
           }
           else {
--- a/browser/components/preferences/cookies.js
+++ b/browser/components/preferences/cookies.js
@@ -66,17 +66,19 @@ var gCookiesWindow = {
     this._updateRemoveAllButton();
 
     this._saveState();
   },
 
   _cookieEquals: function (aCookieA, aCookieB, aStrippedHost) {
     return aCookieA.rawHost == aStrippedHost &&
            aCookieA.name == aCookieB.name &&
-           aCookieA.path == aCookieB.path;
+           aCookieA.path == aCookieB.path &&
+           ChromeUtils.isOriginAttributesEqual(aCookieA.originAttributes,
+                                               aCookieB.originAttributes);
   },
 
   observe: function (aCookie, aTopic, aData) {
     if (aTopic != "cookie-changed")
       return;
 
     if (aCookie instanceof Components.interfaces.nsICookie) {
       var strippedHost = this._makeStrippedHost(aCookie.host);
@@ -271,25 +273,29 @@ var gCookiesWindow = {
         }
         this._filterSet.splice(aIndex, removeCount);
         return;
       }
 
       var item = this._getItemAtIndex(aIndex);
       if (!item) return;
       this._invalidateCache(aIndex - 1);
-      if (item.container)
+      if (item.container) {
         gCookiesWindow._hosts[item.rawHost] = null;
-      else {
+      } else {
         var parent = this._getItemAtIndex(item.parentIndex);
         for (var i = 0; i < parent.cookies.length; ++i) {
           var cookie = parent.cookies[i];
           if (item.rawHost == cookie.rawHost &&
-              item.name == cookie.name && item.path == cookie.path)
+              item.name == cookie.name &&
+              item.path == cookie.path &&
+              ChromeUtils.isOriginAttributesEqual(item.originAttributes,
+                                                  cookie.originAttributes)) {
             parent.cookies.splice(i, removeCount);
+          }
         }
       }
     },
 
     _invalidateCache: function (aIndex) {
       this._cacheValid = Math.min(this._cacheValid, aIndex);
     },
 
@@ -579,17 +585,18 @@ var gCookiesWindow = {
   performDeletion: function gCookiesWindow_performDeletion(deleteItems) {
     var psvc = Components.classes["@mozilla.org/preferences-service;1"]
                          .getService(Components.interfaces.nsIPrefBranch);
     var blockFutureCookies = false;
     if (psvc.prefHasUserValue("network.cookie.blockFutureCookies"))
       blockFutureCookies = psvc.getBoolPref("network.cookie.blockFutureCookies");
     for (var i = 0; i < deleteItems.length; ++i) {
       var item = deleteItems[i];
-      this._cm.remove(item.host, item.name, item.path, blockFutureCookies);
+      this._cm.remove(item.host, item.name, item.path,
+                      item.originAttributes, blockFutureCookies);
     }
   },
 
   deleteCookie: function () {
     // Selection Notes
     // - Selection always moves to *NEXT* adjacent item unless item
     //   is last child at a given level in which case it moves to *PREVIOUS*
     //   item
--- a/dom/base/ChromeUtils.cpp
+++ b/dom/base/ChromeUtils.cpp
@@ -83,10 +83,22 @@ ChromeUtils::CreateOriginAttributesWithU
     aRv.Throw(NS_ERROR_FAILURE);
     return;
   }
 
   attrs.mUserContextId = aUserContextId;
   aAttrs = attrs;
 }
 
+/* static */ bool
+ChromeUtils::IsOriginAttributesEqual(dom::GlobalObject& aGlobal,
+                                     const dom::OriginAttributesDictionary& aA,
+                                     const dom::OriginAttributesDictionary& aB)
+{
+  return aA.mAddonId == aB.mAddonId &&
+         aA.mAppId == aB.mAppId &&
+         aA.mInBrowser == aB.mInBrowser &&
+         aA.mSignedPkg == aB.mSignedPkg &&
+         aA.mUserContextId == aB.mUserContextId;
+}
+
 } // namespace dom
 } // namespace mozilla
--- a/dom/base/ChromeUtils.h
+++ b/dom/base/ChromeUtils.h
@@ -44,29 +44,34 @@ public:
                                              JS::MutableHandle<JS::Value> aRetval,
                                              ErrorResult& aRv);
 };
 
 class ChromeUtils : public ThreadSafeChromeUtils
 {
 public:
   static void
-  OriginAttributesToSuffix(dom::GlobalObject& aGlobal,
+  OriginAttributesToSuffix(GlobalObject& aGlobal,
                            const dom::OriginAttributesDictionary& aAttrs,
                            nsCString& aSuffix);
 
   static bool
   OriginAttributesMatchPattern(dom::GlobalObject& aGlobal,
                                const dom::OriginAttributesDictionary& aAttrs,
                                const dom::OriginAttributesPatternDictionary& aPattern);
 
   static void
   CreateOriginAttributesWithUserContextId(dom::GlobalObject& aGlobal,
                                           const nsAString& aOrigin,
                                           uint32_t aUserContextId,
                                           dom::OriginAttributesDictionary& aAttrs,
                                           ErrorResult& aRv);
+
+  static bool
+  IsOriginAttributesEqual(dom::GlobalObject& aGlobal,
+                          const dom::OriginAttributesDictionary& aA,
+                          const dom::OriginAttributesDictionary& aB);
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_ChromeUtils__
--- a/dom/webidl/ChromeUtils.webidl
+++ b/dom/webidl/ChromeUtils.webidl
@@ -32,16 +32,23 @@ interface ChromeUtils : ThreadSafeChrome
   /**
    * Returns an OriginAttributes dictionary using the origin URI but forcing
    * the passed userContextId.
    */
   [Throws]
   static OriginAttributesDictionary
   createOriginAttributesWithUserContextId(DOMString origin,
                                           unsigned long userContextId);
+
+  /**
+   * Returns true if the 2 OriginAttributes are equal.
+   */
+  static boolean
+  isOriginAttributesEqual(optional OriginAttributesDictionary aA,
+                          optional OriginAttributesDictionary aB);
 };
 
 /**
  * Used by principals and the script security manager to represent origin
  * attributes. The first dictionary is designed to contain the full set of
  * OriginAttributes, the second is used for pattern-matching (i.e. does this
  * OriginAttributesDictionary match the non-empty attributes in this pattern).
  *
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -2361,19 +2361,25 @@ nsCookieService::Remove(const nsACString
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsCookieService::Remove(const nsACString &aHost,
                         const nsACString &aName,
                         const nsACString &aPath,
-                        bool             aBlocked)
+                        JS::HandleValue  aOriginAttributes,
+                        bool             aBlocked,
+                        JSContext*       aCx)
 {
   NeckoOriginAttributes attrs;
+  if (!attrs.Init(aCx, aOriginAttributes)) {
+    return NS_ERROR_FAILURE;
+  }
+
   return Remove(aHost, attrs, aName, aPath, aBlocked);
 }
 
 /******************************************************************************
  * nsCookieService impl:
  * private file I/O functions
  ******************************************************************************/
 
--- a/netwerk/cookie/nsICookieManager.idl
+++ b/netwerk/cookie/nsICookieManager.idl
@@ -34,16 +34,19 @@ interface nsICookieManager : nsISupports
    * directly from the desired nsICookie object.
    *
    * @param aHost The host or domain for which the cookie was set. @see
    *              nsICookieManager2::add for a description of acceptable host
    *              strings. If the target cookie is a domain cookie, a leading
    *              dot must be present.
    * @param aName The name specified in the cookie
    * @param aPath The path for which the cookie was set
+   * @param aOriginAttributes The originAttributes of this cookie
    * @param aBlocked Indicates if cookies from this host should be permanently blocked
    *
    */
-  void remove(in AUTF8String aHost,
-              in ACString    aName,
-              in AUTF8String aPath,
-              in boolean     aBlocked);
+  [implicit_jscontext]
+  void remove(in AUTF8String   aHost,
+              in ACString      aName,
+              in AUTF8String   aPath,
+              in jsval         aOriginAttributes,
+              in boolean       aBlocked);
 };
--- a/netwerk/test/TestCookie.cpp
+++ b/netwerk/test/TestCookie.cpp
@@ -14,16 +14,17 @@
 #include "prprf.h"
 #include "nsNetUtil.h"
 #include "nsISimpleEnumerator.h"
 #include "nsServiceManagerUtils.h"
 #include "nsNetCID.h"
 #include "nsStringAPI.h"
 #include "nsIPrefBranch.h"
 #include "nsIPrefService.h"
+#include "js/Value.h"
 
 static NS_DEFINE_CID(kCookieServiceCID, NS_COOKIESERVICE_CID);
 static NS_DEFINE_CID(kPrefServiceCID,   NS_PREFSERVICE_CID);
 
 // various pref strings
 static const char kCookiesPermissions[] = "network.cookie.cookieBehavior";
 static const char kCookiesLifetimeEnabled[] = "network.cookie.lifetime.enabled";
 static const char kCookiesLifetimeDays[] = "network.cookie.lifetime.days";
@@ -670,17 +671,19 @@ main(int32_t argc, char *argv[])
               hostCookies == 2;
       // check CookieExists() using the third cookie
       bool found;
       rv[9] = NS_SUCCEEDED(cookieMgr2->CookieExists(newDomainCookie, &found)) && found;
       // remove the cookie, block it, and ensure it can't be added again
       rv[10] = NS_SUCCEEDED(cookieMgr->Remove(NS_LITERAL_CSTRING("new.domain"), // domain
                                               NS_LITERAL_CSTRING("test3"),      // name
                                               NS_LITERAL_CSTRING("/rabbit"),    // path
-                                              true));                        // is blocked
+                                              JS::NullHandleValue,              // originAttributes
+                                              true,                             // is blocked
+                                              nullptr));                        // JSContext
       rv[11] = NS_SUCCEEDED(cookieMgr2->CookieExists(newDomainCookie, &found)) && !found;
       rv[12] = NS_SUCCEEDED(cookieMgr2->Add(NS_LITERAL_CSTRING("new.domain"),     // domain
                                             NS_LITERAL_CSTRING("/rabbit"),        // path
                                             NS_LITERAL_CSTRING("test3"),          // name
                                             NS_LITERAL_CSTRING("yes"),            // value
                                             false,                             // is secure
                                             false,                             // is httponly
                                             true,                              // is session
--- a/netwerk/test/moz.build
+++ b/netwerk/test/moz.build
@@ -47,9 +47,11 @@ CppUnitTests([
     'TestUDPSocket',
 ])
 
 RESOURCE_FILES += [
     'urlparse.dat',
     'urlparse_unx.dat',
 ]
 
+USE_LIBS += ['static:js']
+
 CXXFLAGS += CONFIG['TK_CFLAGS']