Previously, if script tried to set a cookie that matched a cookie we had
received via Set-Cookie that was labeled httponly, script would think
that cookie was properly set (even though it wasn't). This ensures that
script knows just enough about httponly cookies to prevent this
inconsistent view while avoiding leakages of the potentially-sensitive
cookie values.
Differential Revision: https://phabricator.services.mozilla.com/D5700
--HG--
extra : moz-landing-system : lando
Everything that goes in a PLDHashtable (and its derivatives, like
nsTHashtable) needs to inherit from PLDHashEntryHdr. But through a lack
of enforcement, copy constructors for these derived classes didn't
explicitly invoke the copy constructor for PLDHashEntryHdr (and the
compiler didn't invoke the copy constructor for us). Instead,
PLDHashTable explicitly copied around the bits that the copy constructor
would have.
The current setup has two problems:
1) Derived classes should be using move construction, not copy
construction, since anything that's shuffling hash table keys/entries
around will be using move construction.
2) Derived classes should take responsibility for transferring bits of
superclass state around, and not rely on something else to handle that.
The second point is not a huge problem for PLDHashTable (PLDHashTable
only has to copy PLDHashEntryHdr's bits in a single place), but future
hash table implementations that might move entries around more
aggressively would have to insert compensation code all over the
place. Additionally, if moving entries is implemented via memcpy (which
is quite common), PLDHashTable copying around bits *again* is
inefficient.
Let's fix all these problems in one go, by:
1) Explicitly declaring the set of constructors that PLDHashEntryHdr
implements (and does not implement). In particular, the copy
constructor is deleted, so any derived classes that attempt to make
themselves copyable will be detected at compile time: the compiler
will complain that the superclass type is not copyable.
This change on its own will result in many compiler errors, so...
2) Change any derived classes to implement move constructors instead of
copy constructors. Note that some of these move constructors are,
strictly speaking, unnecessary, since the relevant classes are moved
via memcpy in nsTHashtable and its derivatives.
In the current implmentation of CookieServiceChild::SetCookieString, pass a null channel will crash the child process. This is because we call aChannel->GetURI() without checking if aChannel is null.
However, set cookie with a null channel is possible in non-e10s mode. To make sure the behavior to be consistent in both non-e10s and e10s mode, we have to pass an empty URIParams in child process.
Differential Revision: https://phabricator.services.mozilla.com/D5432
--HG--
extra : moz-landing-system : lando
> dom/media/gmp/CDMStorageIdProvider.cpp(63,10): warning:
> local variable 'storageId' will be copied despite being returned by name [-Wreturn-std-move]
nsAutoCString -> nsCString, will add std::move().
> layout/painting/DisplayItemClip.cpp(581,10): warning:
> local variable 'str' will be copied despite being returned by name [-Wreturn-std-move]
nsAutoCString -> nsCString, will add std::move().
> layout/painting/DisplayItemClipChain.cpp(88,10): warning:
> local variable 'str' will be copied despite being returned by name [-Wreturn-std-move]
nsAutoCString -> nsCString, will add std::move().
> layout/painting/nsDisplayList.cpp(179,10): warning:
> local variable 'str' will be copied despite being returned by name [-Wreturn-std-move]
nsAutoCString -> nsCString, will add std::move().
> gfx/thebes/gfxWindowsPlatform.cpp(454,10): warning:
> moving a local object in a return statement prevents copy elision [-Wpessimizing-move]
Will remove std::move().
> gfx/thebes/gfxFontEntry.cpp(245,20): warning:
> local variable 'name' will be copied despite being returned by name [-Wreturn-std-move]
nsAutoCString -> nsCString, will add std::move().
> netwerk/cookie/nsCookieService.cpp(4460,10): warning:
> local variable 'path' will be copied despite being returned by name [-Wreturn-std-move]
GetPathFromURI() result is stored in an nsAutoCString, so it might as well return that type.
> toolkit/components/extensions/WebExtensionPolicy.cpp(462,12): warning:
> local variable 'result' will be copied despite being returned by name [-Wreturn-std-move]
> toolkit/components/extensions/WebExtensionPolicy.cpp(475,10): warning:
> local variable 'result' will be copied despite being returned by name [-Wreturn-std-move]
`result` may be empty or may be arbitrarily long, so I'll use nsCString inside the function.
> toolkit/xre/CmdLineAndEnvUtils.h(349,10): warning:
> moving a local object in a return statement prevents copy elision [-Wpessimizing-move]
Returning an UniquePtr, will remove std::move().
Also will `return s` instead of `return nullptr` when `(!s)`, to avoid extra construction which could also prevent elision (not entirely sure, but it's at least not worse!); and it's clearer that the two `return`s return the same already-constructed on-stack object.
> tools/profiler/core/shared-libraries-win32.cc(111,10): warning:
> local variable 'version' will be copied despite being returned by name [-Wreturn-std-move]
nsPrintfCString -> nsCString, will add std::move().
> xpcom/glue/FileUtils.cpp(179,10): warning:
> local variable 'fullName' will be copied despite being returned by name [-Wreturn-std-move]
> xpcom/glue/FileUtils.cpp(209,10): warning:
> local variable 'path' will be copied despite being returned by name [-Wreturn-std-move]
nsAuto{,C}String -> ns{,C}String, will add std::move().
This allowed removals of 'AllowCompilerWarnings' from layout/painting,
netwerk/cookie, and toolkit/components/extensions.
Differential Revision: https://phabricator.services.mozilla.com/D5425
--HG--
extra : moz-landing-system : lando
The fail log shows that the cookie's expiring time is too short(only 1s). The cookie could be already expired.
A simple solution in this patch is just extending the expiring time to 3s.
Differential Revision: https://phabricator.services.mozilla.com/D5034
--HG--
extra : moz-landing-system : lando
1. Add network.cookie.QuotaPerHost, which has the default value 150.
2. When the cookies exceed more than 180, evict cookies to 150.
3. The concept of eviction is to sort all cookies by whether the cookie is expired and the cookie's last access time. Then, evict cookies by the given count.
4. Details of evict algorithm:
4.1 Create a priority queue and push all cookies in it.
4.2 Use custom comparator to compare the cookie by expiry and last access.
4.3 Pop 30(180 - 150) cookies from the queue and append them to an output array.
Differential Revision: https://phabricator.services.mozilla.com/D3342
--HG--
extra : moz-landing-system : lando
Introduce these new blocking state values:
const unsigned long STATE_COOKIES_BLOCKED_BY_PERMISSION = 0x10000000;
const unsigned long STATE_COOKIES_BLOCKED_TRACKER = 0x20000000;
const unsigned long STATE_COOKIES_BLOCKED_ALL = 0x40000000;
const unsigned long STATE_COOKIES_BLOCKED_FOREIGN = 0x80000000;
This allows JS callers to automatically get the correct types during
interation, without having to explicitly specify them.
Differential Revision: https://phabricator.services.mozilla.com/D3728
--HG--
extra : rebase_source : b708f382d8ea571d199c669bfed5b5a7ca9ffac4
extra : histedit_source : 7df6feb82088c8a5ca45dc28fe4d2b852c177fee
This patch introduces a new cookie behavior policy called
BEHAVIOR_REJECT_TRACKER. It also makes it possible to override that
behavior with cookie permissions similar to other cookie behaviors.
Everything that goes in a PLDHashtable (and its derivatives, like
nsTHashtable) needs to inherit from PLDHashEntryHdr. But through a lack
of enforcement, copy constructors for these derived classes didn't
explicitly invoke the copy constructor for PLDHashEntryHdr (and the
compiler didn't invoke the copy constructor for us). Instead,
PLDHashTable explicitly copied around the bits that the copy constructor
would have.
The current setup has two problems:
1) Derived classes should be using move construction, not copy
construction, since anything that's shuffling hash table keys/entries
around will be using move construction.
2) Derived classes should take responsibility for transferring bits of
superclass state around, and not rely on something else to handle
that.
The second point is not a huge problem for PLDHashTable (PLDHashTable
only has to copy PLDHashEntryHdr's bits in a single place), but future
hash table implementations that might move entries around more
aggressively would have to insert compensation code all over the place.
Additionally, if moving entries is implemented via memcpy (which is
quite common), PLDHashTable copying around bits *again* is inefficient.
Let's fix all these problems in one go, by:
1) Explicitly declaring the set of constructors that PLDHashEntryHdr
implements (and does not implement). In particular, the copy
constructor is deleted, so any derived classes that attempt to make
themselves copyable will be detected at compile time: the compiler
will complain that the superclass type is not copyable.
This change on its own will result in many compiler errors, so...
2) Change any derived classes to implement move constructors instead
of copy constructors. Note that some of these move constructors are,
strictly speaking, unnecessary, since the relevant classes are moved
via memcpy in nsTHashtable and its derivatives.
InitializeOriginAttributes takes aArgc and only initializes the
parameter when aArgc is 1. nsCookieService::Add takes another optional
parameter, namely aSameSite. If a caller sets this SameSite flag, then
InitializeOriginAttributes would skip the initialization of the
OriginAttributes.
This was caught by a private browsing test in
toolkit/components/extensions/test/mochitest/test_ext_cookies.html
(after I added support for SameSite flag in the extension API)
MozReview-Commit-ID: HLfte7x1X7T
--HG--
extra : rebase_source : 1feb84ceca157d8c5ec8575c6336cc606c3504fe
After writing a unit test I discovered that updating a cookie's samesite
flag did not work. This is caused by an "optimization" that avoids
modifying a cookie if any of the cookie attributes were not changed.
This check did not account for the SameSite flag, until now.
A unit test for this will be added in a later commit, at
toolkit/components/extensions/test/xpcshell/test_ext_cookies_samesite.js
MozReview-Commit-ID: ChiwwqqOE57
--HG--
extra : rebase_source : f6bd9bd650f6db50a0726451cd781ca7984962a1
This patch is an automatic replacement of s/NS_NOTREACHED/MOZ_ASSERT_UNREACHABLE/. Reindenting long lines and whitespace fixups follow in patch 6b.
MozReview-Commit-ID: 5UQVHElSpCr
--HG--
extra : rebase_source : 4c1b2fc32b269342f07639266b64941e2270e9c4
extra : source : 907543f6eae716f23a6de52b1ffb1c82908d158a
This patch reverts parts of changeset e87e706def11 (bug 1425031).
The problem in bug 1425031 was that when the content process set a cookie
a notification was sent to the parent process. This notification was then
forwarded to all the content processes, including the one it originated from.
The solution was to not forward cookies that originated from a content
process, but this causes the current bug.
The correct fix is to forward the cookie changes to all content processes
except the one they originated from.
The test for bug 1425031 remains, and should keep passing.
MozReview-Commit-ID: 1P6JwHQDy93
--HG--
extra : rebase_source : 85845c93059004836e14d5a46f2df881237fad6e
There seems to be no reason to conditionally fire the cookie-db-read event. Currently it is not fired if no cookies were read. There seems to be only one other consumer of this event (a test) which should work fine if the event were fired every time. This change would eliminate a particularly ugly workaround in cookie-related policy testing.
MozReview-Commit-ID: FbD1cvsBZBO
--HG--
extra : rebase_source : 6611debb3567310c61e5a5dc9cedadeae888cfe5
There seems to be no reason to conditionally fire the cookie-db-read event. Currently it is not fired if no cookies were read. There seems to be only one other consumer of this event (a test) which should work fine if the event were fired every time. This change would eliminate a particularly ugly workaround in cookie-related policy testing.
MozReview-Commit-ID: FbD1cvsBZBO
--HG--
extra : rebase_source : ff5049f36c7f3df3ad182ebb1a6ccc5db1032e23
This patch was autogenerated by my decomponents.py
It covers almost every file with the extension js, jsm, html, py,
xhtml, or xul.
It removes blank lines after removed lines, when the removed lines are
preceded by either blank lines or the start of a new block. The "start
of a new block" is defined fairly hackily: either the line starts with
//, ends with */, ends with {, <![CDATA[, """ or '''. The first two
cover comments, the third one covers JS, the fourth covers JS embedded
in XUL, and the final two cover JS embedded in Python. This also
applies if the removed line was the first line of the file.
It covers the pattern matching cases like "var {classes: Cc,
interfaces: Ci, utils: Cu, results: Cr} = Components;". It'll remove
the entire thing if they are all either Ci, Cr, Cc or Cu, or it will
remove the appropriate ones and leave the residue behind. If there's
only one behind, then it will turn it into a normal, non-pattern
matching variable definition. (For instance, "const { classes: Cc,
Constructor: CC, interfaces: Ci, utils: Cu } = Components" becomes
"const CC = Components.Constructor".)
MozReview-Commit-ID: DeSHcClQ7cG
--HG--
extra : rebase_source : d9c41878036c1ef7766ef5e91a7005025bc1d72b
This was done using the following script:
37e3803c7a/processors/chromeutils-import.jsm
MozReview-Commit-ID: 1Nc3XDu0wGl
--HG--
extra : source : 12fc4dee861c812fd2bd032c63ef17af61800c70
extra : intermediate-source : 34c999fa006bffe8705cf50c54708aa21a962e62
extra : histedit_source : b2be2c5e5d226e6c347312456a6ae339c1e634b0
This was done using the following script:
37e3803c7a/processors/chromeutils-import.jsm
MozReview-Commit-ID: 1Nc3XDu0wGl
--HG--
extra : source : 12fc4dee861c812fd2bd032c63ef17af61800c70
This was done using the following script:
37e3803c7a/processors/chromeutils-import.jsm
MozReview-Commit-ID: 1Nc3XDu0wGl
--HG--
extra : rebase_source : c004a023389f1f6bf3d2f3efe93c13d423b23ccd
This new COOKIE_SCHEME_HTTPS telemetry probe reports the same information as the COOKIE_SCHEME_SECURITY probe, but also categories cookies by whether they are set from an HTTP or HTTPS origin.
MozReview-Commit-ID: IWg8dycCzwq
--HG--
extra : source : 94708be3f00796680377b3235b78f7db70c34510
extra : intermediate-source : eaf32e92b13d54a8e8d70a7b8caf420800641d49
"Nonsecure HTTP" here just means regular, not-HTTPS HTTP. It doesn't mean HTTPS without the `Secure` cookie flag. Honor the expiration time of third-party cookies set over HTTPS, whether or not they have the `Secure` cookie flag. If a third-party cookie is set over HTTPS and then later sent in nonsecure HTTP request (which is allowed for cookies without the `Secure` cookie flag), the cookie won't be turned into a session cookie unless the nonsecure HTTP response sets a new cookie value.
This feature is controlled by the pref "network.cookie.thirdparty.nonsecureSessionOnly".
MozReview-Commit-ID: HlCg21JyvNC
--HG--
rename : extensions/cookie/test/unit/test_cookies_thirdparty_session.js => extensions/cookie/test/unit/test_cookies_thirdparty_nonsecure_session.js
extra : source : d1be2e4265201efd3ee93e965ac68561f548fd05
extra : intermediate-source : f5b382fa1b70e30a907b1f10d74f8c0c6dff344e
The NS_LITERAL_CSTRING macro creates a temporary nsLiteralCString to encapsulate the string literal and its length, but AssignLiteral() can determine the string literal's length at compile-time without nsLiteralCString.
MozReview-Commit-ID: DbTW5Bhd9E1
--HG--
extra : rebase_source : b27f666e5ca832d814fb6846208474e1ec66e5f4
extra : source : 9ff4e11402a9a43ed90298a9c354b0164cf9414f
I noticed a bug where the following can happen. The parent sends a
TrackCookiesLoad message followed by an HTTP OnStartRequest
message. When these messages are received in the child, the
TrackCookiesLoad message goes in the SystemGroup event queue and the
OnStartRequest message goes in the event queue for the relevant
tab. Unfortunately, this means that the OnStartRequest message could
run first since the queues have no guaranteed ordering.
We really should be putting the TrackCookiesLoad message in the same
queue that the OnStartRequest message goes in. I worked on that a
little bit, but it's hard to get right. For now, I would like to leave
the cookie message unlabeled. Any unlabeled message/event is totally
ordered with respect to all other messages/events, so this fixes the
bug.
MozReview-Commit-ID: KiLDAhlrbB8
These methods return an addrefed raw pointer, which makes them easy to use in
ways that cause leaks. If they're to continue returning an addrefed pointer,
they should explicitly return an already_AddRefed.
This also switches to StaticRefPtr with ClearOnShutdown for the cached
pointers for the sake of sanity.
MozReview-Commit-ID: D0lDpU8Hqug
--HG--
extra : rebase_source : 7b199070805fc0472eaf8409932517700ed23d49
We have a minimum requirement of VS 2015 for Windows builds, which supports
the z length modifier for format specifiers. So we don't need SizePrintfMacros.h
any more, and can just use %zu and friends directly everywhere.
MozReview-Commit-ID: 6s78RvPFMzv
--HG--
extra : rebase_source : 009ea39eb4dac1c927aa03e4f97d8ab673de8a0e
All the instances are converted as follows.
- nsAFlatString --> nsString
- nsAFlatCString --> nsCString
--HG--
extra : rebase_source : b37350642c58a85a08363df2e7c610873faa6e41
All the instances are converted as follows.
- nsASingleFragmentString --> nsAString
- nsASingleFragmentCString --> nsACString
--HG--
extra : rebase_source : e6a1ddc8938fecd9a735b15e872c054edf4c7910
Evaluated every method exposed via nsICookieManager and
nsICookieManager2, and ensured that every method uses the right cookie
database depending on whether the mPrivateBrowsingId flag is set in
OriginAttributes.
The usePrivateMode method has been removed since it was introduced in
bugzil.la/777620 and not used anywhere else.
This should be covered by
toolkit/components/extensions/test/mochitest/test_ext_cookies.html,
but due to bugzil.la/1309637, the relevant private browsing tests
have been disabled. These tests will be enabled in one of the next
commits.
MozReview-Commit-ID: DMhE42LGu9S
--HG--
extra : rebase_source : 7882ed7610ff4c7c6f81a7852660d2b36a5c0a92
Avoids a thread-safety race condition on shutdown where we could try to finalize a statement twice.
Allows the async thread to be referenced until ShutdownAsyncThread, so async finalizers can make use of it.
Removes the no more useful mAsyncExecutionThreadIsAlive.
Nullifies the sqlite3_mutex pointer when the connection is closed, since it would be a dangling pointer.
Use a ScopeExit to ensure the connection and the mutex pointers are always nullified on failure.
Makes asyncClose bailout early if a Close method was already invoked before.
Makes AsyncInitDatabase not use AsyncClose to just shutdown the async thread.
Fixes various unfinalized statements in consumers code.
Makes mConnectionClosed better follow mDBConn status.
Replaces some mutex locking isClosed() calls with lockless isConnectionReadyOnThisThread.
MozReview-Commit-ID: 6sftFehsQTt
--HG--
extra : rebase_source : 2bf3088b500376e58e62e8f078d9950588adc649
Change mozilla::Smprintf and friends to return a UniquePtr, rather than
relying on manual memory management. (Though after this patch there are
still a handful of spots needing SmprintfFree.)
MozReview-Commit-ID: COa4nzIX5qa
--HG--
extra : rebase_source : ab4a11b4d2e758099bd0794d5c25d799a7e42680
This patch removes support for mozapp iframes, leaving support for
mozbrowser iframes intact. Some of the code has been rewritten in order
to phrase things in terms of mozbrowser only, as opposed to mozbrowser
or app. In some places, code that was only useful with apps has been
completely removed, so that the APIs consumed can also be removed. In
some places where the notion of appId was bleeding out of this API, now
we use NO_APP_ID. Other notions of appId which were restricted to this
API have been removed.
We will use the new type for the generated IPDL message handler
prototype to make sure correct error handling method is called.
MozReview-Commit-ID: AzVbApxFGZ0
SerializedLoadContext carries the origin attributes of the docshell to
parent process. However document could have different origin attributes
than the docshell's. And the origin attributes of the document comes
from the LoadInfo of the channel.
So we replace SerializedLoadContext with NeckoOriginAttributes, which is from
the loadInfo of the channel, therefore parent can get the correct origin
attributes from the document.
If no expired cookies exist, in order of preference, evict the oldest:
* session cookie with a non-matching path
* session cookie with a matching path
* non-session cookie with a non-matching path
* non-session cookie with a matching path
This replaces the previous heuristic of evicting the oldest cookie, irregardless of any other attributes,
if no expired cookies were present. This ensures that cookies that are already considered transient by
web applications will be removed first, followed by cookies that are unrelated to the response that is
adding new cookies.
* * *
Bug 1264192 - Interdiff
If no expired cookies exist, in order of preference, evict the oldest:
* session cookie with a non-matching path
* session cookie with a matching path
* non-session cookie with a non-matching path
* non-session cookie with a matching path
This replaces the previous heuristic of evicting the oldest cookie, irregardless of any other attributes,
if no expired cookies were present. This ensures that cookies that are already considered transient by
web applications will be removed first, followed by cookies that are unrelated to the response that is
adding new cookies.
--HG--
extra : amend_source : f79ba9dd393a3f37760e643b10b7137e37a6397a
The new name makes the sense of the condition much clearer. E.g. compare:
NS_WARN_IF_FALSE(!rv.Failed());
with:
NS_WARNING_ASSERTION(!rv.Failed());
The new name also makes it clearer that it only has effect in debug builds,
because that's standard for assertions.
--HG--
extra : rebase_source : 886e57a9e433e0cb6ed635cc075b34b7ebf81853
This patch removes checking of all the callback calls in memory reporter
CollectReport() functions, because it's not useful.
The patch also does some associated clean-up.
- Replaces some uses of nsIMemoryReporterCallback with the preferred
nsIHandleReportCallback typedef.
- Replaces aCallback/aCb/aClosure with aHandleRepor/aData for CollectReports()
parameter names, for consistency.
- Adds MOZ_MUST_USE/[must_use] in a few places in nsIMemoryReporter.idl.
- Uses the MOZ_COLLECT_REPORT macro in all suitable places.
Overall the patch reduces code size by ~300 lines and reduces the size of
libxul by about 37 KiB on my Linux64 builds.
--HG--
extra : rebase_source : e94323614bd10463a0c5134a7276238a7ca1cf23
The patch is generated from following command:
rgrep -l unused.h|xargs sed -i -e s,mozilla/unused.h,mozilla/Unused.h,
MozReview-Commit-ID: AtLcWApZfES
--HG--
rename : mfbt/unused.h => mfbt/Unused.h
This patch makes the following changes on many in-class methods.
- NS_IMETHODIMP F() override; --> NS_IMETHOD F() override;
- NS_IMETHODIMP F() override {...} --> NS_IMETHOD F() override {...}
- NS_IMETHODIMP F() final; --> NS_IMETHOD F() final;
- NS_IMETHODIMP F() final {...} --> NS_IMETHOD F() final {...}
Using NS_IMETHOD is the preferred way of marking in-class virtual methods.
Although these transformations add an explicit |virtual|, they are safe --
there's an implicit |virtual| anyway because |override| and |final| only work
with virtual methods.
--HG--
extra : rebase_source : 386ee4e4ea2ecd8d5001efabc3ac87b4d6c0659f
This removes the unnecessary setting of c-basic-offset from all
python-mode files.
This was automatically generated using
perl -pi -e 's/; *c-basic-offset: *[0-9]+//'
... on the affected files.
The bulk of these files are moz.build files but there a few others as
well.
MozReview-Commit-ID: 2pPf3DEiZqx
--HG--
extra : rebase_source : 0a7dcac80b924174a2c429b093791148ea6ac204
NSPR_LOG_MODULES is deprecated.
Programs in the test directory still use PR_NewLogModule,
so NSPR_LOG_MODULES is accurate for them. Getting these
to link against libxul for LazyLogModule is still to do,
so leave those for now.
MozReview-Commit-ID: 3uMR3YPJyyi
--HG--
extra : rebase_source : a94c6392cb0de1b6d6324b192954f48f784e8cbe
It's an annotation that is used a lot, and should be used even more, so a
shorter name is better.
MozReview-Commit-ID: 1VS4Dney4WX
--HG--
extra : rebase_source : b26919c1b0fcb32e5339adeef5be5becae6032cf
Previously, every test and support file would be synced to the objdir
when running any test. Now that only those support files and tests requested
are synced, we note support files required beyond those in a test's
directory in ini manifests.
MozReview-Commit-ID: EmlDz9d4lqt
This change renames OriginAttributes.mInBrowser to mInIsolatedMozBrowser and
nsIPrincipal::GetIsInBrowserElement to GetIsInIsolatedMozBrowserElement. Other
methods that pass these values around also have name changes.
Tokens such as "inBrowser" have previously been serialized into cache keys, used
as DB column names, stored in app registries, etc. No changes are made to any
serialization formats. Only runtime method and variable names are updated.
No behavior changes are made in this patch, so some renamed methods may have
nonsensical implementations. These are corrected in subsequent patches
focused on behavior.
MozReview-Commit-ID: 66HfMlsXFLs