Closed Bug 1480780 Opened Last year Closed Last year

Use network.cookie.cookieBehavior prefs for the new cookie restriction project

Categories

(Core :: DOM: Security, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: baku, Assigned: baku)

References

(Blocks 1 open bug)

Details

(Whiteboard: [domsecurity-backlog1] [domsecurity-active])

Attachments

(1 file)

This bug is about:

1. introducing nsICookieService::BEHAVIOR_REJECT_TRACKER cookie policy.
2. supporting the overwrite of that new cookie behavior via cookie permissions.
Status: NEW → ASSIGNED
Whiteboard: [domsecurity-backlog1] [domsecurity-active]
No longer depends on: 1480131
Attached patch mergePref.patchSplinter Review
Comment on attachment 8997428 [details] [diff] [review]
mergePref.patch

It breaks a couple of ServiceWorker tests, but I haven't investigated why yet. I suspect is an easy fix...
Attachment #8997428 - Flags: review?(ehsan)
Comment on attachment 8997428 [details] [diff] [review]
mergePref.patch

Review of attachment 8997428 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/cookie/nsCookieService.cpp
@@ +2410,5 @@
>  nsCookieService::PrefChanged(nsIPrefBranch *aPrefBranch)
>  {
>    int32_t val;
>    if (NS_SUCCEEDED(aPrefBranch->GetIntPref(kPrefCookieBehavior, &val)))
> +    mCookieBehavior = (uint8_t) LIMIT(val, 0, nsICookieService::BEHAVIOR_REJECT_TRACKER, 0);

Instead of this, please define a new constant, BEHAVIOR_LAST and use it here.

::: toolkit/components/antitracking/AntiTrackingCommon.cpp
@@ +89,5 @@
> +
> +  nsCOMPtr<nsICookiePermission> cps =
> +    do_GetService(NS_COOKIEPERMISSION_CONTRACTID);
> +  if (NS_WARN_IF(!cps)) {
> +    return 1;

I think in error situations, it is better to return unknown...

@@ +379,5 @@
> +  }
> +
> +  if (behavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN ||
> +      behavior == nsICookieService::BEHAVIOR_LIMIT_FOREIGN) {
> +    return false;

Nit: please add a TODO entry here for doing something more sophisticated for BEHAVIOR_LIMIT_FOREIGN...
Attachment #8997428 - Flags: review?(ehsan) → review+
I addressed the review comments myself, and fixed the test failures on try.  I'm now going to land this patch...
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e70b1d7ba1eb
Merge the privacy.3rdpartystorage.enabled pref with the network.cookie.cookieBehavior pref; r=ehsan
Depends on: 1482999
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5d982e6568b
follow-up: Fix the build bustage on a CLOSED TREE
Depends on: 1483035
https://hg.mozilla.org/mozilla-central/rev/e70b1d7ba1eb
https://hg.mozilla.org/mozilla-central/rev/e5d982e6568b
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Depends on: 1483765
Depends on: 1486184
Duplicate of this bug: 1480197
Depends on: 1489624
Depends on: 1492769
Depends on: 1493211
Depends on: 1530132
No longer depends on: 1545909
You need to log in before you can comment on or make changes to this bug.