1376950 - Headers modified in webRequest.onHeadersReceived are not displayed in Netmonitor

Status: NEW

(DevTools :: Netmonitor, defect, P3)

Description

[April King [:April]]

I have a webExtension that listens to webRequest.onHeadersReceived, removes an existing CSP header and then pushes this onto request.responseHeaders:

> request.responseHeaders.push({
>   name: 'Content-Security-Policy',
>   value: `object-src 'none'; script-src 'none'`,
> });

Instead of showing the headers as modified by the webExtension, DevTools will show the original CSP header as received from the website. My guess is that DevTools is subscribing to the same event (request.onHeadersReceived), but is happening prior to the other webExtensions.

This can make debugging very difficult, as there's no real way to see the final headers as seen by Firefox.

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

@Luca, is this the correct way to do it?
Should this report by part of webextension component?

Honza

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

The code fragment from Comment 0 looks correct to me (I'm also wondering if that is happening only for the the CSP policy header or for any changes on the HTTP headers). 

I'm adding Shane, he is going to know more about the internals of the webRequest API.

It is likely that the webextension internals code that allow an extension to change the HTTP response headers (which should be in the WebRequest.jsm module [1]) and the "Remote Debugging Server actor" that is monitoring the HTTP requests for the Network Monitor (which seems to be the webconsole actor through the the network-monitor.js shared webconsole module [2]) are happening at the same time and the Network Monitor is logging the HTTP request before it has been changed by the extension.

I think that we could split this problem into two steps.

- the first step could be fixing the current behavior by making sure that the network monitor panel is always going to log the response headers after any extension has already done its changes

- the second step (which is more like a feature) could be to ensure that the network monitor panel is going to be able to show both the original value of the header and the new value of the header after it has been changed by the extensions, and also which of the extensions has set the final value (this would be really helpful, especially given that more than one installed extension can have subscribed a listener to change the response headers and "last executed listener" is going to "win").

imho, we can consider the first step as a bugfix on the Network Monitor panel (and so this issue would be already on the right bugzilla component), and the second step as a new "Addon Debugging" feature (and so we could file it as a follow-up bugzilla issue filed under the "WebExtensions: Developer Tools" bugzilla component).

how that sound to you?

[1]: http://searchfox.org/mozilla-central/source/toolkit/modules/addons/WebRequest.jsm
[2]: http://searchfox.org/mozilla-central/source/devtools/shared/webconsole/network-monitor.js

Comment 3

[April King [:April]]

> The code fragment from Comment 0 looks correct to me (I'm also wondering if that is happening only for the the CSP policy header or for any changes on the HTTP headers). 

It's any header manipulation. This includes addition, modification, and deletion.

Comment 4

[Shane Caraveo (:mixedpuppy)]

I don't have much to add over what Luca wrote.  We discussed this yesterday, it looks like devtools may be using httpactivityobserver and is getting the headers at an earlier stage than what webrequest does.  Someone from devtools should probably comment.

Comment 5

[David Durst [:ddurst]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #4)

I don't have much to add over what Luca wrote. We discussed this yesterday,
it looks like devtools may be using httpactivityobserver and is getting the
headers at an earlier stage than what webrequest does. Someone from
devtools should probably comment.

Is this still an open question that affects the course of this bug? If so, should we explicitly NI someone?

Comment 6

[Shane Caraveo (:mixedpuppy)]

Took a quick look at the devtools code, from that I'm assuming this is still a problem.

I think I'd want both the original request headers and the extension-modified request headers in the UI. But that's just me. If that were done, what exists now takes care of the first part, and this would be about adding another UI element (in request and response headers) to show post-extension event modifications.

I don't know who takes care of devtools features, but it looks like :Honza is the triage owner. I think the first step is deciding how this should work from a product perspective.

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

To summarize:

#1 The first step - fixing the current behavior by making sure that the network monitor panel is always going to log the response headers after any extension has already done its change

As correctly mentioned in comment #4, DevTools backend is using nsIHttpActivityObserver to observer/intercept HTTP traffic. It's also using observer notifications like the following:

"http-on-examine-response"
"http-on-examine-cached-response"
"http-on-modify-request"

The backend is also using visitOriginalResponseHeaders (when handling the above events) and visitRequestHeaders (when nsIHttpActivityObserver.ACTIVITY_SUBTYPE_REQUEST_HEADER is fired) to collect all headers.

https://searchfox.org/mozilla-central/rev/3d469329a42644b41e66944d8da22d78d8c0f5ec/devtools/server/actors/network-monitor/network-observer.js#282

https://searchfox.org/mozilla-central/rev/3d469329a42644b41e66944d8da22d78d8c0f5ec/devtools/server/actors/network-monitor/network-observer.js#282

• We probably want to change this piece and collect headers (also?) upon another events to see even the version that's modified by extensions.

#2 The second step - (new feature) ensure that the network monitor panel is going to be able to show both the original value of the header and the new value of the header after it has been changed by the extensions, and also which of the extensions has set the final value (this would be really helpful, especially given that more than one installed extension can have subscribed a listener to change the response headers and "last executed listener" is going to "win").

The new UI could:

• have a new option (a checkbox) that could be used to switch between two modes (original vs. modified headers).
• have additional sections in the Headers side panel showing modified values as well as original values at the same time.

@Harald, any ideas about the UI?

Honza

Comment 8

[:Harald Kirschner :digitarald]

I would focus this bug on the first issue (which you described here in great detail, thank you!) – which is the defect as the resources listed don't reflect reality.

The follow up enhancement bugs I'd see is that

1. As a first step, have an indicator that a request/response was modified by an extension at all? Do we know which extension did it?
2. Being able to see the original/modified values

@Harald, any ideas about the UI?

Seeing the older versions grayed out in the headers list might be a simple solution that doesn't require another toggle. The addons that modified them could be referenced with their icon in the same line, as badge.

Comment 9

[Shane Caraveo (:mixedpuppy)]

Rough thoughts...

• Update dev tools to use the WebRequest.jsm module to monitor requests
• Update WebRequest.jsm to support monitoring events.
  • add WebRequest.onMonitorEvents which is fired for any changes made by an extension
  • maybe it is fired before and after each event
  • The event details could contain what modifications were made by what extensions

Devtools would have to track the changes so that it knows which extension made which changes, that could become a part of the UI