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.

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

Implicite assert type #4

Closed
JeremiePat opened this issue Sep 18, 2020 · 2 comments
Closed

Implicite assert type #4

JeremiePat opened this issue Sep 18, 2020 · 2 comments

Comments

@JeremiePat
Copy link

After reading the proposal, and as a web developer I was slightly uneasy with an aspect of the JSON module syntax:

import json from "./foo.json" assert { type: "json" };

Because of the .json extension it feels like having the assert is, in this specific case, somewhat redundant.

I get through the whole security thread about the necessity to explicitly state the nature of what is imported and fully agree on the risk and mitigation suggested there. That said, I tend to think that we could introduce some basic heuristic to ease developer work (and adoption).

I would tend to suggest that if the URL in the import ends up with a .json extension we should assume assert { type: "json" } because it is the most basic web developer intent. On the other hand if a developer decide to serve some non-json content with a .json extension, then we should just fail at parsing the content as JSON. But if the developer think that this is something legit then in that case, it would make sens to ask them to opt-in for interpretation with for example assert { type: "ecmascript" }. This is counter intuitive but could be necessary in some weird cases.

So to summaries, I'm suggesting something along that line:

// All the following import should be parsed a JSON and nothing else.
import json from "./foo" assert { type: "json" };
import json from "./foo.js" assert { type: "json" };
import json from "./foo.whatever" assert { type: "json" };
import json from "./foo.json" assert { type: "json" };
import json from "./foo.json";

// All the following import should be parsed and interpreted as ECMAScript
import ems from "./foo";
import ems from "./foo.js";
import ems from "./foo.whatever"; // where ".whatever" isn't ".json"
import ems from "./foo.json" assert { type: "ecmascript" }; // Active optin for what is supposed to be JSON

Regarding dynamic import I would not get into that same suggestion for a simple fact: because the first argument of import() can be hidden behind a variable name, the intent of the developer can't be as clear as they think and in that case requiring an explicit type parsing is safer.

@dandclark
Copy link
Collaborator

What you suggest is actually permitted by this proposal in its current form. This proposal requires that if a type: "json" assert is present, then the host must treat the module as JSON, or fail the import. But it doesn't require the converse, that if a host treats an import as JSON then a type: "json" assert must be present. So hosts are free to interpret import json from "./foo.json"; as a JSON module, and I expect that some non-HTML hosts who don't have the same security concerns as the web will choose to do so.

Reviewing various comments (1), (2) in the security thread, I think it's unlikely that stakeholders there will be willing to introduce any reliance on file extension even in this scoped case. There is long precedent against it, and investigation has shown that there are surprising mismatches between file extension and actual content type on the web today.

If you want to make this case, though, this JSON modules proposal permits hosts to omit the assertion, so the people to convince would be the web folks in that security thread who are against any use of the file extension.

@JeremiePat
Copy link
Author

Thanks for the clarification. It wasn't that obvious that the specification allowed implicite type: "json". I'll get more into the security side of it. Thanks again 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants