Status

()

defect
P3
normal
4 years ago
2 years ago

People

(Reporter: sicking, Assigned: sicking)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [necko-backlog])

Attachments

(5 attachments)

Now that channel creation usually has access to the loading document or principal, we can use that information to set the referrer.

We will still need to let individual callsites override that default referrer, but more often than not some default logic should work fine.
Hi Jonas, Does this replace the referrer with triggering Principal?  Or is this just about referrer policy?

Is this the patch I should be looking at https://hg.mozilla.org/try/rev/6c64e5de432c ?

Thanks!
Yes, this grabs the referrer from the triggering principal. And the referrer-policy from the loadingNode.

It's still too early to look at patches though as they are still failing a number of tests. But yes, the one you linked to is the latest version.
Comment on attachment 8690662 [details] [diff] [review]
Part 1: Automatically set a default referrer when creating http channels. Use default policy in XHR, fetch() and plugins. r=ckerschb

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

In general I am quite happy with the approach of centralizing the referrerPolicy within CSP. One downside (not even sure if that is really a downside) is that CSP functionality of ::toString() or also ::toJSON() will now potentially include a referrer policy that might not have been set through meta CSP but rather through meta referrer. Anyway, I think we can live with that.

The thing I don't fully understand is, why are you having these 'else' branches and explicitly set the referrer to a nullptr (e.g. in SendPing). Wouldn't it make more sense to initialize mReferrer to a nullptr in the constructor of the httpbaseChannel?

I think the renaming of the referrerpolicy-patch landed in the meantime. Can you please rebase the patches and I'll have those patches reviewed in no time.

::: ipc/glue/BackgroundUtils.cpp
@@ +228,5 @@
> +    triggeringPrincipalInfo = info;
> +  }
> +  else {
> +    triggeringPrincipalInfo = void_t();
> +  }

What is the advantage of using OptionalPrincipalInfo? The triggeringPrincipal will always be set to the loadingPrincipal in the LoadInfo constructor and hence will never be null, no?

::: netwerk/base/LoadInfo.cpp
@@ +536,5 @@
>  }
>  
> +already_AddRefed<nsIURI>
> +LoadInfo::GetReferrerAndPolicy(ReferrerPolicy* aPolicy)
> +{

As discussed in person, it might make more sense to have that functionality exposed as a static function on the ioservice (or also on the contentSecurityManager).
Attachment #8690662 - Flags: review?(mozilla) → feedback+
Comment on attachment 8690663 [details] [diff] [review]
Part 2: Use the default referrer in EventSource, sendBeacon, <object>, HTMLMediaElement, and XSLT

As discussed over IRC, clearing those flags for now.
Attachment #8690663 - Flags: review?(mozilla)
Whiteboard: [necko-backlog]
Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: -- → P1
Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: P1 → P3
You need to log in before you can comment on or make changes to this bug.