Open
Bug 1220346
Opened 9 years ago
Updated 2 years ago
Move referrer-setting into necko
Categories
(Core :: Networking: HTTP, defect, P3)
Core
Networking: HTTP
Tracking
()
NEW
People
(Reporter: sicking, Unassigned)
References
Details
(Whiteboard: [necko-backlog])
Attachments
(5 files)
46.70 KB,
patch
|
ckerschb
:
feedback+
|
Details | Diff | Splinter Review |
14.95 KB,
patch
|
Details | Diff | Splinter Review | |
7.02 KB,
patch
|
Details | Diff | Splinter Review | |
2.05 KB,
patch
|
Details | Diff | Splinter Review | |
13.88 KB,
patch
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•9 years ago
|
||
Reporter | ||
Comment 2•9 years ago
|
||
Reporter | ||
Comment 3•9 years ago
|
||
Reporter | ||
Comment 4•9 years ago
|
||
Reporter | ||
Comment 5•9 years ago
|
||
Comment 6•9 years ago
|
||
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!
Reporter | ||
Comment 7•9 years ago
|
||
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.
Reporter | ||
Comment 8•9 years ago
|
||
Reporter | ||
Comment 9•9 years ago
|
||
Reporter | ||
Comment 10•9 years ago
|
||
Assignee: nobody → jonas
Attachment #8690662 -
Flags: review?(mozilla)
Reporter | ||
Comment 11•9 years ago
|
||
Attachment #8690663 -
Flags: review?(mozilla)
Reporter | ||
Comment 12•9 years ago
|
||
Attachment #8690664 -
Flags: review?(mozilla)
Reporter | ||
Comment 13•9 years ago
|
||
Attachment #8690665 -
Flags: review?(mozilla)
Reporter | ||
Comment 14•9 years ago
|
||
Attachment #8690667 -
Flags: review?(mozilla)
Comment 15•9 years ago
|
||
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 16•9 years ago
|
||
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)
Updated•9 years ago
|
Attachment #8690664 -
Flags: review?(mozilla)
Updated•9 years ago
|
Attachment #8690665 -
Flags: review?(mozilla)
Updated•9 years ago
|
Attachment #8690667 -
Flags: review?(mozilla)
Updated•9 years ago
|
Whiteboard: [necko-backlog]
Comment 17•7 years ago
|
||
Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: -- → P1
Comment 18•7 years ago
|
||
Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: P1 → P3
Updated•2 years ago
|
Severity: normal → S3
Comment 19•2 years ago
|
||
The bug assignee is inactive on Bugzilla, so the assignee is being reset.
Assignee: jonas → nobody
You need to log in
before you can comment on or make changes to this bug.
Description
•