Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Should JSON MIME Types include all */*+json? #112

Open
hiroshige-g opened this issue Nov 8, 2019 · 12 comments · May be fixed by #113
Open

Should JSON MIME Types include all */*+json? #112

hiroshige-g opened this issue Nov 8, 2019 · 12 comments · May be fixed by #113

Comments

@hiroshige-g
Copy link

https://mimesniff.spec.whatwg.org/#json-mime-type defines JSON MIME Types as:

  • application/json
  • text/json
  • */*+json

But I'm wondering whether it's better to limit the third case to application/*+json or something, or not, mainly in the context of JSON modules.

pros:

  • The current browser implementations don't seem to consider all */*+json MIME types as JSON. JSON MIME types are used in https://html.spec.whatwg.org/C/#process-a-navigate-response, and thus navigations to responses with JSON MIME types should be like navigating to text page (e.g. text/plain page), but according to draft WPT ([TEST] Test MIME Types + navigation web-platform-tests/wpt#20169):
    • Following JSON MIME types are rejected (i.e. trigger download instead of navigation) on Firefox:
      • application/*+json
      • text/*+json
    • Following JSON MIME types are rejected on Firefox and Chromium:
      • All other */*+json
  • As JSON modules is a fairly new feature, it might be worth considering to setting stricter restriction on MIME types (just like module scripts have stricter MIME type restriction than classic scripts).

cons:

  • The current spec is aligned with how XML MIME Types are defined, i.e. including */*+xml in RFC7303, and the rationale behind it: https://tools.ietf.org/html/rfc7303#appendix-A.
    • Similarly, all */*+xml MIME Types don't seem considered as XMLs by the current browsers, according to the draft WPT above though.

WDYT?
@hiroshige-g
Copy link
Author

@domenic @nhiroki @littledan

@annevk
Copy link
Member

annevk commented Nov 8, 2019

Your pros and cons mainly consider status in implementations, but what would the engineering benefit be, so to speak?

@littledan
Copy link

I am not an expert in this area so I don't have an opinion; I was just trying to follow existing spec conventions. Maybe @Ms2ger @dandclark @BoCupp-Microsoft would know more.

Will JSON modules be the first time that the JSON MIME type is actually checked in running an HTML page?

@annevk
Copy link
Member

annevk commented Nov 8, 2019

It might be yes, fetch() and XMLHttpRequest notably ignore it. (Web Manifest doesn't seem to check the MIME type, filed an issue on that.)

@littledan littledan mentioned this issue Nov 8, 2019
Open
@dandclark
Copy link

(just like module scripts have stricter MIME type restriction than classic scripts)

Is anyone familiar with the reasoning behind that decision? That could help inform our thinking here. In particular I wonder if it was just a simplification or if there was some security concern.

I'm no expert in this area either, but it seems like engineering benefit of allowing */*+json is that JSON modules could import any future dual-format that is introduced someday with this MIME type, e.g. an image format that is also valid JSON. Although no such thing exists now that I'm aware of, so maybe that's an argument for keeping the MIME type check restricted for the time being while being open to loosening the restrictions to allow it in the future if needed.

@hiroshige-g
Copy link
Author

(just like module scripts have stricter MIME type restriction than classic scripts)

IIUC:
Classic scripts allow MIME sniffing and accept a quite broad range of MIME types, including text/html which are clearly non-JavaScript, can be executed as scripts by attacks/mistake but are widely used.
Module scripts are new and don't have to be worried about breaking existing scripts, so allow only strict JavaScript MIME types for improved security.

I'm not sure whether similar argument applies to JSON modules.

The benefit to allow */*+json would be clarifying that we allow a broad range of MIME types (including e.g. image/foobar+json) intentionally as JSON modules.

The benefit to narrow the range would be improving security. WICG/webcomponents#839 will make JSON modules NOT to SOLELY rely on MIME types, but will we still need MIME type checks as a part of security guard? I don't have specific concerning cases for now and thus I filed this issue whether someone has specific thoughts aboud security.

@hiroshige-g
Copy link
Author

Will JSON modules be the first time that the JSON MIME type is actually checked in running an HTML page?

Navigation (https://html.spec.whatwg.org/C/#process-a-navigate-response, see #112 (comment)) also checks JSON MIME type, but I'm not sure about its importance.

@annevk
Copy link
Member

annevk commented Nov 9, 2019

Ah, that's what you meant. So yes, requiring a JSON MIME type is important, just like it's important to require a JavaScript MIME type in new contexts (especially if they can parse differently), but it doesn't have to be a specific one, as far as I can tell.

Navigation is observable if you navigate an <iframe> or popup to a JSON response. Per those rules it ought to be rendered (and be accessible) as a text document.

@domenic
Copy link
Member

domenic commented Nov 11, 2019

Is anyone familiar with the reasoning behind that decision? That could help inform our thinking here. In particular I wonder if it was just a simplification or if there was some security concern.

It is a security concern. There have been attacks in the past (e.g. this paper (C)) based on non-JS files being treated as JS files and "executed".

In general, allowing web sites to treat the contents of URLs as a different type than their declared content-type is an attack vector that gets exploited fairly regularly.

@dandclark
Copy link

dandclark commented Nov 13, 2019 •
edited

I'm thinking that in this case the benefit of allowing potential JSON-like types like image/foobar+json to be loaded as JSON modules outweighs any benefit of narrowing the JSON MIME-type set. I understand now the security concerns of loading non-JS as script, but it's hard to see how parsing even arbitrary content as JSON could be harmful on the same level. And adding +json to a MIME type seems like a pretty straightforward signal that the resource owner expects that it could be interpreted as JSON.

@hiroshige-g
Copy link
Author

So, the concensus here can be:

  • Importing */*+json, including image/foobar+json has valid use cases
  • Importing (and navigating to) */*+json doesn't look to cause security problems (because JSON isn't executed)
  • so let's keep the current spec and allow */*+json as-is.

Does this sounds good?

If so, Chromium will allow */*+json for JSON modules.
If fixing the existing interoperability issue of navigation + */*+json is desired, we might want to file a Pri-3 bug against Chromium (and other browsers), but currently I don't have immediate motivation to changing Chromium's behavior around navigation.

@hiroshige-g hiroshige-g linked a pull request Nov 19, 2019 that will close this issue
Open
@hiroshige-g
Copy link
Author

Created #113 that feeds the discussion here back to the spec.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 20, 2021
@dandclark @chromium-wpt-export-bot
Add JSON modules tests for charset, integrity, and MIME type parameters
4782e84 
Add JSON modules tests for the following:
- The <script> element's integrity attribute is respected.
- JSON modules are always decoded as UTF-8 regardless of Content-Type
  or the document's encoding.  The existing coverage we had on this in
  utf8.tenative.html was renamed to charset.html, refactored, and
  expanded.
- Adding parameters to the MIME type doesn't prevent it from being
  evaluated as a JSON MIME type.

Note, some of the existing MIME type tests for */*+json still fail
because it seems the spec issue on these at
whatwg/mimesniff#112 hasn't been fully
resolved. I'll follow up with that separately.

Bug: 1132413
Change-Id: I682de01bbb14b53214fcc16f427a4875f8cfed7f
Merged
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 22, 2021
@dandclark @chromium-wpt-export-bot
Add JSON modules tests for charset, integrity, and MIME type parameters
80c9552 
Add JSON modules tests for the following:
- The <script> element's integrity attribute is respected.
- JSON modules are always decoded as UTF-8 regardless of Content-Type
  or the document's encoding.  The existing coverage we had on this in
  utf8.tenative.html was renamed to charset.html, refactored, and
  expanded.
- Adding parameters to the MIME type doesn't prevent it from being
  evaluated as a JSON MIME type.

Note, some of the existing MIME type tests for */*+json still fail
because it seems the spec issue on these at
whatwg/mimesniff#112 hasn't been fully
resolved. I'll follow up with that separately.

Bug: 1132413
Change-Id: I682de01bbb14b53214fcc16f427a4875f8cfed7f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2841103
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#875274}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 22, 2021
@dandclark @chromium-wpt-export-bot
Add JSON modules tests for charset, integrity, and MIME type parameters
cda4273 
Add JSON modules tests for the following:
- The <script> element's integrity attribute is respected.
- JSON modules are always decoded as UTF-8 regardless of Content-Type
  or the document's encoding.  The existing coverage we had on this in
  utf8.tenative.html was renamed to charset.html, refactored, and
  expanded.
- Adding parameters to the MIME type doesn't prevent it from being
  evaluated as a JSON MIME type.

Note, some of the existing MIME type tests for */*+json still fail
because it seems the spec issue on these at
whatwg/mimesniff#112 hasn't been fully
resolved. I'll follow up with that separately.

Bug: 1132413
Change-Id: I682de01bbb14b53214fcc16f427a4875f8cfed7f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2841103
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#875274}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 25, 2021
@dandclark @moz-wptsync-bot
Bug 1706465 [wpt PR 28606] - Add JSON modules tests for charset, inte…
81e9506 
…grity, and MIME type parameters, a=testonly

Automatic update from web-platform-tests
Add JSON modules tests for charset, integrity, and MIME type parameters

Add JSON modules tests for the following:
- The <script> element's integrity attribute is respected.
- JSON modules are always decoded as UTF-8 regardless of Content-Type
  or the document's encoding.  The existing coverage we had on this in
  utf8.tenative.html was renamed to charset.html, refactored, and
  expanded.
- Adding parameters to the MIME type doesn't prevent it from being
  evaluated as a JSON MIME type.

Note, some of the existing MIME type tests for */*+json still fail
because it seems the spec issue on these at
whatwg/mimesniff#112 hasn't been fully
resolved. I'll follow up with that separately.

Bug: 1132413
Change-Id: I682de01bbb14b53214fcc16f427a4875f8cfed7f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2841103
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#875274}

--

wpt-commits: cda427384ac75751fe809c6006ca5167e0c8f7ad
wpt-pr: 28606
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
@dandclark
Add JSON modules tests for charset, integrity, and MIME type parameters
58791d7 
Add JSON modules tests for the following:
- The <script> element's integrity attribute is respected.
- JSON modules are always decoded as UTF-8 regardless of Content-Type
  or the document's encoding.  The existing coverage we had on this in
  utf8.tenative.html was renamed to charset.html, refactored, and
  expanded.
- Adding parameters to the MIME type doesn't prevent it from being
  evaluated as a JSON MIME type.

Note, some of the existing MIME type tests for */*+json still fail
because it seems the spec issue on these at
whatwg/mimesniff#112 hasn't been fully
resolved. I'll follow up with that separately.

Bug: 1132413
Change-Id: I682de01bbb14b53214fcc16f427a4875f8cfed7f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2841103
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#875274}
GitOrigin-RevId: 5d7b584b349398b5d42204b977fc0fca102aebe2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Clarify that we intentionally allow */*+json as JSON MIME types hiroshige-g/mimesniff
6 participants
@littledan @domenic @annevk @dandclark @hiroshige-g and others