1957333 - HTML preview of response is affected by the netmonitor's CSP

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P2)

Description

[Kamil Pešek]

Attachment: error.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:137.0) Gecko/20100101 Firefox/137.0

Steps to reproduce:

I want to see HTML response in Developer Tools -> Network -> Select request -> Response -> HTML preview.

I believe it stopped working max month ago. It's similar problem which was there previously https://bugzilla.mozilla.org/show_bug.cgi?id=1804232

Actual results:

HTML is not shown correctly, sometimes nothing is shown (probably when there is more complex HTML or CSS), sometimes just part is shown but without any details.

Expected results:

Correct HTML preview should be shown, as previously.

Comment 1

[Kamil Pešek]

Attachment: Screenshot.png

Comment 2

[Kamil Pešek]

Comment on attachment 9475640 [details]
error.html

This is debug message generated by PHP framework Symfony

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'DevTools::Netmonitor' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Hello Kamil, thanks for the report.
Would you be able to run https://mozilla.github.io/mozregression/ and report the results here so we know when it regressed?

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

(looks like Bug 1957202 is similar)

Comment 6

[Tom Schuster (MoCo)]

This is caused by D238046 apparently we are rendering the response using an iframe with a data: URL inside the web console. netmonitor/index.html now has a CSP that blocks most styles/images etc.

We really should not be rendering this untrusted HTML directly inside the devtools. I think for similar cases inside the browser (like the sidebar) we would usually use a <browser> element.

Comment 7

[Tom Schuster (MoCo)]

To make uplifts easier we probably need to remove the CSP in 137/138.

Comment 9

[Tom Schuster (MoCo)]

[Tracking Requested - why for this release]:

Comment 10

[Tom Schuster (MoCo)]

Attachment: Bug 1957333 - Beta: Remove CSP from Netmonitor. r?#devtools-reviewers

This partially reverts D238046.

Comment 11

[Tom Schuster (MoCo)]

Attachment: Bug 1957333 - Use loadURI for HTML preview to prevent inheriting the CSP. r?#devtools-reviewers!,nika!

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1943360

Comment 13

[Julian Descottes [:jdescottes]]

Tom, do you think we should move the second patch to another bug so that we can close this one once the regression is fixed and uplifted?

Comment 14

[Tom Schuster (MoCo)]

(In reply to Julian Descottes [:jdescottes] from comment #13)

Tom, do you think we should move the second patch to another bug so that we can close this one once the regression is fixed and uplifted?

I actually think it's easier to track everything in one bug. We have release specific tracking flags, and the Beta (and maybe Release) patch is basically just a backout.

Comment 15

[Tom Schuster (MoCo)]

Comment on attachment 9475978 [details]
Bug 1957333 - Beta: Remove CSP from Netmonitor. r?#devtools-reviewers

Beta/Release Uplift Approval Request

• User impact if declined/Reason for urgency: The HTML preview in the Devtools Network Monitor doesn't use CSS stylesheets, images etc. So this only matters to web developers.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is essentially just a backout.
• String changes made/needed:
• Is Android affected?: No

Comment 16

[Tom Schuster (MoCo)]

In D243788 I am adding a test that covers this problem.

Comment 17

[Dianna Smith [:diannaS]]

Approved for 138.0b5 (fixed by backout). This reverts part of bug 1943360 but remains in 139 which should be fixed by D243788.

Tom , whats the eta in landing that patch for 139?

Comment 18

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/58716a313143

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/340abfffec9d
Use loadURI for HTML preview to prevent inheriting the CSP. r=nika,ochameau

Comment 20

[Tom Schuster (MoCo)]

Tom , whats the eta in landing that patch for 139?

Just landed now.

Comment 21

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/340abfffec9d