modules icon indicating copy to clipboard operation
modules copied to clipboard

Remove restriction on '/' prefixes

Open bmeck opened this issue 5 years ago • 28 comments

Given that we have self referential packages, and "imports" in package.json. In addition since // is reserved I think we should remove the reservation since it will by the default URL parser be treated as protocol relative.

bmeck avatar Aug 05 '20 13:08 bmeck

To be clear, this is about direct import specifiers (https://nodejs.org/api/esm.html#esm_import_specifiers), e.g.:

// imports file:///usr/local/etc/mytool.config.mjs, assuming import.meta.url has file:// protocol.
import config from '/usr/local/etc/mytool.config.mjs';

I think that makes sense. Given "imports", I don't see a strong reason to reserve design space here (IIRC package-relative was the only thing ever discussed for this spot).

hybrist avatar Aug 05 '20 14:08 hybrist

Wait, why would we want to allow this? What are the use cases for root-absolute specifiers?

I can't conceive of a time in CJS where requiring from a /-prefixed path wasn't a bug (or bad design, as was later revealed when it was ran on a different machine).

ljharb avatar Aug 05 '20 17:08 ljharb

@ljharb because it adds consistency and I cannot think of a reason to disallow these specifiers on concrete concerns if we are not reserving them for future features. Any future feature using those prefixes not matching the generally accepted behavior of URL resolution would be bad so it isn't like we can use the feature for other purposes beyond pinning to some kind of root.

bmeck avatar Aug 05 '20 17:08 bmeck

I absolutely agree that if / is allowed as a non-// prefix, it would have to function as an absolute path, but I don't think "consistency" is a good argument to enable something that is almost always a footgun.

ljharb avatar Aug 05 '20 17:08 ljharb

@ljharb why is it a footgun? is there clear reasoning here or is it just a linting issue?

bmeck avatar Aug 05 '20 17:08 bmeck

It's a footgun because it's not project-portable or machine-portable; any time you have code that uses /, it's highly likely to break on someone else's machine (including a different docker image, host vs guest, if you migrate from one computer to a new one, etc).

ljharb avatar Aug 05 '20 17:08 ljharb

@ljharb that statement is true for any given one off script that isn't installed via a package manager and uses bare specifiers for things. I do see utility in various things like the ability to import '/etc/config/foo.json'; regardless.

bmeck avatar Aug 05 '20 18:08 bmeck

I have definitely imported things using absolute paths. If it's top-level application code that will only ever run in a docker container, I don't see anything wrong with importing something (like a config file) from an absolute path. I haven't seen strong evidence that this behavior caused issues in require but have seen people use it there for - what I would consider - valid reasons. But that's admittedly anecdotal.

hybrist avatar Aug 05 '20 18:08 hybrist

I have some thoughts on this topic, hopefully, it will help in coming to a conclusion. :)


Behavior # 1 LAMP server

Assuming content-type headers are set correctly on a standard-issue LAMP web server, the following should be possible.

import * as ns from '/foo.mjs';

This would import foo.mjs located at /var/www/html/foo.mjs a.k.a. https://example.com/foo.mjs.

AFAIK (note: I don't use node as a web server) Node.js doesn't have a directory like this, so we can't really mimic this behavior.


Behavior # 2 Flags

IMO, Node.js should be able to access the filesystem root. After all, one of the major advantages Node.js has over Deno is that Node.js does not put access to the filesystem, network, or environment behind flags like Deno. Because of this lack of flag necessity, dependencies can access the filesystem root. (BTW, I think this lack of flag + package ecosystem is what allows Node.js to compete w/ Python for back-end scripting language market share.)


Funny enough, this (access to the filesystem root) is already working as I would expect on OSs w/ POSIX paths, so changing it to something different now might be a bit awkward.

In closing, this is just how it seems to me at the moment, there could be things I'm unaware of (i.e., impossible to determine filesystem root dir in some rare OSs, et al.) — please take w/ grain of salt there could be other things I'm unaware of!

DerekNonGeneric avatar Aug 11 '20 16:08 DerekNonGeneric

AFAIK (note: I don't use node as a web server) Node.js doesn't have a directory like this, so we can't really mimic this behavior.

Node.js does have a "directory" like this - because modules in node use the file: protocol and host-relative resolution of / works the same here as it does in HTTP URLs. Resolving /zapp relative to file:///foo/bar is well-defined across platforms. Most importantly it's not generally a path relative to the system root, not even on OSs with POSIX paths. Resolving /a%20b.js will not resolve to the file with POSIX path /a%20b.js. It will resolve to a "completely" different path: /a b.js. Those are two different files.

The fact that Apache may serve some of the http: URLs from some directory on disk is a special case with one particular configuration. E.g. Apache could serve some URLs from a constant string in the config or from running a CGI script or from various directories depending on URL patterns. In terms of resolution, that shouldn't really matter.

hybrist avatar Aug 11 '20 16:08 hybrist

That node's implementation uses URLs doesn't mean that node module specifiers are conceptually URLs - they're file paths (which happen to be a subset of URLs).

ljharb avatar Aug 11 '20 16:08 ljharb

@ljharb the file specifiers are URLs, not paths, they do have fragments taken into consideration.

bmeck avatar Aug 11 '20 16:08 bmeck

I realize they are URLs. I'm saying most users will never think of them that way.

ljharb avatar Aug 11 '20 17:08 ljharb

@ljharb people are using this preservation of URL aspects for things like cache busting or ferrying meta-data already, it isn't something we can decouple.

see docs

bmeck avatar Aug 11 '20 17:08 bmeck

That node's implementation uses URLs doesn't mean that node module specifiers are conceptually URLs - they're file paths (which happen to be a subset of URLs).

I think it would be dangerous if we allow users to think that they are file paths. If allowing / reinforces that notion, then I'd consider it an argument to keep / "forbidden". Code like import(path.resolve(filename))) may look at first glance like it should work but it will not be portable or safe. It will only work if both filename and the working directory happen to only contain URL-safe characters that are interpreted the same way in both filenames and file: URLs. True for many common filenames but not true for arbitrary filenames.

hybrist avatar Aug 11 '20 20:08 hybrist

I think it would be dangerous if we allow users to think that they are file paths. If allowing / reinforces that notion, then I'd consider it an argument to keep / "forbidden".

Something perhaps worth noting is that MDN's very first example of a module specifier in their documentation page on static import statements uses a leading /. 😕 Could it be that this is just a path in this case?

For some reason it seems to me that we may have a feature missing that's supposed to control what a specifier with a leading / is supposed to be relative to (e.g. somehow being able to set a DocumentRoot of sorts)? It would function by passing that directory to something akin to require.resolve()'s options.paths <string[]> to determine where to start looking. 🤔

It could take various options:

  • FileSystemRoot
  • PackageScopeRoot
  • UserHome
  • Temp
  • AppData
  • etc.

DerekNonGeneric avatar Aug 11 '20 21:08 DerekNonGeneric

@DerekNonGeneric that's because MDN is geared towards the web, not node, and on the web, that's a domain-relative URL.

I feel very strongly that if a leading / is allowed, it can only ever mean "the filesystem root", because that's what it already means in CJS.

ljharb avatar Aug 11 '20 22:08 ljharb

I feel very strongly that if a leading / is allowed, it can only ever mean "the filesystem root", because that's what it already means in CJS.

Yes, for file: referrers, it would roughly be "the file system root". But if we explain it as "just like CommonJS", we're asking for trouble. Because while it may be relative to the file system root, it's not a file system path (!). The following will load two different files, even though the specifiers are equal and both interpreted relative to the file system root:

require('/tmp/a%20b.js'); // the file "/tmp/a%20b.js" ("file:///tmp/a%2520b.js")
import('/tmp/a%20b.js'); // the file "/tmp/a b.js" ("file:///tmp/a%20b.js")

If we don't think people can differentiate between those two different paths, maybe we shouldn't allow a leading slash in import.

P.S.: I think we can expect people to think of imports as URL-based longer term. There's way more JavaScript-first devs than node.js-first devs and hopefully the former will become used to thinking in URLs when using import.

hybrist avatar Aug 11 '20 22:08 hybrist

@ljharb, I'm not opposed to keeping the leading / to mean relative to the filesystem root as you like. I quite like it that way too. I was simply proposing that there may be value in being able to override that default behavior via config (perhaps a pjson field).

Essentially it would be opt-in to override the default. I think the Node.js web server community could see some real value in this, but I'm still researching the status-quo atm. :)


Framework Status quo (maybe)
Express https://expressjs.com/en/resources/middleware/serve-static.html
Fastify https://github.com/fastify/fastify-static
Hapi https://futurestud.io/tutorials/hapi-how-to-serve-static-files-images-js-etc#servefilesfromadirectory
Nest https://github.com/nestjs/serve-static

/cc @wesleytodd as Express users could be into this?

DerekNonGeneric avatar Aug 12 '20 00:08 DerekNonGeneric

@DerekNonGeneric that seems like you just want to run a chroot around node, not that node would actually need to do anything.

bmeck avatar Aug 12 '20 17:08 bmeck

@DerekNonGeneric that seems like you just want to run a chroot around node, not that node would actually need to do anything.

@bmeck, that's not cross-platform, I don't understand how that is the suggestion here.

DerekNonGeneric avatar Aug 18 '20 02:08 DerekNonGeneric

Wait, what does / in CJS on Windows map to now??

ljharb avatar Aug 18 '20 02:08 ljharb

FS Root, just like you said, right?

DerekNonGeneric avatar Aug 18 '20 02:08 DerekNonGeneric

C:\>node --eval "require('/index.js')"
C:\index.js:3
export {};
^^^^^^

SyntaxError: Unexpected token 'export'

DerekNonGeneric avatar Aug 18 '20 02:08 DerekNonGeneric

Right, but "c:" isn't necessarily the fs root on windows - where linux has /, and all volumes are mounted under that, windows has up to 26 (i think) volume roots, and i'm not aware there is any pathable location that contains all of them.

Perhaps it maps to the root of the working directory's drive?

ljharb avatar Aug 18 '20 02:08 ljharb

@DerekNonGeneric

that's not cross-platform, I don't understand how that is the suggestion here.

It works on everywhere but windows natively and windows has workarounds for this via cygwin, windows subsystem for linux, etc. The point was that the request doesn't seem to be something that node needs to specifically handle, and the details of chroot are quite complex.

bmeck avatar Aug 18 '20 03:08 bmeck

@DerekNonGeneric there is no point that I stated that windows is second class.

bmeck avatar Aug 18 '20 03:08 bmeck

After reading the other recent thread, I think my bigger concern is with *nix platforms where people may confuse /some/file/url/path with /some/os/file/name and then run into issues with the different encoding. It seemed very hard to get the point across that there are (some) practical differences between the two because they look so similar.

I would be in favor of a clear warning in our resolver when resolving a host-relative URL against a file: URL, suggesting people use an explicit protocol. Not a really strong opinion (there's plenty of common file naming patterns where it's fine) but it may be worth it.

hybrist avatar Aug 18 '20 16:08 hybrist