Closed Bug 1487600 Opened Last year Closed Last year

Report to console any content blocking

Categories

(Core :: Security, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla64
Tracking Status
firefox63 --- verified
firefox64 --- verified

People

(Reporter: baku, Assigned: baku)

References

Details

Attachments

(1 file)

Would be nice to have something written in the console when cookie policy blocks some content/cookie/storage access.
Attached patch console.patchSplinter Review
Attachment #9005551 - Flags: review?(ehsan)
Comment on attachment 9005551 [details] [diff] [review]
console.patch

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

r=me with the below comments addressed.

::: netwerk/locales/en-US/necko.properties
@@ +41,5 @@
>  AutomaticAuth=You are about to log in to the site “%1$S” with the username “%2$S”.
>  
>  TrackerUriBlocked=The resource at “%1$S” was blocked because content blocking is enabled.
>  UnsafeUriBlocked=The resource at “%1$S” was blocked by Safe Browsing.
> +CookieBlockedByPermission=Content blocked on “%1$S” because of custom cookie policy permission.

Nit: please explain exactly what happened, like:

  Request to access cookies or storage on “%1$S” was blocked because of custom cookie permission.

@@ +42,5 @@
>  
>  TrackerUriBlocked=The resource at “%1$S” was blocked because content blocking is enabled.
>  UnsafeUriBlocked=The resource at “%1$S” was blocked by Safe Browsing.
> +CookieBlockedByPermission=Content blocked on “%1$S” because of custom cookie policy permission.
> +CookieBlockedTracker=Content blocked on “%1$S” because the resource has been marked as tracker.

Request to access cookie or storage on “%1$S” was blocked because it came from a tracker and content blocking is enabled.

@@ +43,5 @@
>  TrackerUriBlocked=The resource at “%1$S” was blocked because content blocking is enabled.
>  UnsafeUriBlocked=The resource at “%1$S” was blocked by Safe Browsing.
> +CookieBlockedByPermission=Content blocked on “%1$S” because of custom cookie policy permission.
> +CookieBlockedTracker=Content blocked on “%1$S” because the resource has been marked as tracker.
> +CookieBlockedAll=Content blocked on “%1$S” because cookie policy blocks any storage access.

Request to access cookie or storage on “%1$S” was blocked because we're blocking all storage access requests.

@@ +44,5 @@
>  UnsafeUriBlocked=The resource at “%1$S” was blocked by Safe Browsing.
> +CookieBlockedByPermission=Content blocked on “%1$S” because of custom cookie policy permission.
> +CookieBlockedTracker=Content blocked on “%1$S” because the resource has been marked as tracker.
> +CookieBlockedAll=Content blocked on “%1$S” because cookie policy blocks any storage access.
> +CookieBlockedForeign=Content blocked on “%1$S” because cookie policy blocks any storage access from third party contexts.

Request to access cookie or storage on “%1$S” was blocked because we're blocking all third-party storage access requests and content blocking is enabled.

@@ +45,5 @@
> +CookieBlockedByPermission=Content blocked on “%1$S” because of custom cookie policy permission.
> +CookieBlockedTracker=Content blocked on “%1$S” because the resource has been marked as tracker.
> +CookieBlockedAll=Content blocked on “%1$S” because cookie policy blocks any storage access.
> +CookieBlockedForeign=Content blocked on “%1$S” because cookie policy blocks any storage access from third party contexts.
> +CookieBlockedSlowTrackingContent=Content blocked on “%1$S” because the resource has been marked as slow tracker.

The resource at “%1$S” was blocked because content blocking is enabled and the resource was classified as a slow tracking resource.

::: toolkit/components/antitracking/AntiTrackingCommon.cpp
@@ +197,5 @@
> +             aRejectedReason == nsIWebProgressListener::STATE_COOKIES_BLOCKED_TRACKER ||
> +             aRejectedReason == nsIWebProgressListener::STATE_COOKIES_BLOCKED_ALL ||
> +             aRejectedReason == nsIWebProgressListener::STATE_COOKIES_BLOCKED_FOREIGN ||
> +             aRejectedReason == nsIWebProgressListener::STATE_BLOCKED_SLOW_TRACKING_CONTENT);
> +

Please check the content blocking pref here and bail out early if it is not set.

@@ +215,5 @@
> +      message = "CookieBlockedByPermission";
> +      break;
> +
> +    case nsIWebProgressListener::STATE_COOKIES_BLOCKED_TRACKER:
> +      message = "CookieBlockedTracker=";

typo?
Attachment #9005551 - Flags: review?(ehsan) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/77566fd9183f
Report to console any content blocking, r=ehsan
Backout by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/453c5173e50f
Backed out changeset 77566fd9183f for assertion failures on AntiTrackingCommon.cpp:200 on a CLOSED TREE
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/16c75edf53e2
Report to console any content blocking, r=ehsan
https://hg.mozilla.org/mozilla-central/rev/16c75edf53e2
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Duplicate of this bug: 1485843
Comment on attachment 9005551 [details] [diff] [review]
console.patch

Approval Request Comment
[Feature/Bug causing the regression]: This is not a regression. It's a feature we would like to have in beta for the content blocking/fastblock/anti-tracking shield study.
[User impact if declined]: Hard to know why/when something has been blocked.
[Is this code covered by automated tests?]: n/a
[Has the fix been verified in Nightly?]: n/a
[Needs manual test from QE? If yes, steps to reproduce]: n/a 
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: It introduces console messages when something is blocked because of fastblocking, or cookie policy.
[String changes made/needed]: 5, but they are shown just in the browser console.
Attachment #9005551 - Flags: approval-mozilla-beta?
Comment on attachment 9005551 [details] [diff] [review]
console.patch

Checked with flod, the new strings are not a problem. Approved for 63 beta 4
Attachment #9005551 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QE, please verify this thanks.
Flags: qe-verify+
I verified this issue on Nightly 64.0a1 (2018-09-07), Firefox 63.0b4 and all the logs appear as expected.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
There are websites that keep triggering this warning non-stop. Is this of any concern? If yes I can file a bug. One example that I have here is a gizmodo tab that puts around 5 warnings of this per second, but there are other open tabs doing the same.

One problem is that it gets hit from different call sites on their code, so it's not aggregated together because at least the line number differs.
Yes, please file a bug with more info on how to reproduce.  We probably shouldn't make the console unusable for other purposes.  :-)
Depends on: 1493361
See Also: → 1546581
You need to log in before you can comment on or make changes to this bug.