Closed Bug 1192940 Opened 9 years ago Closed 7 years ago

sendBeacon does not honor meta referrer policy

Categories

(Core :: DOM: Core & HTML, defect, P2)

Product:
Core
Component:
DOM: Core & HTML
Version:
39 Branch
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox53 --- fixed
firefox54 --- fixed
firefox55 --- verified

People

(Reporter: patrick.toomey, Assigned: tnguyen)

Details

Attachments

(3 files, 1 obsolete file)

Bug 1192940 - Support referrer policy in sendBeacon
59 bytes, text/x-review-board-request
ehsan.akhgari
: review+
Details
Support referrer policy in sendBeacon - Aurora
20.62 KB, patch
gchang
: approval-mozilla-aurora+
Details | Diff | Splinter Review
Support referrer policy in sendBeacon-beta
18.03 KB, patch
lizzard
: approval-mozilla-beta+
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.130 Safari/537.36

Steps to reproduce:

1) Go to a site that has a meta referrer policy like (be sure you visit a page that has a subpath to show that the subpath is sent in the referrer):
    
    <meta content="origin" name="referrer" />
  
2) open the dev console and paste something like the following:
  
   navigator.sendBeacon("https://www.google-analytics.com/collect", "foo=bar");




Actual results:

Go to the network tab and observe that the full referrer is sent despite the fact that the site has an "origin" referrer policy set.


Expected results:

sendBeacon should respect the referrer policy the same way that any other XHR request would.
A quick proof of concept page:

http://biasedcoin.com/meta-referrer-tests/origin.html

var http = new XMLHttpRequest();
var url = "https://www.google-analytics.com/collect";
var params = "foo=bar";
http.open("POST", url, true);
http.send(params);

sends: Referer:"http://biasedcoin.com"

navigator.sendBeacon("https://www.google-analytics.com/collect", "foo=bar");

sends: Referer:"http://biasedcoin.com/meta-referrer-tests/origin.html"
Component: Untriaged → DOM
Product: Firefox → Core
Priority: -- → P2
Thomas, do you have any time to look at this one?  Its pretty clear we violate the spec here and it makes sendBeacon() less useful.  It seems sites like github are having compat issues with this.

It seems like this could be a fairly easy fix.  We need to add a WPT test, though.
Flags: needinfo?(tnguyen)
The issue is manifesting itself in sending sensitive path parameters to our analytics provider.
Assignee: nobody → tnguyen
Flags: needinfo?(tnguyen)
Attachment #8846936 - Flags: review?(ehsan)
Hi Ehsan,
Could you please take a look at this?
AFAIK, sendBeacon is not working in worker at the moment, so just use document referrer's policy when sending beacon.
I added some wpt tests, but have to move some of them to /mozilla/. The reason is we have to set some prefs to disable mixed content block to allow request downgrade from https -> http. I'm not sure we should push a WPT test that requires non-standard prefs like that
Comment on attachment 8846936 [details]
Bug 1192940 - Support referrer policy in sendBeacon

https://reviewboard.mozilla.org/r/120016/#review122072

Thanks for the fix!

I think this is probably worth backporting to branches, given that the fix should be low risk and it makes us honour the referrer policy which is important.  Can you please nominate this for an uplift?  Thanks!
Attachment #8846936 - Flags: review?(ehsan) → review+
(In reply to Thomas Nguyen[:tnguyen] ni plz from comment #5)
> Hi Ehsan,
> Could you please take a look at this?
> AFAIK, sendBeacon is not working in worker at the moment, so just use
> document referrer's policy when sending beacon.
> I added some wpt tests, but have to move some of them to /mozilla/. The
> reason is we have to set some prefs to disable mixed content block to allow
> request downgrade from https -> http. I'm not sure we should push a WPT test
> that requires non-standard prefs like that

There are other similar tests already, for example: <http://searchfox.org/mozilla-central/rev/a5c2b278897272497e14a8481513fee34bbc7e2c/testing/web-platform/meta/service-workers/service-worker/fetch-event.https.html.ini#3>
Keywords: checkin-needed
Comment on attachment 8848376 [details] [diff] [review]
Support referrer policy in sendBeacon - Aurora

Approval Request Comment
[Feature/Bug causing the regression]: No
[User impact if declined]: Leak referrer when sendbeacon
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: See comment 0
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: Low risky
[Why is the change risky/not risky?]: User referrer policy of doc instead of default
[String changes made/needed]: No
Attachment #8848376 - Flags: approval-mozilla-aurora?
Attachment #8848376 - Attachment description: Support referrer policy in sendBeacon → Support referrer policy in sendBeacon - Aurora
Comment on attachment 8848381 [details] [diff] [review]
Support referrer policy in sendBeacon-beta

Approval Request Comment
[Feature/Bug causing the regression]: no
[User impact if declined]: leak referrer when sendbeacon
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: comment 0
[List of other uplifts needed for the feature/fix]: aurora
[Is the change risky?]: low risky
[Why is the change risky/not risky?]: Change correct referrer when sendbeacon
[String changes made/needed]: no
Attachment #8848381 - Attachment description: Support referrer policy in sendBeacon → Support referrer policy in sendBeacon-beta
Attachment #8848381 - Flags: approval-mozilla-beta?
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/eb872c42dc7f
Support referrer policy in sendBeacon r=Ehsan
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/eb872c42dc7f
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(brindusa.tot)
Doesn't seem to apply
grafting 386618:eb872c42dc7f "Bug 1192940 - Support referrer policy in sendBeacon r=Ehsan"
merging dom/base/Navigator.cpp
merging testing/web-platform/meta/MANIFEST.json
merging testing/web-platform/mozilla/meta/MANIFEST.json
warning: conflicts while merging dom/base/Navigator.cpp! (edit, then use 'hg resolve --mark')
warning: conflicts while merging testing/web-platform/meta/MANIFEST.json! (edit, then use 'hg resolve --mark')
warning: conflicts while merging testing/web-platform/mozilla/meta/MANIFEST.json! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(tnguyen)
(In reply to Sylvestre Ledru [:sylvestre] from comment #18)
> Doesn't seem to apply
> grafting 386618:eb872c42dc7f "Bug 1192940 - Support referrer policy in
> sendBeacon r=Ehsan"
> merging dom/base/Navigator.cpp
> merging testing/web-platform/meta/MANIFEST.json
> merging testing/web-platform/mozilla/meta/MANIFEST.json
> warning: conflicts while merging dom/base/Navigator.cpp! (edit, then use 'hg
> resolve --mark')
> warning: conflicts while merging testing/web-platform/meta/MANIFEST.json!
> (edit, then use 'hg resolve --mark')
> warning: conflicts while merging
> testing/web-platform/mozilla/meta/MANIFEST.json! (edit, then use 'hg resolve
> --mark')
> abort: unresolved conflicts, can't continue
> (use 'hg resolve' and 'hg graft --continue')

Which branch did you fail to merge? aurora, beta?
It seems ok to me,  comment 12 for aurora, and comment 14 for beta
I was able to reproduce the bug on a Nightly 54.0a1, Build ID: 20170210030206.

I can confirm this is fixed on latest Nightly build 55.0a1, Build ID 20170320030209, on Windows 10x64, Mac OS X 10.10 and Ubuntu 16.04. Verified on site
http://biasedcoin.com/meta-referrer-tests/origin.html

and navigator.sendBeacon("https://www.google-analytics.com/collect", "foo=bar");
 sends only: Referer:"http://biasedcoin.com".
Status: RESOLVED → VERIFIED
status-firefox55: fixed → verified
Flags: needinfo?(brindusa.tot)
Comment on attachment 8848376 [details] [diff] [review]
Support referrer policy in sendBeacon - Aurora

Fix sendBeacon API and was verified. Aurora54+ & Beta53+.
Attachment #8848376 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8848381 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
sorry had to back this out from beta for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=85588158&repo=mozilla-beta
status-firefox53: fixed → affected
Flags: needinfo?(tnguyen)
My bad, we are testing a script, sorry!
Flags: needinfo?(sledru)
MozReview-Commit-ID: K9szPNxXtrf
(In reply to Carsten Book [:Tomcat] from comment #25)
> sorry had to back this out from beta for failures like
> https://treeherder.mozilla.org/logviewer.html#?job_id=85588158&repo=mozilla-
> beta

That is wpt json conflict. Ran ./mach wpt-manifest-update and it should be ok now
Flags: needinfo?(tnguyen)
Attachment #8848381 - Attachment is obsolete: true
Comment on attachment 8850460 [details] [diff] [review]
Support referrer policy in sendBeacon-beta


Approval Request Comment
[Feature/Bug causing the regression]: no
[User impact if declined]: leak referrer when sendbeacon
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: comment 0
[List of other uplifts needed for the feature/fix]: aurora
[Is the change risky?]: low risky
[Why is the change risky/not risky?]: Change correct referrer when sendbeacon
[String changes made/needed]: no

Request beta uplift again
Attachment #8850460 - Attachment description: Support referrer policy in sendBeacon r=ehsan, a=gchang → Support referrer policy in sendBeacon-beta
Attachment #8850460 - Flags: approval-mozilla-beta?
Comment on attachment 8850460 [details] [diff] [review]
Support referrer policy in sendBeacon-beta

Retroactive beta re-approval!
Attachment #8850460 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: