1316393 - JSON Viewer broken by CSP base-uri

Status: VERIFIED FIXED

(DevTools :: JSON Viewer, defect, P2)

Description

[:Felipe Gomes (needinfo for replies!)]

I don't know what's particular about this page, but it breaks the JSON Viewer. Instead of loading correctly, it dumps the original JSON content, along with the HTTP headers, as a JSON text dump in the main page.

Note: this is not a recent regression. I bisected this and it went back to the time when devtools.jsonviewer.enabled was flipped to true for Nightly (in March)

Comment 1

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

I don't understand what's the reported problem.
Can you please provide STR?

Honza

Comment 2

[:Felipe Gomes (needinfo for replies!)]

Attachment: bug1173548.png

Sure, just access the URL with and without devtools.jsonview.enabled true.
https://addons.mozilla.org/api/v3/addons/addon/[e-mail]/feature_compatibility/

See screenshot. With the jsonviewer activated, the UI doesn't show up. Instead, the data of the jsonviewer is dumped as plaintext alongside with the actual content.

Comment 3

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

Thanks, I can see it now.

Honza

Comment 4

[Julien Cristau [:jcristau]]

Mass wontfix for bugs affecting firefox 52.

Comment 5

[Oriol Brufau [:Oriol]]

This is because of the CSP. It blocks both the styles that hid the headers and the script that loads the JSON Viewer.

At least, after bug 1367894 you will only see the raw JSON, without the headers.

Comment 6

[Oriol Brufau [:Oriol]]

Attachment: json-csp.patch

The problem is that CSP can prevent a baseURI change. The workaround is using absolute URIs. And for some reason, then .js extensions are needed.

Comment 7

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

Comment on attachment 8879944 [details] [diff] [review]
json-csp.patch

Review of attachment 8879944 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for working on this. The patch fixes the problem for me.

R+, but it would be great to have a note about the reason why we are using explicit '.js' file extension. Perhas we could introduce a README.md (in devtools/client/jsonview dir). Just like e.g. the netmonitor has it.

Honza

::: devtools/client/jsonview/converter-child.js
@@ +205,5 @@
>    } else {
>      os = "linux";
>    }
>  
> +  let baseURI = "resource://devtools/client/jsonview/";

Please make a comment about why we are using absolute URI

Comment 8

[Oriol Brufau [:Oriol]]

Attachment: json-csp.patch

The .js extensions were needed because I didn't change baseUrl in requirejs config.

Comment 9

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

Comment on attachment 8880578 [details] [diff] [review]
json-csp.patch

Review of attachment 8880578 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

R+, assuming try is green

Honza

Comment 10

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

Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=32244bd1d58441c30645330a63d17f94fe26f7f6

Honza

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cbf8e6de9bf7
Circumvent CSP base-uri restriction in JSON Viewer. r=Honza

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cbf8e6de9bf7

Comment 13

[Md.Tarikul Islam Oashi]

I have reproduced this bug with nightly 52.0a1 (2016-11-09) on "Linux Mint (64 Bit).

The bug's fix is now verified on Latest Nightly 56.0a1

Build ID 	20170630100234
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0

[Bugday-20170628]

Comment 15

[Oriol Brufau [:Oriol]]

Marking this bug as verified fixed, per comment 13.