Backed out changeset 7d5f1e9ce04c (bug 1259169) for various cookie-related crashes and test failures.

This commit is contained in:
Ryan VanderMeulen 2016-04-09 15:36:07 -04:00
parent 2c139014ed
commit 1c7872d0af
19 changed files with 40 additions and 62 deletions

View File

@ -249,7 +249,7 @@ Sanitizer.prototype = {
if (cookie.creationTime > range[0]) { if (cookie.creationTime > range[0]) {
// This cookie was created after our cutoff, clear it // This cookie was created after our cutoff, clear it
cookieMgr.remove(cookie.host, cookie.name, cookie.path, cookieMgr.remove(cookie.host, cookie.name, cookie.path,
false, cookie.originAttributes); cookie.originAttributes, false);
if (++yieldCounter % YIELD_PERIOD == 0) { if (++yieldCounter % YIELD_PERIOD == 0) {
yield new Promise(resolve => setTimeout(resolve, 0)); // Don't block the main thread too long yield new Promise(resolve => setTimeout(resolve, 0)); // Don't block the main thread too long

View File

@ -623,7 +623,7 @@ Cookies.prototype = {
// a domain cookie. See bug 222343. // a domain cookie. See bug 222343.
if (host.length > 0) { if (host.length > 0) {
// Fist delete any possible extant matching host cookie. // Fist delete any possible extant matching host cookie.
Services.cookies.remove(host, name, path, false, {}); Services.cookies.remove(host, name, path, {}, false);
// Now make it a domain cookie. // Now make it a domain cookie.
if (host[0] != "." && !hostIsIPAddress(host)) if (host[0] != "." && !hostIsIPAddress(host))
host = "." + host; host = "." + host;

View File

@ -591,7 +591,7 @@ var gCookiesWindow = {
for (var i = 0; i < deleteItems.length; ++i) { for (var i = 0; i < deleteItems.length; ++i) {
var item = deleteItems[i]; var item = deleteItems[i];
this._cm.remove(item.host, item.name, item.path, this._cm.remove(item.host, item.name, item.path,
blockFutureCookies, item.originAttributes); item.originAttributes, blockFutureCookies);
} }
}, },

View File

@ -894,8 +894,8 @@ var cookieHelpers = {
cookie.host, cookie.host,
cookie.name, cookie.name,
cookie.path, cookie.path,
false, cookie.originAttributes,
cookie.originAttributes false
); );
} }
} }

View File

@ -133,7 +133,7 @@ exports.items = [
if (isCookieAtHost(cookie, host)) { if (isCookieAtHost(cookie, host)) {
if (cookie.name == args.name) { if (cookie.name == args.name) {
cookieMgr.remove(cookie.host, cookie.name, cookie.path, cookieMgr.remove(cookie.host, cookie.name, cookie.path,
false, cookie.originAttributes); cookie.originAttributes, false);
} }
} }
} }

View File

@ -28,9 +28,9 @@ function run_test() {
do_check_throws(function() { do_check_throws(function() {
cm.countCookiesFromHost("..baz.com"); cm.countCookiesFromHost("..baz.com");
}, Cr.NS_ERROR_ILLEGAL_VALUE); }, Cr.NS_ERROR_ILLEGAL_VALUE);
cm.remove("BAZ.com.", "foo", "/", false, {}); cm.remove("BAZ.com.", "foo", "/", {}, false);
do_check_eq(cm.countCookiesFromHost("baz.com"), 1); do_check_eq(cm.countCookiesFromHost("baz.com"), 1);
cm.remove("baz.com", "foo", "/", false, {}); cm.remove("baz.com", "foo", "/", {}, false);
do_check_eq(cm.countCookiesFromHost("baz.com"), 0); do_check_eq(cm.countCookiesFromHost("baz.com"), 0);
// Test that 'baz.com' and 'baz.com.' are treated differently // Test that 'baz.com' and 'baz.com.' are treated differently
@ -40,9 +40,9 @@ function run_test() {
do_check_eq(cm.countCookiesFromHost(".baz.com"), 0); do_check_eq(cm.countCookiesFromHost(".baz.com"), 0);
do_check_eq(cm.countCookiesFromHost("baz.com."), 1); do_check_eq(cm.countCookiesFromHost("baz.com."), 1);
do_check_eq(cm.countCookiesFromHost(".baz.com."), 1); do_check_eq(cm.countCookiesFromHost(".baz.com."), 1);
cm.remove("baz.com", "foo", "/", false, {}); cm.remove("baz.com", "foo", "/", {}, false);
do_check_eq(cm.countCookiesFromHost("baz.com."), 1); do_check_eq(cm.countCookiesFromHost("baz.com."), 1);
cm.remove("baz.com.", "foo", "/", false, {}); cm.remove("baz.com.", "foo", "/", {}, false);
do_check_eq(cm.countCookiesFromHost("baz.com."), 0); do_check_eq(cm.countCookiesFromHost("baz.com."), 0);
// test that domain cookies are illegal for IP addresses, aliases such as // test that domain cookies are illegal for IP addresses, aliases such as
@ -174,10 +174,10 @@ function run_test() {
}, Cr.NS_ERROR_ILLEGAL_VALUE); }, Cr.NS_ERROR_ILLEGAL_VALUE);
do_check_eq(getCookieCount(), 1); do_check_eq(getCookieCount(), 1);
cm.remove("", "foo2", "/", false, {}); cm.remove("", "foo2", "/", {}, false);
do_check_eq(getCookieCount(), 0); do_check_eq(getCookieCount(), 0);
do_check_throws(function() { do_check_throws(function() {
cm.remove(".", "foo3", "/", false, {}); cm.remove(".", "foo3", "/", {}, false);
}, Cr.NS_ERROR_ILLEGAL_VALUE); }, Cr.NS_ERROR_ILLEGAL_VALUE);
// test that the 'domain' attribute accepts a leading dot for IP addresses, // test that the 'domain' attribute accepts a leading dot for IP addresses,

View File

@ -58,7 +58,7 @@ function do_run_test() {
}, Cr.NS_ERROR_NOT_AVAILABLE); }, Cr.NS_ERROR_NOT_AVAILABLE);
do_check_throws(function() { do_check_throws(function() {
Services.cookiemgr.remove("foo.com", "", "oh4", false, {}); Services.cookiemgr.remove("foo.com", "", "oh4", {}, false);
}, Cr.NS_ERROR_NOT_AVAILABLE); }, Cr.NS_ERROR_NOT_AVAILABLE);
do_check_throws(function() { do_check_throws(function() {

View File

@ -86,11 +86,11 @@ function do_run_test() {
// remove some of the cookies, in both reverse and forward order // remove some of the cookies, in both reverse and forward order
for (let i = 100; i-- > 0; ) { for (let i = 100; i-- > 0; ) {
let host = i.toString() + ".com"; let host = i.toString() + ".com";
Services.cookiemgr.remove(host, "oh", "/", false, {}); Services.cookiemgr.remove(host, "oh", "/", {}, false);
} }
for (let i = CMAX - 100; i < CMAX; ++i) { for (let i = CMAX - 100; i < CMAX; ++i) {
let host = i.toString() + ".com"; let host = i.toString() + ".com";
Services.cookiemgr.remove(host, "oh", "/", false, {}); Services.cookiemgr.remove(host, "oh", "/", {}, false);
} }
// check the count // check the count

View File

@ -12,13 +12,11 @@
#include "mozilla/net/NeckoCommon.h" #include "mozilla/net/NeckoCommon.h"
#include "nsCookieService.h" #include "nsCookieService.h"
#include "nsContentUtils.h"
#include "nsIServiceManager.h" #include "nsIServiceManager.h"
#include "nsIIOService.h" #include "nsIIOService.h"
#include "nsIPrefBranch.h" #include "nsIPrefBranch.h"
#include "nsIPrefService.h" #include "nsIPrefService.h"
#include "nsIScriptError.h"
#include "nsICookiePermission.h" #include "nsICookiePermission.h"
#include "nsIURI.h" #include "nsIURI.h"
#include "nsIURL.h" #include "nsIURL.h"
@ -2368,35 +2366,21 @@ NS_IMETHODIMP
nsCookieService::Remove(const nsACString &aHost, nsCookieService::Remove(const nsACString &aHost,
const nsACString &aName, const nsACString &aName,
const nsACString &aPath, const nsACString &aPath,
bool aBlocked,
JS::HandleValue aOriginAttributes, JS::HandleValue aOriginAttributes,
JSContext* aCx, bool aBlocked,
uint8_t aArgc) JSContext* aCx)
{ {
MOZ_ASSERT(aArgc == 0 || aArgc == 1);
if (aArgc == 0) {
// This is supposed to be temporary and in 1 or 2 releases we want to
// have originAttributes param as mandatory. But for now, we don't want to
// break existing addons, so we write a console message to inform the addon
// developers about it.
nsContentUtils::ReportToConsole(nsIScriptError::warningFlag,
NS_LITERAL_CSTRING("Cookie Manager"),
nullptr,
nsContentUtils::eNECKO_PROPERTIES,
"nsICookieManagerRemoveDeprecated");
}
NeckoOriginAttributes attrs; NeckoOriginAttributes attrs;
MOZ_ASSERT(attrs.Init(aCx, aOriginAttributes)); MOZ_ASSERT(attrs.Init(aCx, aOriginAttributes));
return RemoveNative(aHost, aName, aPath, aBlocked, &attrs); return RemoveNative(aHost, aName, aPath, &attrs, aBlocked);
} }
NS_IMETHODIMP_(nsresult) NS_IMETHODIMP_(nsresult)
nsCookieService::RemoveNative(const nsACString &aHost, nsCookieService::RemoveNative(const nsACString &aHost,
const nsACString &aName, const nsACString &aName,
const nsACString &aPath, const nsACString &aPath,
bool aBlocked, NeckoOriginAttributes* aOriginAttributes,
NeckoOriginAttributes* aOriginAttributes) bool aBlocked)
{ {
if (NS_WARN_IF(!aOriginAttributes)) { if (NS_WARN_IF(!aOriginAttributes)) {
return NS_ERROR_FAILURE; return NS_ERROR_FAILURE;

View File

@ -47,24 +47,21 @@ interface nsICookieManager : nsISupports
* dot must be present. * dot must be present.
* @param aName The name specified in the cookie * @param aName The name specified in the cookie
* @param aPath The path for which the cookie was set * @param aPath The path for which the cookie was set
* @param aOriginAttributes The originAttributes of this cookie. This * @param aOriginAttributes The originAttributes of this cookie
* attribute is optional to avoid breaking add-ons. * @param aBlocked Indicates if cookies from this host should be permanently blocked
* In 1 or 2 releases it will be mandatory: bug 1260399.
* @param aBlocked Indicates if cookies from this host should be permanently
* blocked.
* *
*/ */
[implicit_jscontext, optional_argc] [implicit_jscontext]
void remove(in AUTF8String aHost, void remove(in AUTF8String aHost,
in ACString aName, in ACString aName,
in AUTF8String aPath, in AUTF8String aPath,
in boolean aBlocked, in jsval aOriginAttributes,
[optional] in jsval aOriginAttributes); in boolean aBlocked);
[notxpcom] [notxpcom]
nsresult removeNative(in AUTF8String aHost, nsresult removeNative(in AUTF8String aHost,
in ACString aName, in ACString aName,
in AUTF8String aPath, in AUTF8String aPath,
in boolean aBlocked, in NeckoOriginAttributesPtr aOriginAttributes,
in NeckoOriginAttributesPtr aOriginAttributes); in boolean aBlocked);
}; };

View File

@ -43,6 +43,3 @@ TrackingUriBlocked=The resource at "%1$S" was blocked because tracking protectio
# LOCALIZATION NOTE (APIDeprecationWarning): # LOCALIZATION NOTE (APIDeprecationWarning):
# %1$S is the deprected API; %2$S is the API function that should be used. # %1$S is the deprected API; %2$S is the API function that should be used.
APIDeprecationWarning=Warning: '%1$S' deprecated, please use '%2$S' APIDeprecationWarning=Warning: '%1$S' deprecated, please use '%2$S'
# LOCALIZATION NOTE (nsICookieManagerRemoveDeprecated): don't localize nsICookieManager.remove() and originAttributes.
nsICookieManagerRemoveDeprecated="'nsICookieManager.remove()' is changed. Update your code and pass the correct originAttributes. Read more on MDN: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsICookieManager'

View File

@ -665,8 +665,8 @@ main(int32_t argc, char *argv[])
rv[10] = NS_SUCCEEDED(cookieMgr->RemoveNative(NS_LITERAL_CSTRING("new.domain"), // domain rv[10] = NS_SUCCEEDED(cookieMgr->RemoveNative(NS_LITERAL_CSTRING("new.domain"), // domain
NS_LITERAL_CSTRING("test3"), // name NS_LITERAL_CSTRING("test3"), // name
NS_LITERAL_CSTRING("/rabbit"), // path NS_LITERAL_CSTRING("/rabbit"), // path
true, // is blocked &attrs, // originAttributes
&attrs)); // originAttributes true)); // is blocked
rv[11] = NS_SUCCEEDED(cookieMgr2->CookieExists(newDomainCookie, &found)) && !found; rv[11] = NS_SUCCEEDED(cookieMgr2->CookieExists(newDomainCookie, &found)) && !found;
rv[12] = NS_SUCCEEDED(cookieMgr2->Add(NS_LITERAL_CSTRING("new.domain"), // domain rv[12] = NS_SUCCEEDED(cookieMgr2->Add(NS_LITERAL_CSTRING("new.domain"), // domain
NS_LITERAL_CSTRING("/rabbit"), // path NS_LITERAL_CSTRING("/rabbit"), // path

View File

@ -2285,8 +2285,8 @@ GeckoDriver.prototype.deleteAllCookies = function*(cmd, resp) {
cookie.host, cookie.host,
cookie.name, cookie.name,
cookie.path, cookie.path,
false, cookie.originAttributes,
cookie.originAttributes); false);
return true; return true;
}; };
this.mm.addMessageListener("Marionette:deleteCookie", cb); this.mm.addMessageListener("Marionette:deleteCookie", cb);
@ -2303,8 +2303,8 @@ GeckoDriver.prototype.deleteCookie = function*(cmd, resp) {
cookie.host, cookie.host,
cookie.name, cookie.name,
cookie.path, cookie.path,
false, cookie.originAttributes,
cookie.originAttributes); false);
return true; return true;
}; };
this.mm.addMessageListener("Marionette:deleteCookie", cb); this.mm.addMessageListener("Marionette:deleteCookie", cb);

View File

@ -295,7 +295,7 @@ extensions.registerSchemaAPI("cookies", "cookies", (extension, context) => {
remove: function(details) { remove: function(details) {
for (let cookie of query(details, ["url", "name", "storeId"], extension)) { for (let cookie of query(details, ["url", "name", "storeId"], extension)) {
Services.cookies.remove(cookie.host, cookie.name, cookie.path, false, cookie.originAttributes); Services.cookies.remove(cookie.host, cookie.name, cookie.path, cookie.originAttributes, false);
// Todo: could there be multiple per subdomain? // Todo: could there be multiple per subdomain?
return Promise.resolve({ return Promise.resolve({
url: details.url, url: details.url,

View File

@ -117,7 +117,7 @@ function* testCookies(options) {
yield extension.awaitMessage("change-cookies"); yield extension.awaitMessage("change-cookies");
cookieSvc.add(domain, "/", "x", "y", options.secure, false, false, options.expiry); cookieSvc.add(domain, "/", "x", "y", options.secure, false, false, options.expiry);
cookieSvc.add(domain, "/", "x", "z", options.secure, false, false, options.expiry); cookieSvc.add(domain, "/", "x", "z", options.secure, false, false, options.expiry);
cookieSvc.remove(domain, "x", "/", false, {}); cookieSvc.remove(domain, "x", "/", {}, false);
extension.sendMessage("cookies-changed"); extension.sendMessage("cookies-changed");
yield extension.awaitFinish("cookie-permissions"); yield extension.awaitFinish("cookie-permissions");
@ -168,7 +168,7 @@ function* testCookies(options) {
} }
for (let cookie of cookies) { for (let cookie of cookies) {
cookieSvc.remove(cookie.host, cookie.name, "/", false, {}); cookieSvc.remove(cookie.host, cookie.name, "/", {}, false);
} }
// Make sure we don't silently poison subsequent tests if something goes wrong. // Make sure we don't silently poison subsequent tests if something goes wrong.
is(getCookies(options.domain).length, 0, "cookies cleared"); is(getCookies(options.domain).length, 0, "cookies cleared");

View File

@ -77,7 +77,7 @@ this.ForgetAboutSite = {
let enumerator = cm.getCookiesFromHost(aDomain); let enumerator = cm.getCookiesFromHost(aDomain);
while (enumerator.hasMoreElements()) { while (enumerator.hasMoreElements()) {
let cookie = enumerator.getNext().QueryInterface(Ci.nsICookie); let cookie = enumerator.getNext().QueryInterface(Ci.nsICookie);
cm.remove(cookie.host, cookie.name, cookie.path, false, cookie.originAttributes); cm.remove(cookie.host, cookie.name, cookie.path, cookie.originAttributes, false);
} }
// EME // EME

View File

@ -31,7 +31,7 @@ function finish_test(count) {
var cm = Components.classes["@mozilla.org/cookiemanager;1"] var cm = Components.classes["@mozilla.org/cookiemanager;1"]
.getService(Components.interfaces.nsICookieManager2); .getService(Components.interfaces.nsICookieManager2);
cm.remove("example.com", "xpinstall", "/browser/" + RELATIVE_DIR, false, {}); cm.remove("example.com", "xpinstall", "/browser/" + RELATIVE_DIR, {}, false);
Services.perms.remove(makeURI("http://example.com"), "install"); Services.perms.remove(makeURI("http://example.com"), "install");

View File

@ -33,7 +33,7 @@ function finish_test(count) {
var cm = Components.classes["@mozilla.org/cookiemanager;1"] var cm = Components.classes["@mozilla.org/cookiemanager;1"]
.getService(Components.interfaces.nsICookieManager2); .getService(Components.interfaces.nsICookieManager2);
cm.remove("example.com", "xpinstall", "/browser/" + RELATIVE_DIR, false, {}); cm.remove("example.com", "xpinstall", "/browser/" + RELATIVE_DIR, {}, false);
Services.prefs.clearUserPref("network.cookie.cookieBehavior"); Services.prefs.clearUserPref("network.cookie.cookieBehavior");

View File

@ -33,7 +33,7 @@ function finish_test(count) {
is(count, 0, "No add-ons should have been installed"); is(count, 0, "No add-ons should have been installed");
var cm = Components.classes["@mozilla.org/cookiemanager;1"] var cm = Components.classes["@mozilla.org/cookiemanager;1"]
.getService(Components.interfaces.nsICookieManager2); .getService(Components.interfaces.nsICookieManager2);
cm.remove("example.org", "xpinstall", "/browser/" + RELATIVE_DIR, false, {}); cm.remove("example.org", "xpinstall", "/browser/" + RELATIVE_DIR, {}, false);
Services.prefs.clearUserPref("network.cookie.cookieBehavior"); Services.prefs.clearUserPref("network.cookie.cookieBehavior");
Services.perms.remove(makeURI("http://example.com"), "install"); Services.perms.remove(makeURI("http://example.com"), "install");