Allow to display Referrer Policy for a given request

RESOLVED FIXED in Firefox 65

Status

P3
normal
RESOLVED FIXED
6 months ago
a month ago

People

(Reporter: ckerschb, Assigned: tanhengyeow, Mentored)

Tracking

({dev-doc-complete, good-first-bug})

unspecified
Firefox 65
dev-doc-complete, good-first-bug

Firefox Tracking Flags

(firefox65 fixed)

Details

(Whiteboard: good-first-bug)

User Story

See comment #4 for detailed instructions about how to fix this bug

Attachments

(2 attachments, 2 obsolete attachments)

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 months ago
FWIW, we extended referrer policy coverage to CSS files within Bug 1330487 (just as a reference).
Posted image headers-panel.jpg
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)
Priority: -- → P3
(Reporter)

Comment 3

6 months 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)
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
Mentor: odvarko
Keywords: good-first-bug
Whiteboard: good-first-bug
User Story: (updated)
(Assignee)

Comment 5

6 months 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)
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
Assignee: nobody → E0032242
Flags: needinfo?(odvarko)
(Assignee)

Comment 7

5 months ago
Display Referrer Policy for a given request.
(Assignee)

Comment 8

5 months ago
Posted image referrer-policy-1.png (obsolete) —
(Assignee)

Comment 9

5 months ago
Posted image referrer-policy-3.png (obsolete) —
(Assignee)

Comment 10

5 months 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)
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

5 months 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)
@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

5 months 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

5 months 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

5 months 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)
@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

5 months ago
Flags: needinfo?(tnguyen)
Keywords: dev-doc-needed
@Heng Yeow: bug 1501206 has been fixed, so we can do progress on this bug!

Honza
Flags: needinfo?(E0032242)
(Assignee)

Updated

5 months ago
Attachment #9018324 - Attachment is obsolete: true
Flags: needinfo?(E0032242)
(Assignee)

Updated

5 months ago
Attachment #9018325 - Attachment is obsolete: true
(Assignee)

Comment 19

5 months 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

5 months ago
Updated the relevant networkEvent stubs, new try results shown here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2023b6d6078aa20e96071b7a50bc0f872fa467c
(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)
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)
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

5 months ago
Updated the commit message, should fix the error :)
Flags: needinfo?(E0032242) → needinfo?(odvarko)

Comment 24

5 months 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
(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

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d689ddba40a1
Status: NEW → RESOLVED
Last Resolved: 5 months ago
status-firefox65: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65

Comment 27

5 months 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

2 months 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

a month 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.