Closed
Bug 1496742
Opened 6 years ago
Closed 6 years ago
Allow to display Referrer Policy for a given request
Categories
(DevTools :: Netmonitor, enhancement, P3)
DevTools
Netmonitor
Tracking
(firefox65 fixed)
RESOLVED
FIXED
Firefox 65
Tracking | Status | |
---|---|---|
firefox65 | --- | fixed |
People
(Reporter: ckerschb, Assigned: tanhengyeow, Mentored)
References
Details
(Keywords: dev-doc-complete, good-first-bug, Whiteboard: good-first-bug)
User Story
See comment #4 for detailed instructions about how to fix this bug
Attachments
(2 files, 2 obsolete files)
The HTTP Referrer Policy [1] allows webpages to gain more control over referrer values. It would be nice if Devtools could not only provide the referrer value but also the referrer policy applied to the request.
[1] https://www.w3.org/TR/referrer-policy/
Reporter | ||
Comment 1•6 years ago
|
||
FWIW, we extended referrer policy coverage to CSS files within Bug 1330487 (just as a reference).
Comment 2•6 years ago
|
||
Thanks for the report!
Referrer Policy should be displayed in the Headers panel - as part of the general info section (at the top of the panel).
See the attached screenshot. The new info should be underneath of `Version: HTTP/1.1`
For example:
Referrer Policy: origin
@Christoph: what platform API should we use to get the current referrer policy value?
Honza
Flags: needinfo?(ckerschb)
Updated•6 years ago
|
Priority: -- → P3
Reporter | ||
Comment 3•6 years ago
|
||
Hey Honza,
thanks for taking on that work.
(In reply to Jan Honza Odvarko [:Honza] from comment #2)
> @Christoph: what platform API should we use to get the current referrer
> policy value?
I guess you can query information from the nsIHttpChannel, right? If so, then the following should work:
Referer:
> https://searchfox.org/mozilla-central/source/netwerk/protocol/http/nsIHttpChannel.idl#58
Policy:
> https://searchfox.org/mozilla-central/source/netwerk/protocol/http/nsIHttpChannel.idl#94
Flags: needinfo?(ckerschb)
Comment 4•6 years ago
|
||
Thanks for the pointers Christoph!
Some instructions for anyone interested in this bug:
1) Referrer Policy should be displayed in the Headers panel - as part of the general info section (at the top of the panel).
See the attached screenshot. The new info should be underneath of `Version: HTTP/1.1`
For example:
Referrer Policy: origin
2) The current policy value can be read here:
https://searchfox.org/mozilla-central/source/netwerk/protocol/http/nsIHttpChannel.idl#94
3) Network monitor backend (aka the Network monitor actor) should do it here:
https://searchfox.org/mozilla-central/rev/80ac71c1c54af788b32e851192dfd2de2ec18e18/devtools/server/actors/network-monitor/network-observer.js#497
4) These two bugs implement support for resource tracking, which also involves reading a value in the backend and sending it tow the front end (client)
Bug 1333994 - The network tab should flag resources on the tracking protection list
Bug 1487274 - Devtools incorrectly suggests that www.reddit.com is a tracking URL on reddit.com
The second bug is a follow up and uses different backend field. The patch nicely shows all the places where the new filed needs to be tracked. This bug will do the same thing to transfer the policy value.
Here is a patch from the second bug:
https://phabricator.services.mozilla.com/D5018
5) Here is the UI that is responsible for rendering the summary (in Headers panel) e.g. the HTTP version info:
https://searchfox.org/mozilla-central/rev/80ac71c1c54af788b32e851192dfd2de2ec18e18/devtools/client/netmonitor/src/components/HeadersPanel.js#284
This place needs to also render the policy value.
Honza
Updated•6 years ago
|
User Story: (updated)
Assignee | ||
Comment 5•6 years ago
|
||
:Honza
Sounds like an interesting bug to learn more parts of the codebase :) Can I be assigned to work on this while waiting for reviews?
Flags: needinfo?(odvarko)
Comment 6•6 years ago
|
||
6) Also, this new features needs a test
you might look at existing ones to get some inspiration, for example:
https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/test/browser_net_header-docs.js#10
You might also search for: `request.httpVersion` in the test dir.
Honza
Updated•6 years ago
|
Assignee: nobody → E0032242
Flags: needinfo?(odvarko)
Assignee | ||
Comment 7•6 years ago
|
||
Display Referrer Policy for a given request.
Assignee | ||
Comment 8•6 years ago
|
||
Assignee | ||
Comment 9•6 years ago
|
||
Assignee | ||
Comment 10•6 years ago
|
||
Hi :Honza
Production code is ready for review, I'll write tests in the meantime.
I've attached two pictures to show how it looks like now in the panel :)
Flags: needinfo?(odvarko)
Comment 11•6 years ago
|
||
Thanks for the patch!
It looks great overall, just one comment. We should display self-explanatory string in the Headers panel instead of a number.
For example:
Referrer Policy: no-referrer-when-downgrade
Referrer Policy: origin-when-cross-origin
Referrer Policy: same-origin
Etc.
Here is a list of possible values:
https://searchfox.org/mozilla-central/rev/c56977420df7a1b692ce0f7e499ddb364d9fd7b2/netwerk/base/ReferrerPolicy.h#14
So we need a function that converts number to string e.g. `referrerPolicyToString` (similarly to what we have for http cause `causeTypeToString`)
Can be defined in utils/request-utils.js I guess
Honza
Flags: needinfo?(odvarko) → needinfo?(E0032242)
Assignee | ||
Comment 12•6 years ago
|
||
Hi :Honza
Do you know where to find more numeric representations of Referrer Policy values?
There are 8 string values listed here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy#Syntax (which corresponds to what https://searchfox.org/mozilla-central/rev/c56977420df7a1b692ce0f7e499ddb364d9fd7b2/netwerk/base/ReferrerPolicy.h#14 has)
Further down the document it states:
```
Possible values are:
0 — no-referrer
1 — same-origin
2 — strict-origin-when-cross-origin
3 — no-referrer-when-downgrade (the default)
```
I can display the string values of headers in the panel if they fall between values 0-3, but there seem to be a possibility of values being 4 when I test it manually. I suspect 4 refers to any of the other string values among the 8 listed.
Also, I noticed that there is a `ReferrerPolicyToString` function in https://searchfox.org/mozilla-central/rev/c56977420df7a1b692ce0f7e499ddb364d9fd7b2/netwerk/base/ReferrerPolicy.h#73
Any API available for us to get the string from the backend instead?
Flags: needinfo?(E0032242) → needinfo?(odvarko)
Comment 13•6 years ago
|
||
@Christoph: please see comment #12
What is the right way to get referrer policy label?
(the UI should render a label not cryptic number)
Honza
Flags: needinfo?(odvarko) → needinfo?(ckerschb)
Reporter | ||
Comment 14•6 years ago
|
||
(In reply to Heng Yeow (:tanhengyeow) from comment #12)
> Also, I noticed that there is a `ReferrerPolicyToString` function in
> https://searchfox.org/mozilla-central/rev/
> c56977420df7a1b692ce0f7e499ddb364d9fd7b2/netwerk/base/ReferrerPolicy.h#73
I suppose you mean there is *no* ReferrerPolicyToString(), right? There is only a ReferrerPolicyFromString() but for the purpose of displaying the policy in the panel we would need to translate the attribute into an actual string.
> Any API available for us to get the string from the backend instead?
I guess there is no alternative at the moment.
@Thomas: Could you think of anything? I guess we need to write a ReferrerPolicyToString() function and make it available to the panel.
Flags: needinfo?(ckerschb) → needinfo?(tnguyen)
Assignee | ||
Comment 15•6 years ago
|
||
> I suppose you mean there is *no* ReferrerPolicyToString(), right?
Yup you're right. Having that available would be nice :) Thanks for the help!
Comment 16•6 years ago
|
||
Thanks, we can put the function here: https://searchfox.org/mozilla-central/source/netwerk/base/ReferrerPolicy.h
Probably it would be nice to scan if there's any place in our codebase need to use that function too.
Flags: needinfo?(tnguyen)
Comment 17•6 years ago
|
||
@Thomas: I created a new bug report for the ReferrerPolicyToString() function - bug 1501206
Thanks for the help!
Honza
Depends on: 1501206
Flags: needinfo?(tnguyen)
Updated•6 years ago
|
Flags: needinfo?(tnguyen)
Updated•6 years ago
|
Keywords: dev-doc-needed
Comment 18•6 years ago
|
||
@Heng Yeow: bug 1501206 has been fixed, so we can do progress on this bug!
Honza
Flags: needinfo?(E0032242)
Assignee | ||
Updated•6 years ago
|
Attachment #9018324 -
Attachment is obsolete: true
Flags: needinfo?(E0032242)
Assignee | ||
Updated•6 years ago
|
Attachment #9018325 -
Attachment is obsolete: true
Assignee | ||
Comment 19•6 years ago
|
||
Hi :Honza
Great! I saw bug 1501206's changes and have updated my patch accordingly. Updated the test too!
Try results here for reference:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fbbde8086a808cfb3ea907669954e40f9232785c
Flags: needinfo?(odvarko)
Assignee | ||
Comment 20•6 years ago
|
||
Updated the relevant networkEvent stubs, new try results shown here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2023b6d6078aa20e96071b7a50bc0f872fa467c
Comment 21•6 years ago
|
||
(In reply to Heng Yeow (:tanhengyeow) from comment #19)
> Hi :Honza
>
> Great! I saw bug 1501206's changes and have updated my patch accordingly.
> Updated the test too!
>
> Try results here for reference:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=fbbde8086a808cfb3ea907669954e40f9232785c
Looks good, thanks!
Honza
Flags: needinfo?(odvarko)
Comment 22•6 years ago
|
||
Failed to land:
https://lando.services.mozilla.com/revisions/D9128/31503/
With Diff 31503 on Mon, November 5, 2018, 3:35 PM GMT+1, by jodvarko@mozilla.com.
Error: hg error in cmd: hg push -r tip upstream: pushing to ssh://hg.mozilla.org/integration/autoland searching for changes remote: adding changesets remote: adding manifests remote: adding file changes remote: added 1 changesets with 10 changes to 10 files remote: remote: remote: ************************** ERROR **************************** remote: Rev aedcbe255350 needs "Bug N" or "No bug" in the commit message. remote: tanhengyeow <E0032242@u.nus.edu> remote: Bug-1496742 - Allow to display Referrer Policy for a given request. r=Honza remote: remote: Display Referrer Policy for a given request. remote: remote: Differential Revision: https://phabricator.services.mozilla.com/D9128 remote: ************************************************************* remote: remote: remote: transaction abort! remote: rollback completed remote: pretxnchangegroup.c_commitmessage hook failed abort: push failed on remote
Flags: needinfo?(E0032242)
Updated•6 years ago
|
Attachment #9018321 -
Attachment description: Bug-1496742 - Allow to display Referrer Policy for a given request. r=Honza → Bug 1496742 - Allow to display Referrer Policy for a given request. r=Honza
Assignee | ||
Comment 23•6 years ago
|
||
Updated the commit message, should fix the error :)
Flags: needinfo?(E0032242) → needinfo?(odvarko)
Comment 24•6 years ago
|
||
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d689ddba40a1
Allow to display Referrer Policy for a given request. r=Honza
Comment 25•6 years ago
|
||
(In reply to Heng Yeow (:tanhengyeow) from comment #23)
> Updated the commit message, should fix the error :)
Great thanks!
Honza
Flags: needinfo?(odvarko)
Comment 26•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Comment 27•6 years ago
|
||
Note:
If the network.http.referer.hideOnionSource is set and the referrer is .onion ended
The "Referrer Policy" in devtool always shows "no-referrer-when-downgrade" which is default.
It should be fine since the goal of that pref is to hide information of referrer/origin header.
Comment 28•6 years ago
|
||
Added the following line to Network request details page:
The Referrer Policy, which governs which referrer information, sent in the Referer header, should be included with requests. (See Referrer-Policy for a description of possible values)
I also took the opportunity to reorganize the headers section of the page to improve the clarity of the section.
Also added the information to the Firefox 65 Release Notes.
Flags: needinfo?(E0032242)
Keywords: dev-doc-needed → dev-doc-complete
Assignee | ||
Comment 29•6 years ago
|
||
Hi Irene
Looks good! Thanks for the update :)
Flags: needinfo?(E0032242)
You need to log in
before you can comment on or make changes to this bug.
Description
•