dop251 / goja_nodejs

Nodejs compatibility library for Goja
MIT License
329 stars 78 forks source link

require directory not working #5

Closed artvel closed 4 years ago

artvel commented 7 years ago

require directory not working as in: https://stackoverflow.com/questions/5364928/node-js-require-all-files-in-a-folder

nwidger commented 4 years ago

@dop251 I have been looking into writing SourceLoader(s) which would implement some or all of Node's module loading algorithm as described here: https://nodejs.org/api/modules.html#modules_all_together. I believe this would address this issue so I'm posting here, but if this isn't the right place for that discussion I'm happy to open a new issue.

In reading over the pseudo-code in the link above and after experimenting a bit, I see two issues in trying to implement Node's module loading algorithm as a SourceLoader:

  1. Node's module loading algorithm uses the path of file that is calling require() but this information is not known to a SourceLoader as it only gets passed the require() calls' path argument. I realize that in some cases (i.e. a call to require() called from a call to runtime.RunString()) this information might not be available, but I believe in other cases that information is known by the runtime but is not currently available outside of the goja package.

  2. 7 introduced passing the path passed to the SourceLoader through filepath.Clean. Unfortunately, this causes require('./foo') to look exactly like require('foo') to a SourceLoader since the leading ./ is stripped off: https://play.golang.org/p/H432MdUXpnQ. The details in the PR are very sparse and I do not have a Windows machine to test with, thus I'm not sure how best to address this issue without risking bringing back the original issue.

Perhaps in the end, we'll only be able to provide SourceLoaders for parts of the algorithm, but I wanted to check with you first.

dop251 commented 4 years ago

SourceLoader is not the right place to implement this. Its only job is to load a source code given a full, canonical path. You'd implement a custom SourceLoader if you needed to keep you sources somewhere else other than the filesystem: database, S3 bucket, zip archive, pre-defined test mocks, etc., or if you need additional security for a sandbox, or if you generate them on the fly...

The algorithm needs to be implemented in Require(). As for knowing the path of the currently running code, there is Program.src.name, but it's not presently exposed. Should not be difficult though.

nwidger commented 4 years ago

@dop251 Okay, thanks for the reply. It wasn't obvious to me that the job of SourceLoader was meant to be so restricted, from reading the code it seemed reasonable to assume that if a module isn't native, then the SourceLoader would do the job of searching for the module on the filesystem (local or otherwise). Perhaps part of your response could be turned into some documentation for SourceLoader to make your vision for how this is supposed to work clearer?

Would it make sense to introduce a new type that would implement the module search algorithm? Something like

type ModuleSearcher func(path, from string, srcLoader SourceLoader) ([]byte, error)

a registry could be created with a custom ModuleSearcher, otherwise a default one that implements the Node algorithm linked above would be used.

dop251 commented 4 years ago

Why would you need to make Require algorithm pluggable? There is only one way require() can behave in nodejs. And if you want it to behave differently for some reason, just write your own require()

nwidger commented 4 years ago

That's a fair assessment, although I suppose someone could use the same line of thinking to argue that SourceLoader shouldn't be pluggable, either. My thinking for something like ModuleSearcher is that applications using goja might like to offer the use of require() to untrusted scripts but with some restrictions. For example, you want these scripts to be able to require() modules but to only load a set of "core" modules that come from a single, well-known directory that you control. Maybe you don't want the scripts to be able to load modules that are found using absolute/relative imports paths, hunting upwards to find node_module directories, searching through directories in NODE_PATH, etc.

dop251 commented 4 years ago

All this can and should be done in the SourceLoader. I just don't see why you need another layer.

nwidger commented 4 years ago

That's fair, a custom SourceLoader could be used to prevent loading modules that the VM shouldn't have access to. I might take a stab at a PR for this then, if you'd be willing to accept one and think we've fleshed things out enough. Do you have any thoughts on the filepath.Clean issue?

dop251 commented 4 years ago

filepath.Clean should still happen but at some later stage