Backed out 2 changesets (bug 1828126) for causing xpcshell failures in test_timestamp_fixup.js. CLOSED TREE

Backed out changeset 66859868cbe0 (bug 1828126)
Backed out changeset bf9de9179b04 (bug 1828126)
This commit is contained in:
Stanca Serban 2023-04-27 16:59:43 +03:00
parent 4e02dd13fb
commit 18b9a1123b
8 changed files with 7 additions and 279 deletions

View File

@ -11272,14 +11272,6 @@
value: false
mirror: always
# If true cookies loaded from the sqlite DB that have a creation or
# last accessed time that is in the future will be fixed and the
# timestamps will be set to the current time.
- name: network.cookie.fixup_on_db_load
type: RelaxedAtomicBool
value: true
mirror: always
# If we should attempt to race the cache and network.
- name: network.http.rcwn.enabled
type: bool

View File

@ -7,7 +7,6 @@
#include "CookieStorage.h"
#include "mozilla/Encoding.h"
#include "mozilla/dom/ToJSValue.h"
#include "mozilla/glean/GleanMetrics.h"
#include "mozilla/StaticPrefs_network.h"
#include "nsIURLParser.h"
#include "nsURLHelper.h"
@ -45,21 +44,6 @@ int64_t Cookie::GenerateUniqueCreationTime(int64_t aCreationTime) {
already_AddRefed<Cookie> Cookie::Create(
const CookieStruct& aCookieData,
const OriginAttributes& aOriginAttributes) {
RefPtr<Cookie> cookie =
Cookie::FromCookieStruct(aCookieData, aOriginAttributes);
// If the creationTime given to us is higher than the running maximum,
// update our maximum.
if (cookie->mData.creationTime() > gLastCreationTime) {
gLastCreationTime = cookie->mData.creationTime();
}
return cookie.forget();
}
already_AddRefed<Cookie> Cookie::FromCookieStruct(
const CookieStruct& aCookieData,
const OriginAttributes& aOriginAttributes) {
RefPtr<Cookie> cookie = new Cookie(aCookieData, aOriginAttributes);
// Ensure mValue contains a valid UTF-8 sequence. Otherwise XPConnect will
@ -67,6 +51,12 @@ already_AddRefed<Cookie> Cookie::FromCookieStruct(
UTF_8_ENCODING->DecodeWithoutBOMHandling(aCookieData.value(),
cookie->mData.value());
// If the creationTime given to us is higher than the running maximum,
// update our maximum.
if (cookie->mData.creationTime() > gLastCreationTime) {
gLastCreationTime = cookie->mData.creationTime();
}
// If sameSite/rawSameSite values aren't sensible reset to Default
// cf. 5.4.7 in draft-ietf-httpbis-rfc6265bis-09
if (!Cookie::ValidateSameSite(cookie->mData)) {
@ -77,52 +67,6 @@ already_AddRefed<Cookie> Cookie::FromCookieStruct(
return cookie.forget();
}
already_AddRefed<Cookie> Cookie::CreateValidated(
const CookieStruct& aCookieData,
const OriginAttributes& aOriginAttributes) {
if (!StaticPrefs::network_cookie_fixup_on_db_load()) {
return Cookie::Create(aCookieData, aOriginAttributes);
}
RefPtr<Cookie> cookie =
Cookie::FromCookieStruct(aCookieData, aOriginAttributes);
int64_t currentTimeInUsec = PR_Now();
// Assert that the last creation time is not higher than the current time.
// The 10000 wiggle room accounts for the fact that calling
// GenerateUniqueCreationTime might go over the value of PR_Now(), but we'd
// most likely not add 10000 cookies in a row.
MOZ_ASSERT(gLastCreationTime < currentTimeInUsec + 10000,
"Last creation time must not be higher than NOW");
// If the creationTime given to us is higher than the current time then
// update the creation time to now.
if (cookie->mData.creationTime() > currentTimeInUsec) {
uint64_t diffInSeconds =
(cookie->mData.creationTime() - currentTimeInUsec) / PR_USEC_PER_SEC;
mozilla::glean::networking::cookie_creation_fixup_diff.AccumulateSamples(
{diffInSeconds});
glean::networking::cookie_timestamp_fixed_count.Get("creationTime"_ns)
.Add(1);
cookie->mData.creationTime() =
GenerateUniqueCreationTime(currentTimeInUsec);
}
if (cookie->mData.lastAccessed() > currentTimeInUsec) {
uint64_t diffInSeconds =
(cookie->mData.lastAccessed() - currentTimeInUsec) / PR_USEC_PER_SEC;
mozilla::glean::networking::cookie_access_fixup_diff.AccumulateSamples(
{diffInSeconds});
glean::networking::cookie_timestamp_fixed_count.Get("lastAccessed"_ns)
.Add(1);
cookie->mData.lastAccessed() = currentTimeInUsec;
}
return cookie.forget();
}
size_t Cookie::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const {
return aMallocSizeOf(this) +
mData.name().SizeOfExcludingThisIfUnshared(MallocSizeOf) +

View File

@ -44,10 +44,6 @@ class Cookie final : public nsICookie {
const OriginAttributes& aOriginAttributes)
: mData(aCookieData), mOriginAttributes(aOriginAttributes) {}
static already_AddRefed<Cookie> FromCookieStruct(
const CookieStruct& aCookieData,
const OriginAttributes& aOriginAttributes);
public:
// Returns false if rawSameSite has an invalid value, compared to sameSite.
static bool ValidateSameSite(const CookieStruct& aCookieData);
@ -61,13 +57,6 @@ class Cookie final : public nsICookie {
const CookieStruct& aCookieData,
const OriginAttributes& aOriginAttributes);
// Same as Cookie::Create but fixes the lastAccessed and creationDates
// if they are set in the future.
// Should only get called from CookiePersistentStorage::InitDBConn
static already_AddRefed<Cookie> CreateValidated(
const CookieStruct& aCookieData,
const OriginAttributes& aOriginAttributes);
size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
// fast (inline, non-xpcom) getters

View File

@ -411,7 +411,6 @@ already_AddRefed<Cookie> CookieCommons::CreateCookieFromDocument(
nsCString cookieString(aCookieString);
CookieStruct cookieData;
MOZ_ASSERT(cookieData.creationTime() == 0, "Must be initialized to 0");
bool canSetCookie = false;
CookieService::CanSetCookie(principalURI, baseDomain, cookieData,
requireHostMatch, cookieStatus, cookieString,

View File

@ -1751,17 +1751,8 @@ void CookiePersistentStorage::InitDBConn() {
CookieDomainTuple& tuple = mReadArray[i];
MOZ_ASSERT(!tuple.cookie->isSession());
// CreateValidated fixes up the creation and lastAccessed times.
// If the DB is corrupted and the timestaps are far away in the future
// we don't want the creation timestamp to update gLastCreationTime
// as that would contaminate all the next creation times.
// We fix up these dates to not be later than the current time.
// The downside is that if the user sets the date far away in the past
// then back to the current date, those cookies will be stale,
// but if we don't fix their dates, those cookies might never be
// evicted.
RefPtr<Cookie> cookie =
Cookie::CreateValidated(*tuple.cookie, tuple.originAttributes);
Cookie::Create(*tuple.cookie, tuple.originAttributes);
AddCookieToList(tuple.key.mBaseDomain, tuple.key.mOriginAttributes, cookie);
}

View File

@ -1,125 +0,0 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */
const USEC_PER_SEC = 1000 * 1000;
const ONE_DAY = 60 * 60 * 24 * USEC_PER_SEC;
const ONE_YEAR = ONE_DAY * 365;
const LAST_ACCESSED_DIFF = 10 * ONE_YEAR;
const CREATION_DIFF = 100 * ONE_YEAR;
function initDB(conn, now) {
// Write the schema v7 to the database.
conn.schemaVersion = 7;
conn.executeSimpleSQL(
"CREATE TABLE moz_cookies (" +
"id INTEGER PRIMARY KEY, " +
"baseDomain TEXT, " +
"originAttributes TEXT NOT NULL DEFAULT '', " +
"name TEXT, " +
"value TEXT, " +
"host TEXT, " +
"path TEXT, " +
"expiry INTEGER, " +
"lastAccessed INTEGER, " +
"creationTime INTEGER, " +
"isSecure INTEGER, " +
"isHttpOnly INTEGER, " +
"appId INTEGER DEFAULT 0, " +
"inBrowserElement INTEGER DEFAULT 0, " +
"CONSTRAINT moz_uniqueid UNIQUE (name, host, path, originAttributes)" +
")"
);
conn.executeSimpleSQL(
"CREATE INDEX moz_basedomain ON moz_cookies (baseDomain, " +
"originAttributes)"
);
conn.executeSimpleSQL("PRAGMA synchronous = OFF");
conn.executeSimpleSQL("PRAGMA journal_mode = WAL");
conn.executeSimpleSQL("PRAGMA wal_autocheckpoint = 16");
conn.executeSimpleSQL(
`INSERT INTO moz_cookies(baseDomain, host, name, value, path, expiry, lastAccessed, creationTime, isSecure, isHttpOnly)
VALUES ('foo.com', '.foo.com', 'foo', 'bar=baz', '/',
${now + ONE_DAY}, ${now + LAST_ACCESSED_DIFF} , ${now +
CREATION_DIFF} , 1, 1)`
);
}
add_task(async function test_timestamp_fixup() {
let now = Date.now() * 1000; // date in microseconds
Services.prefs.setBoolPref("network.cookie.fixup_on_db_load", true);
do_get_profile();
let dbFile = Services.dirsvc.get("ProfD", Ci.nsIFile);
dbFile.append("cookies.sqlite");
let conn = Services.storage.openDatabase(dbFile);
initDB(conn, now);
Services.fog.initializeFOG();
Services.fog.testResetFOG();
// Now start the cookie service, and then check the fields in the table.
// Get sessionCookies to wait for the initialization in cookie thread
Assert.lessOrEqual(
Math.floor(Services.cookies.cookies[0].creationTime / 1000),
now
);
Assert.equal(conn.schemaVersion, 12);
Assert.equal(
await Glean.networking.cookieTimestampFixedCount.creationTime.testGetValue(),
1,
"One fixup of creation time"
);
Assert.equal(
await Glean.networking.cookieTimestampFixedCount.lastAccessed.testGetValue(),
1,
"One fixup of lastAccessed"
);
{
let {
values,
} = await Glean.networking.cookieCreationFixupDiff.testGetValue();
info(JSON.stringify(values));
let keys = Object.keys(values).splice(-2, 2);
Assert.equal(keys.length, 2, "There should be two entries in telemetry");
Assert.equal(values[keys[0]], 1, "First entry should have value 1");
Assert.equal(values[keys[1]], 0, "Second entry should have value 0");
const creationDiffInSeconds = CREATION_DIFF / USEC_PER_SEC;
Assert.lessOrEqual(
parseInt(keys[0]),
creationDiffInSeconds,
"The bucket should be smaller than time diff"
);
Assert.lessOrEqual(
creationDiffInSeconds,
parseInt(keys[1]),
"The next bucket should be larger than time diff"
);
}
{
let {
values,
} = await Glean.networking.cookieAccessFixupDiff.testGetValue();
info(JSON.stringify(values));
let keys = Object.keys(values).splice(-2, 2);
Assert.equal(keys.length, 2, "There should be two entries in telemetry");
Assert.equal(values[keys[0]], 1, "First entry should have value 1");
Assert.equal(values[keys[1]], 0, "Second entry should have value 0");
info(now);
const lastAccessedDiffInSeconds = LAST_ACCESSED_DIFF / USEC_PER_SEC;
Assert.lessOrEqual(
parseInt(keys[0]),
lastAccessedDiffInSeconds,
"The bucket should be smaller than time diff"
);
Assert.lessOrEqual(
lastAccessedDiffInSeconds,
parseInt(keys[1]),
"The next bucket should be larger than time diff"
);
}
conn.close();
});

View File

@ -12,4 +12,3 @@ head =
[test_parser_0019.js]
[test_rawSameSite.js]
[test_schemeMap.js]
[test_timestamp_fixup.js]

View File

@ -31,64 +31,3 @@ networking:
- aborted_socket_fail
- aborted_https_not_enabled
telemetry_mirror: NETWORKING_SPECULATIVE_CONNECT_OUTCOME
cookie_timestamp_fixed_count:
type: labeled_counter
description: >
Counts the number of times a cookie's invalid timestamp was fixed when
reading it from the DB.
bugs:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1828126
data_reviews:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1828126#c5
data_sensitivity:
- technical
notification_emails:
- necko@mozilla.com
- vgosu@mozilla.com
labels:
- creationTime
- lastAccessed
expires: never
cookie_creation_fixup_diff:
type: custom_distribution
unit: second
description: >
If we fix up a cookie creation timestamp that is in the future this
metric records the number of seconds that timestamp was off from NOW.
bugs:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1828126
data_reviews:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1828126#c5
data_sensitivity:
- technical
notification_emails:
- necko@mozilla.com
- vgosu@mozilla.com
expires: never
range_min: 0
range_max: 315360000000
bucket_count: 100
histogram_type: exponential
cookie_access_fixup_diff:
type: custom_distribution
unit: second
description: >
If we fix up a cookie lastAccessed timestamp that is in the future this
metric records the number of seconds that timestamp was off from NOW.
bugs:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1828126
data_reviews:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1828126#c5
data_sensitivity:
- technical
notification_emails:
- necko@mozilla.com
- vgosu@mozilla.com
expires: never
range_min: 0
range_max: 315360000000
bucket_count: 100
histogram_type: exponential