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

fetch: Enable fetch via file URL (under flag) #2751

Open
micheleriva opened this issue Feb 13, 2024 · 11 comments
Open

fetch: Enable fetch via file URL (under flag) #2751

micheleriva opened this issue Feb 13, 2024 · 11 comments

Comments

@micheleriva
Copy link

What is the problem this feature will solve?

When trying to perform a fetch request to a file:// schema, fetch fails with the following error:

cause: Error: not implemented... yet...

Looking at the WHATWG specs, it seems like there is no specification for this schema:

"file"
For now, unfortunate as it is, file: URLs are left as an exercise for the reader.

When in doubt, return a network error.

and undici follows this behaviour correctly.

There are many use cases where fetching a local file using fetch would be handy, and this is particularly true for WebAssembly modules.

Right now, libraries like wasm-pack (a Rust to WASM lib) have to perform different builds for browsers and Node.js, since Node.js is not able to fetch the compiled WASM binary from the file system.

Other runtimes and browsers handle this without any particular problem, but except for Deno (which has its own security model), I do understand the security implications of fetching a file from the filesystem.

What is the feature you are proposing to solve the problem?

With the introduction of the new Node.js permission APIs, we could enable fetch on the file system under an option, such as allowFileUrl: true.

That way, the user could explicitly enable fetching a local file, acknowledging the security implications that come with it.

About the actual implementation, I see that WinterCG also hasn't specified fetch for URLs with a file:// schema: https://fetch.spec.wintercg.org. Therefore, I'd also ask if that's the case to discuss how actually to implement this - if interesting for anyone.

What alternatives have you considered?

No response

@micheleriva micheleriva changed the title fetch: Enable fetch via file URL fetch: Enable fetch via file URL (under flag) Feb 13, 2024
@mcollina
Copy link
Member

I’m ok in supporting this under a allowFileUrl option in fetch.

@aduh95
Copy link
Contributor

aduh95 commented Feb 13, 2024

Related: wintercg/fetch#5

@mcollina mcollina transferred this issue from nodejs/node Feb 13, 2024
@mcollina
Copy link
Member

Moving to Undici repository

@metcoder95
Copy link
Member

What would be the shape of it, and its considerations?

From what I've seen from the issue shared by @aduh95 the main security points seem valid; Chromium seems to have some initial implementation.

All assuming its behind a flag, ofc

@KhafraDev
Copy link
Member

If we implement this we will deviate from literally every other platform. We should either let wintercg handle it, or let the spec handle it. -1

@micheleriva
Copy link
Author

@KhafraDev I agree with you that it’d be optimal to have wintercg to formalize this. What do you mean by “will deviate from literally every other platform”? Other runtimes (Bun and Deno for sure) implement a this already

@KhafraDev
Copy link
Member

From a quick glance:

  1. Bun allows fetching file:// urls with any method, Deno throws on non-GET calls.
  2. Chrome only allows this for extensions, if users opt-in to it.
  3. Firefox and Safari do not implement this.

If we implement our own implementation (ie. with a non-standard option), that's another implementation that differs from every browser and every server environment. No implementation is spec compliant because there is no spec, which will also cause issues when a spec is written, and these platforms now have to break their users to match it.

There are also a number of edge cases that haven't been decided on wintercg/fetch#5 (comment)


I'm quite confused as to your mention of wasm-pack

Right now, libraries like wasm-pack (a Rust to WASM lib) have to perform different builds for browsers and Node.js, since Node.js is not able to fetch the compiled WASM binary from the file system.

Browsers aren't able to do this either. But from looking at the code, they instead download the binary from a url, which will work in node and browsers, ignoring CORs errors. Maybe you could expand on this?

@micheleriva
Copy link
Author

No implementation is spec compliant because there is no spec, which will also cause issues when a spec is written, and these platforms now have to break their users to match it.

This is very true, but that's also the purpose of experimental APIs. I personally think that missing APIs cause more damage to the user than experimental APIs.


Browsers aren't able to do this either. But from looking at the code, they instead download the binary from a url, which will work in node and browsers, ignoring CORs errors. Maybe you could expand on this?

Correct. Right now, if you use wasm-pack, you have to run at least three compilation processes:

  1. For browsers and runtimes. Since the mainstream runtimes support fetching from a file, they can share the same bindings built for the browsers.
  2. ESM bindings for Node.js, which doesn't support fetch for local files
  3. CJS bindings for Node.js, which doesn't support fetch for local files

The only real blocker I found to enable a single compilation process, it's by allowing Node.js to fetch the compiled WASM binaries from the filesystem at runtime.

I wouldn't focus too much on the wasm-pack use case only, though. I'm using Deno a lot and being able to avoid calling server-side methods (such as fs.readFileSync for Node.js) allows you to write code once and use it both on the frontend and the backend, which is kinda handy.

I understand the spec is basically non-existent, yet, as I said earlier, an experimental API is far better than no API at all to me.

But of course, that's a personal opinion and I'm happy to discuss

@KhafraDev
Copy link
Member

But we aren't missing any apis, we implement fetching file urls exactly as the spec tells us to (throw an error). I would claim that Deno and Bun, by implementing this, cause much more damage in the long run (in the style of mootools or others that added methods to Array/Object prototypes, preventing the tc39 from adding certain methods).

I'm also still not sure how fetching file urls can be used in both browsers and runtimes, but that doesn't seem relevant.

@aduh95
Copy link
Contributor

aduh95 commented Feb 14, 2024

I'm also still not sure how fetching file urls can be used in both browsers and runtimes, but that doesn't seem relevant.

tbf it doesn't need to be supported in browsers to improve interop, the use case I have in mind is fetch(new URL('./some/relative/path', import.meta.url)), where import.meta.url would be a HTTPS URL in browsers and a file: URL on runtimes.

@KhafraDev
Copy link
Member

Thanks for the use case, makes sense now! Interop with other server environments is precisely the point of WinterCG. In the past we've followed Deno, which has only ever caused issues for us (namely security vulnerabilities, edge cases, more edge cases, other issues). But the proposed solution is not interoperable with other runtimes as it requires explicitly opting in to it, for every request - the other two runtimes just work. As in, you would have to change your code for it to work in node, whereas you don't for other runtimes, nor will you when/if it's ever added into the spec.

I mentioned earlier, but Bun isn't entirely interoperable with Deno, and there are also a number of questions about the api that WinterCG should solve before we even consider implementing it.

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

5 participants