Bug Bug 1964216 - CookieStore API must consider the http-only flag, r=valentin,cookie-reviewers

Differential Revision: https://phabricator.services.mozilla.com/D247774
This commit is contained in:
Andrea Marchesini
2025-05-06 14:13:29 +00:00
committed by amarchesini@mozilla.com
parent 4f8de96804
commit 7e56d9f7ec
9 changed files with 65 additions and 20 deletions

View File

@@ -417,11 +417,10 @@ bool CookieStoreParent::SetRequestOnMainThread(
rv = service->AddNative(
aCookieURI, domainWithDot, NS_ConvertUTF16toUTF8(aPath),
NS_ConvertUTF16toUTF8(aName), NS_ConvertUTF16toUTF8(aValue),
true, // secure
false, // mHttpOnly,
aSession, aSession ? INT64_MAX : aExpires, &attrs, aSameSite,
nsICookie::SCHEME_HTTPS, aPartitioned, &aOperationID,
[&](mozilla::net::CookieStruct& aCookieStruct) -> bool {
/* secure: */ true,
/* http-only: */ false, aSession, aSession ? INT64_MAX : aExpires, &attrs,
aSameSite, nsICookie::SCHEME_HTTPS, aPartitioned, /* from http: */ false,
&aOperationID, [&](mozilla::net::CookieStruct& aCookieStruct) -> bool {
return CookieParser::CheckCookieStruct(aCookieStruct, aCookieURI, ""_ns,
domain, requireMatch, false) ==
CookieParser::NoRejection;
@@ -521,7 +520,8 @@ bool CookieStoreParent::DeleteRequestOnMainThread(
notificationWatcher->CallbackWhenNotified(aOperationID, notificationCb);
rv = cookieManager->RemoveNative(cookie->Host(), matchName, cookie->Path(),
&attrs, &aOperationID);
&attrs, /* from http: */ false,
&aOperationID);
if (NS_WARN_IF(NS_FAILED(rv))) {
return false;
}

View File

@@ -722,7 +722,7 @@ CookieService::Add(const nsACString& aHost, const nsACString& aPath,
return AddNative(nullptr, aHost, aPath, aName, aValue, aIsSecure, aIsHttpOnly,
aIsSession, aExpiry, &attrs, aSameSite, aSchemeMap,
aIsPartitioned, nullptr,
aIsPartitioned, /* from-http: */ true, nullptr,
[](CookieStruct&) -> bool { return true; });
}
@@ -733,7 +733,7 @@ CookieService::AddNative(nsIURI* aCookieURI, const nsACString& aHost,
bool aIsHttpOnly, bool aIsSession, int64_t aExpiry,
OriginAttributes* aOriginAttributes, int32_t aSameSite,
nsICookie::schemeType aSchemeMap, bool aIsPartitioned,
const nsID* aOperationID,
bool aFromHttp, const nsID* aOperationID,
const std::function<bool(CookieStruct&)>& aCheck) {
if (NS_WARN_IF(!aOriginAttributes)) {
return NS_ERROR_FAILURE;
@@ -770,7 +770,7 @@ CookieService::AddNative(nsIURI* aCookieURI, const nsACString& aHost,
CookieStorage* storage = PickStorage(*aOriginAttributes);
storage->AddCookie(nullptr, baseDomain, *aOriginAttributes, cookie,
currentTimeInUsec, aCookieURI, VoidCString(), true,
currentTimeInUsec, aCookieURI, VoidCString(), aFromHttp,
!aOriginAttributes->mPartitionKey.IsEmpty(), nullptr,
aOperationID);
return NS_OK;
@@ -779,7 +779,7 @@ CookieService::AddNative(nsIURI* aCookieURI, const nsACString& aHost,
nsresult CookieService::Remove(const nsACString& aHost,
const OriginAttributes& aAttrs,
const nsACString& aName, const nsACString& aPath,
const nsID* aOperationID) {
bool aFromHttp, const nsID* aOperationID) {
// first, normalize the hostname, and fail if it contains illegal characters.
nsAutoCString host(aHost);
nsresult rv = NormalizeHost(host);
@@ -797,7 +797,7 @@ nsresult CookieService::Remove(const nsACString& aHost,
CookieStorage* storage = PickStorage(aAttrs);
storage->RemoveCookie(baseDomain, aAttrs, host, PromiseFlatCString(aName),
PromiseFlatCString(aPath), aOperationID);
PromiseFlatCString(aPath), aFromHttp, aOperationID);
return NS_OK;
}
@@ -812,19 +812,21 @@ CookieService::Remove(const nsACString& aHost, const nsACString& aName,
return NS_ERROR_INVALID_ARG;
}
return RemoveNative(aHost, aName, aPath, &attrs, nullptr);
return RemoveNative(aHost, aName, aPath, &attrs, /* from http: */ true,
nullptr);
}
NS_IMETHODIMP_(nsresult)
CookieService::RemoveNative(const nsACString& aHost, const nsACString& aName,
const nsACString& aPath,
OriginAttributes* aOriginAttributes,
OriginAttributes* aOriginAttributes, bool aFromHttp,
const nsID* aOperationID) {
if (NS_WARN_IF(!aOriginAttributes)) {
return NS_ERROR_FAILURE;
}
nsresult rv = Remove(aHost, *aOriginAttributes, aName, aPath, aOperationID);
nsresult rv =
Remove(aHost, *aOriginAttributes, aName, aPath, aFromHttp, aOperationID);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
@@ -1486,7 +1488,8 @@ class RemoveAllSinceRunnable : public Runnable {
auto* cookie = static_cast<Cookie*>(mList[mIndex].get());
if (cookie->CreationTime() > mSinceWhen &&
NS_FAILED(mSelf->Remove(cookie->Host(), cookie->OriginAttributesRef(),
cookie->Name(), cookie->Path(), nullptr))) {
cookie->Name(), cookie->Path(),
/* from http: */ true, nullptr))) {
continue;
}
}

View File

@@ -98,7 +98,7 @@ class CookieService final : public nsICookieService,
*/
nsresult Remove(const nsACString& aHost, const OriginAttributes& aAttrs,
const nsACString& aName, const nsACString& aPath,
const nsID* aOperationID);
bool aFromHttp, const nsID* aOperationID);
bool SetCookiesFromIPC(const nsACString& aBaseDomain,
const OriginAttributes& aAttrs, nsIURI* aHostURI,

View File

@@ -322,13 +322,19 @@ void CookieStorage::RemoveCookie(const nsACString& aBaseDomain,
const OriginAttributes& aOriginAttributes,
const nsACString& aHost,
const nsACString& aName,
const nsACString& aPath,
const nsACString& aPath, bool aFromHttp,
const nsID* aOperationID) {
CookieListIter matchIter{};
RefPtr<Cookie> cookie;
if (FindCookie(aBaseDomain, aOriginAttributes, aHost, aName, aPath,
matchIter)) {
cookie = matchIter.Cookie();
// If the old cookie is httponly, make sure we're not coming from script.
if (cookie && !aFromHttp && cookie->IsHttpOnly()) {
return;
}
RemoveCookieFromList(matchIter);
}

View File

@@ -110,7 +110,8 @@ class CookieStorage : public nsIObserver, public nsSupportsWeakReference {
void RemoveCookie(const nsACString& aBaseDomain,
const OriginAttributes& aOriginAttributes,
const nsACString& aHost, const nsACString& aName,
const nsACString& aPath, const nsID* aOperationID);
const nsACString& aPath, bool aFromHttp,
const nsID* aOperationID);
virtual void RemoveCookiesWithOriginAttributes(
const OriginAttributesPattern& aPattern, const nsACString& aBaseDomain);

View File

@@ -88,6 +88,7 @@ interface nsICookieManager : nsISupports
in ACString aName,
in AUTF8String aPath,
in OriginAttributesPtr aOriginAttributes,
in boolean aFromHttp,
in nsIDPtr aOperationID);
/**
@@ -158,6 +159,7 @@ interface nsICookieManager : nsISupports
in int32_t aSameSite,
in nsICookie_schemeType aSchemeMap,
in boolean aIsPartitioned,
in boolean aFromHttp,
in nsIDPtr aOperationID,
in CheckStructFunctionRef aCheckValid);

View File

@@ -771,6 +771,7 @@ TEST(TestCookie, TestCookieMain)
&attrs, // originAttributes
nsICookie::SAMESITE_NONE, nsICookie::SCHEME_HTTPS,
false, // is partitioned
true, // from http
nullptr, // operation ID
[](CookieStruct&) -> bool { return true; })));
EXPECT_TRUE(NS_SUCCEEDED(
@@ -786,6 +787,7 @@ TEST(TestCookie, TestCookieMain)
&attrs, // originAttributes
nsICookie::SAMESITE_NONE, nsICookie::SCHEME_HTTPS,
false, // is partitioned
true, // from http
nullptr, // operation ID
[](CookieStruct&) -> bool { return true; })));
EXPECT_TRUE(NS_SUCCEEDED(
@@ -801,6 +803,7 @@ TEST(TestCookie, TestCookieMain)
&attrs, // originAttributes
nsICookie::SAMESITE_NONE, nsICookie::SCHEME_HTTPS,
false, // is partitioned
true, // from http
nullptr, // operation ID
[](CookieStruct&) -> bool { return true; })));
// confirm using enumerator

View File

@@ -67,3 +67,33 @@ cookie_test(async t => {
'cookie1=value1; cookie2=value2; cookie3=value3',
'httpOnly is not an option for CookieStore.set()');
}, 'HttpOnly cookies can not be set by CookieStore');
cookie_test(async t => {
await setCookieStringHttp('HTTPONLY-cookie=value; path=/; httponly');
assert_equals(
await getCookieString(),
undefined,
'HttpOnly cookie we wrote using HTTP in cookie jar' +
' is invisible to script');
assert_equals(
await getCookieStringHttp(),
'HTTPONLY-cookie=value',
'HttpOnly cookie we wrote using HTTP in HTTP cookie jar');
try {
await cookieStore.set('HTTPONLY-cookie', 'dummy');
} catch(e) {}
assert_equals(
await getCookieString(),
undefined,
'HttpOnly cookie is not overwritten');
try {
await cookieStore.delete('HTTPONLY-cookie');
} catch(e) {}
assert_equals(await getCookieString(), undefined, 'HttpOnly cookie is not overwritten');
assert_equals(await getCookieStringHttp(), 'HTTPONLY-cookie=value', 'HttpOnly cookie is not deleted');
}, 'HttpOnly cookies are not deleted/overwritten');

View File

@@ -327,7 +327,7 @@ nsresult nsCookieInjector::InjectCookiesFromRules(
nullptr, c.Host(), c.Path(), c.Name(), c.Value(), c.IsSecure(),
c.IsHttpOnly(), c.IsSession(), c.Expiry(), &aOriginAttributes,
c.SameSite(), static_cast<nsICookie::schemeType>(c.SchemeMap()),
/* is partitioned: */ false, nullptr,
/* is partitioned: */ false, /* is from http: */ true, nullptr,
[](mozilla::net::CookieStruct&) { return true; });
NS_ENSURE_SUCCESS(rv, rv);