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

Static non-code resources in modules #4001

Closed
seishun opened this issue Feb 14, 2020 · 19 comments
Closed

Static non-code resources in modules #4001

seishun opened this issue Feb 14, 2020 · 19 comments
Labels

Comments

@seishun
Copy link
Contributor

seishun commented Feb 14, 2020

Let's say I want to write a module that communicates with some service using Protobuf.

In Node.js, the npm package would contain .proto files which the module would load directly from the file system (using __dirname).

What's the deno equivalent? It seems import works only for code and fetch wouldn't be cached.

@nayeemrmn
Copy link
Collaborator

The import system shouldn't be used for this, but being able to use the cache system for non-code resources is a very good point (may as well change the title). That would be applicable to suggestions in #3448.

@ry
Copy link
Member

ry commented Feb 14, 2020

import.meta.url provides more-or-less the same functionality as __dirname.

@seishun
Copy link
Contributor Author

seishun commented Feb 14, 2020

For a module imported via a URL, this will be the module's URL, right? It won't be a local directory where you can directly read files from, like in Node.js.

@kitsonk
Copy link
Contributor

kitsonk commented Feb 14, 2020

Then you would use Deno.cwd().

@seishun
Copy link
Contributor Author

seishun commented Feb 14, 2020

Deno.cwd() won't contain the module's resources.

@kitsonk
Copy link
Contributor

kitsonk commented Feb 14, 2020

Only the imported resources are ever retrieved, so if you are loading a remote module, why are you trying to read local resources that don't exist?!

@seishun
Copy link
Contributor Author

seishun commented Feb 14, 2020

Only the imported resources are ever retrieved

Correct, and since only code files can be imported, there is seemingly no way to retrieve non-code resources required by the module other than to use fetch. In Node.js, all resources bundled with the module are local, so it's not an issue there.

@kitsonk
Copy link
Contributor

kitsonk commented Feb 15, 2020

That is very intentional, as allowing privilaged access to non-code resources would break the security model. Using import.meta.url as a relative base with fetch and --allow-net is the way to go.

@seishun
Copy link
Contributor Author

seishun commented Feb 15, 2020

That is very intentional, as allowing privilaged access to non-code resources would break the security model.

Could you elaborate?

Using import.meta.url as a relative base with fetch and --allow-net is the way to go.

See #4001 (comment):

It seems import works only for code and fetch wouldn't be cached.

@kitsonk
Copy link
Contributor

kitsonk commented Feb 15, 2020

That is very intentional, as allowing privileged access to non-code resources would break the security model.

Could you elaborate?

Yes. We are fairly strict with remote modules that their media type needs to match the set of known modules that we process, and we process those modules, we then inject those modules into the runtime. Runtime code has 0 access to this code directly, including JSON, meaning that it can simply request code to be retrieved and added to the runtime, and the runtime code only sees the results of the parsed code. This is the only implied system access that comes out of the box. Everything else requires explicit permissions. If we had a "backdoor" to load any network resources that wouldn't be a very secure model.

It seems import works only for code and fetch wouldn't be cached.

@seishun see #3756

@kt3k
Copy link
Member

kt3k commented Feb 15, 2020

@seishun
I'm not sure this is an ideal solution but, If your module is in local disk, import.meta.url is like file:///path/to/module.ts. So you can get the absolute path (=~ __dirname + filename) of it by stripping the first file:// part.

@seishun
Copy link
Contributor Author

seishun commented Feb 15, 2020

If we had a "backdoor" to load any network resources that wouldn't be a very secure model.

Would a proxy that wraps arbitrary resources in a JS module default export as described in #3756 (comment) be considered a backdoor?

@seishun see #3756

From deno manual:

Remote code is fetched and cached on first execution, and never updated until the code is run with the --reload flag. (So, this will still work on an airplane.)

It's unclear how the proposed fetch cache will work exactly, but the quoted behavior would be desirable for non-code resources too. Otherwise it's not an ideal solution.

@kitsonk
Copy link
Contributor

kitsonk commented Feb 15, 2020

Would a proxy that wraps arbitrary resources in a JS module default export as described in #3756 (comment) be considered a backdoor?

Possibly, great for discussion on that issue, as we wouldn't want to introduce one.

It's unclear how the proposed fetch cache will work exactly, but the quoted behavior would be desirable for non-code resources too. Otherwise it's not an ideal solution.

Again, great for discussion on that issue.

@seishun seishun changed the title Non-code resources in modules Static non-code resources in modules Feb 16, 2020
@seishun
Copy link
Contributor Author

seishun commented Feb 16, 2020

Possibly, great for discussion on that issue, as we wouldn't want to introduce one.

That issue is about fetch, I don't think the usage of import I described is relevant there.

Again, great for discussion on that issue.

Left a comment, but that issue is specifically about fetch cache, which is just one of possible solutions to the problem discussed here.

@brandonkal
Copy link
Contributor

As far as I am aware, browsers do not handle arbitrary resources through import. I would like to see the ability to statically import YAML as a JS object but that isn't portable to browser runtimes.

Now I use a proxy and parse its default export text as YAML. That works fine for now because I can use Deno's existing import cache.

It would be the same for other resource types. If browsers support their static import, Deno should as well. Otherwise it should not diverge here.

@kitsonk
Copy link
Contributor

kitsonk commented Feb 16, 2020

As far as I am aware, browsers do not handle arbitrary resources through import.

Correct. We play loose by converting JSON (we also support JSX, TSX). I would say we stop there, because everything else could potentially lead to a security issue and code injection.

@teleclimber
Copy link

It's unclear how the proposed fetch cache will work exactly, but the quoted behavior would be desirable for non-code resources too. Otherwise it's not an ideal solution.

Again, great for discussion on that issue.

For the sake of discussion let's consider the original scenario where there is a remote module that depends on non-importable resources like .proto. You certainly want the code and the resource files to be "in sync". Meaning that they are the same version.

So would --reload delete the fetch cache too? Otherwise it's easy to see how the two get out of sync. You'd run deno with --reload and it would get a fresh new version of the code, but use the old cached copy for .proto. Hmm, not good.

It's also possible that if fetch behaves like a browser cache but import does not, then you might end up reloading your .proto because they're expired but not your code files, resulting in a mismatch again.

@stale
Copy link

stale bot commented Jan 6, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 6, 2021
@kitsonk
Copy link
Contributor

kitsonk commented Jan 6, 2021

With import assets being added to JavaScript, any changes to this space will be aligned to Deno supporting that. Closing in favour of #7623.

@kitsonk kitsonk closed this as completed Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants