1506843 - Cloning a request doesn't recognise principals

Status: NEW

(DevTools :: Netmonitor, enhancement, P3)

Description

[Jonathan Kingston [:jkt] he/him]

When using the "Edit and Resend" feature in the browser it doesn't obey CORS which is likely because it doesn't clone the original triggering principal used for the load and instead possibly uses system.

1. Open example.com
2. Open the devtools
3. Type fetch("https://google.com");
4. The load is denied
5. Click on the load in the network tab
6. Click "Edit and Resend"
7. Click "Send"

Expected results:

Second load should fail

Actual Results

Second load is permitted


The following code appears to populate the form that is used to send the request, however it doesn't serialise the principal into the form:
https://searchfox.org/mozilla-central/rev/7f7c353e969e61a6a85201cc8ad3c3de12ac30d8/devtools/client/netmonitor/src/reducers/requests.js#121

Comment 1

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

Thanks for the report!

So, here is the back-end code that sends the request (cloned)
https://searchfox.org/mozilla-central/rev/007b66c1f5f7a1b9a900a2038652a16a020f010c/devtools/server/actors/webconsole.js#1616

What API we should exactly use to set the principal? An example?
And how should we get the principal from an existing nsIHttpChannel?

Honza

Comment 2

[Jonathan Kingston [:jkt] he/him]

So I think you should be able to get this through channel.loadInfo.triggeringPrincipal. For example like:
https://searchfox.org/mozilla-central/rev/007b66c1f5f7a1b9a900a2038652a16a020f010c/toolkit/components/passwordmgr/LoginManagerContent.jsm#99

The NetUtil.newChannel({ should accept an argument of a triggeringPrincipal.

My suggestion would be the form could take a hidden serialized triggeringPrincipal which would then be deserialized and passed into the newChannel.

Looking through some prior bugs suggest there might be more to it than this though.

Comment 3

[Jonathan Kingston [:jkt] he/him]

So the other issue is that
> securityFlags: Ci.nsILoadInfo.SEC_ALLOW_CROSS_ORIGIN_DATA_IS_NULL
is being used.

This bypasses the checks also, this would require serializing this also.


Also when I set this and the network error fails as expected I get the following error:

> error occurred while processing 'sendHTTPRequest: [Exception... "Failed to open input source 'http://google.com/'"  nsresult: "0x805303f4 (<unknown>)"  location: "JS frame :: resource://devtools/server/actors/webconsole.js :: sendHTTPRequest :: line 1656"  data: yes]

The form keeps showing as if it hasn't sent.

Comment 4

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

Thanks for the info!

One question, couldn't it be actually useful to bypass the principal sometimes and successfully execute the request without restrictions? 

I am asking because we might want to have an option in the UI allowing to follow the principal and security flags or not.

Honza

Comment 5

[Jonathan Kingston [:jkt] he/him]

> One question, couldn't it be actually useful to bypass the principal sometimes and successfully execute the request without restrictions? 

I can see that being useful perhaps, especially in debugging things like cors where you want to check the server is behaving correctly. It probably just makes sense to make it a single toggle rather than offer internals.

Taking this as I'm looking into this (I'll unassign if I get busy). (I would like to use this as a test bed to adding a serializedLoadInfo to NetUtils.newChannel and a serialize method to a loadInfo which would reduce the chance of breakage in future).

Comment 6

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

(In reply to Jonathan Kingston [:jkt] from comment #5)
> > One question, couldn't it be actually useful to bypass the principal sometimes and successfully execute the request without restrictions? 
> 
> I can see that being useful perhaps, especially in debugging things like
> cors where you want to check the server is behaving correctly. It probably
> just makes sense to make it a single toggle rather than offer internals.
Yes, agree.

> Taking this as I'm looking into this (I'll unassign if I get busy). (I would
> like to use this as a test bed to adding a serializedLoadInfo to
> NetUtils.newChannel and a serialize method to a loadInfo which would reduce
> the chance of breakage in future).
Sounds good, thanks!

Honza

Comment 7

[BugBot [:suhaib / :marco]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:Honza, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 8

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

Still something we want to fix, but not on our immediate roadmap.

Honza