1571742 - Json data is displayed as raw text instead of using the built-in Json viewer

Status: VERIFIED FIXED

(DevTools :: JSON Viewer, defect, P2)

Description

[Pascal Chevrel (relman team) -> :pascalc]

Mozilla/5.0 (X11; Linux x86_64; rv:70.0) Gecko/20100101 Firefox/70.0 ID:20190805220030

1/ Go to https://product-details.mozilla.org/1.0/firefox_versions.json (or any json feed

Expected result:
Json is prettified with the built-in Firefox viewer for Json

Actual result:
Json is displayed as raw text, as if the file was sent with a text/plain mimetype (it's not)

This is a regression, it workd in 69.0b11. I'll see if I can find a regression window.

Comment 1

[BugBot [:suhaib / :marco]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Pascal Chevrel (relman team) -> :pascalc]

16:26.08 INFO: Last good revision: 1795156aec4f053365bc5a7160b77e2b6e752e64
16:26.08 INFO: First bad revision: 0aa6dca8717afcf336ed340caa233841104701c5
16:26.08 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=1795156aec4f053365bc5a7160b77e2b6e752e64&tochange=0aa6dca8717afcf336ed340caa233841104701c5

Sebastian, it seems that your patch in bug 1428473 broke partly our json viewer, most of the Json files I am loading in Firefox are now displayed as text/plain.

For example:
https://product-details.mozilla.org/1.0/firefox_versions.json

Is that an intended behaviour change?

Comment 3

[Pascal Chevrel (relman team) -> :pascalc]

Tracking for 70 because it feels broken now so it either needs a fix before shipping 70 or if extra HTTP headers have tpo be sent to allow our json viewer to kick in , we need to document that change in our developers release notes.

Comment 4

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

Thanks for the report and regression window!

Honza

Comment 5

[Sebastian Streich [:sstreich]]

Hey!
As it seems the patch is working as expected. On the page "X-Content-Type:nosniff" is set and we disable all sniffers. But it appears the JSON-Viewer relies on sniffing to be enabled. In [1] we have a json-view-sniffer that changes the mime-type from "application/json" to "application/vnd.mozilla.json.view". Without that set, its rendered as text/plain.

We could allow the json-view sniffer anyway even if nosniff is enabled, if that is okay?

[1] https://searchfox.org/mozilla-central/source/devtools/client/jsonview/converter-observer.js

Comment 6

[Sebastian Streich [:sstreich]]

Attachment: Bug 1571742 - Move Nosniff-Check into Sniffers r=ckerschb

Comment 7

[Dragana Damjanovic [:dragana]]

(In reply to Sebastian Streich [:sstreich] from comment #5)

Hey!
As it seems the patch is working as expected. On the page "X-Content-Type:nosniff" is set and we disable all sniffers. But it appears the JSON-Viewer relies on sniffing to be enabled. In [1] we have a json-view-sniffer that changes the mime-type from "application/json" to "application/vnd.mozilla.json.view". Without that set, its rendered as text/plain.

We could allow the json-view sniffer anyway even if nosniff is enabled, if that is okay?

[1] https://searchfox.org/mozilla-central/source/devtools/client/jsonview/converter-observer.js

I would like to ask Christoph if this is ok. I think it is.

Comment 8

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Dragana Damjanovic [:dragana] from comment #7)

I would like to ask Christoph if this is ok. I think it is.

It's corner casy and Sebastian and I already discussed before we flagged your for ni. Now that you are also agreeing that it is ok to add a carveout for json, I think we should just add that carveout :-)

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

This failed to apply:

Please manually rebase your commits and try again. applying /tmp/tmpQYbX1M netwerk/protocol/http/nsHttpChannel.cpp Hunk #1 succeeded at 1448 with fuzz 1 (offset 2 lines). netwerk/base/nsNetUtil.cpp Hunk #2 FAILED at 2893. 1 out of 2 hunks FAILED -- saving rejects to file netwerk/base/nsNetUtil.cpp.rej abort: patch command failed: exited with status 256

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b3ee5fed44ec
Move Nosniff-Check into Sniffers r=ckerschb,dragana

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

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

Comment 12

[Negritas Sergiu, Desktop QA]

Following the STR from the first comment, I managed to reproduce the issue on nightly 70.0a1 (2019-08-01).
I can confirm that the issue is fixed in 70.0b7.