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

Semantics of this feature in Node.js #25

Open
littledan opened this issue Nov 9, 2019 · 20 comments
Open

Semantics of this feature in Node.js #25

littledan opened this issue Nov 9, 2019 · 20 comments

Comments

@littledan
Copy link
Member

littledan commented Nov 9, 2019

Note: The semantics of this proposal in Node.js would be determined by Node collaborators and the Node Modules Team, not in this repository. This issue is purely for requirements-gathering/brainstorming.

A lot of the discussion in the issues seems to relate to how this feature would integrate into non-Web environments. For concreteness, I'm going to talk about how this might integrate into Node.js, since I know slightly more about it than other non-Web environments (but I am no Node expert). I see two general paths for the interpretation. (Note, these are not the only possible paths, they're just two that I thought of to initiate discussion)

Option A: Maximizing for similarity with the web

If we want to be maximally compatible with the web -- that is, code written for Node.js would work on the Web without a build step to handle this aspect of semantics -- then we could simply use the logic in #24, but with s/MIME type/file extension (and possibly more information in package.json)/. The file extension used here would be after some resolution is done, as described in #4, so it may not be what's textually included in source. This would mean that importing JSON modules would require type: "json" to be textually in JS source that's used in Node.js, which differs from traditional ES6 module -> CJS transforms (which of course didn't have this syntax, but JSON modules were present anyway).

Option B: Maximizing for terseness/non-redundancy

On the other hand, we could make type: optional for non-JS all module types in Node.js. The semantics would be based on the template of Option A, but with the following change: if no type: is provided, then there is no check done, and processing is based just on the file extension (and possibly more information in package.json). This would mean that a build step is expected to add the with type: "json" or whatever else when building for the web.

How this would interact with other alternatives for where to put the attributes

If the attributes are put in an out-of-band file, or in the module specifier, or in a single string, Options A and B are still both available. They make different tradeoffs about web compatibility vs lack of redundancy. This proposal would permit Node.js and other environments to make this decision themselves, regardless of the choice of format for where to put the metadata.

@ljharb
Copy link
Member

ljharb commented Nov 9, 2019

node has no need to use this type information at all; it would have to be optional for everything.

@littledan
Copy link
Member Author

@ljharb Are you saying you agree with Option B, or do you have concerns with it? Under Option B, the type is optional, though if it's provided, it's checked for.

@ljharb
Copy link
Member

ljharb commented Nov 9, 2019

Option B says optional for non-JS types; in node it’d be optional for every type.

@littledan
Copy link
Member Author

Heh, the idea is that it's optional for non-JS types because of course it's just not provided for JS types... so yes, it'd be optional for all types.

It sounds like you're speaking on behalf of Node about what its semantics would be. Has the Node Modules team made a decision about this yet?

@littledan
Copy link
Member Author

I've edited the original post to make the clarification.

@ljharb
Copy link
Member

ljharb commented Nov 9, 2019

To clarify, I’m not speaking on behalf of node or the modules group; however, i can’t imagine that the UX tax of providing a type inline would gain consensus in either venue.

@littledan
Copy link
Member Author

Is Option B acceptable to you, in your personal opinion about developer UX (modulo the question of how unknown types are processed)?

@ljharb
Copy link
Member

ljharb commented Nov 9, 2019

The concerns motivating this proposal don’t exist in node, so if the syntax exists in the language, option B seems like the only non-disruptive way it could work (since compatible web code still needs to be able to work in node)

@bmeck
Copy link
Member

bmeck commented Nov 11, 2019

I'd leave this to the node and the modules WG rather than attempting to state only a few possible options.

@littledan
Copy link
Member Author

Yes, I definitely didn't mean that these are the only two options, and am supportive of this being decided by the Node Modules Team. I wanted to start the discussion, though. You can see that I've also filed issues about the web semantics, even though those would also happen in web standards bodies.

@bmeck
Copy link
Member

bmeck commented Nov 11, 2019

@littledan I've added this to the modules WG. I'm just not comfortable speaking on behalf of the WG as neither of those options seem likely in personal viewpoint.

@jkrems
Copy link

jkrems commented Nov 11, 2019

To add to the node use case: There have been discussions around supporting https: module loading in node, either built-in or as a relatively common loader. So I'm personally not convinced that node is fundamentally different from the web here. The same kind of security concerns could apply in the future and making it optional now could be an issue then.

@bmeck
Copy link
Member

bmeck commented Nov 11, 2019

The same kind of security concerns could apply in the future and making it optional now could be an issue then.

Of note, some people might be skeptical of the need for mandating the type attribute rather than it being an opt-in assertion, myself included. IDK how this would affect other environments. Even if we support loading via https: the security models differ between the runtimes and the mitigations also vary.

@jkrems
Copy link

jkrems commented Nov 13, 2019

Of note, some people might be skeptical of the need for mandating the type attribute rather than it being an opt-in assertion, myself included.

I would count myself in that camp. But to me that applies to the browser as well: In many (most?) cases, I would expect the JSON being imported to be from the same domain etc., controlled by the app owner. So forcing verbose syntax on them (instead of making it an opt-in assertion), feels punitive with little actual security win. Which is always likely to motivate people to find workarounds, undermining the few security wins left.

@xtuc
Copy link
Member

xtuc commented Nov 14, 2019

@jkrems If you think that a JSON file on your web server will only be served with a Content-Type: application/json (and be actual JSON), there's no need to enforce it by having the type attribute.

Serving your website over some CDN is already a moving piece that might be misconfigured or malicious. It's more common that you might think. content-type-vs-file-extension can give you an idea.

@jkrems
Copy link

jkrems commented Nov 14, 2019

Serving your website over some CDN is already a moving piece that might be misconfigured or malicious.

I'd like us to stay realistic though. If you make the argument "but what if your CDN is compromised", then that's entirely out of scope for this issue. Because it's unlikely that somebody has a CDN just for their JSON files. So why would the attacker bother to intercept the JSON load? And if your CDN is misconfigured, with or without a type annotation your website will be broken in fairly obvious ways. I'm aware of bad content-type headers. The question is: How likely is it that having this annotation will make a difference in the face of typical bad content-type scenarios. I have a hard time seeing a practical difference here apart from the safety assertion about arbitrary code execution.

@devsnek
Copy link
Member

devsnek commented Nov 14, 2019

I'd also prefer a noexecute attribute instead of arbitrary metadata, but I think people are unlikely to go for that.

@xtuc
Copy link
Member

xtuc commented Nov 15, 2019

@jkrems I agree that my example was not great.
My point was that there are cases where a browser can misinterpret a resource. It's related to security (json vs js, which noexecute fixes) but also provide a good UX for developer mistakes (css vs html module, which type attribute fixes).

@ByteEater-pl
Copy link

Is the intended semantics that type: "json" with a module specifier pointing to a file with js, mjs or exe extension causes it to be interpreted as JSON anyway? That could be useful and actually, as far as I know, add power to Node.js.

@bmeck
Copy link
Member

bmeck commented Mar 29, 2020

@ByteEater-pl it wouldn't affect how the module is interpreted. It would only assert it is the expected type.

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

No branches or pull requests

7 participants