-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
file:// URLs #75
Comments
+1 |
There might be some security implications. |
It's not on my roadmap at the moment, |
This should work as it works in Edit: This is not defined in the spec https://fetch.spec.whatwg.org/#basic-fetch |
I guess we've decided that |
Just to clarify, if this works consistently across major browsers we will re-open. But more as a clear warning that we don't support |
@bitinn @TimothyGu I don't I understand the aversion to If we're on the server side (using Node.js + Express), and I have to call another rest endpoint whose swagger is not easily obtainable, why can't I take care to put it under source control and simply use it instead of the undownloadable spec from the service I'm calling? This limitation seems arbitrary. Please clarify its justification. |
|
The Response constructor can take a Readable stream so why not simply do something like this? let {Response} = fetch
let stream = fs.createReadStream('myfile.txt')
let promise = new Response(stream).text() I don't think file protocol should be supported, sorry |
I don't see why it can't be supported behind an option to enable it. |
NB: I'm @matthewadams, not @matthew-andrews. Sorry for the noise, @matthew-andrews @TimothyGu I'm confused by your response. Please see swagger-api/swagger-js#1044 for justification. Do you have a pointer for Node.js v8 supporting |
@jimmywarting I still don't understand why |
As you could easily switch to using |
@jimmywarting the spec is already broken here: https://github.com/bitinn/node-fetch/blob/master/LIMITS.md |
+1 again ... it still seems a no-brainer, to me. URL stands for Uniform Resource Locator, after all. Tim Berners Lee would be rolling over in his grave. (If he were dead, that is.) |
Much thought have gone into this issue, for sure. I'd like to condense the most important reasons I believe URLs of file scheme should not be implemented in node-fetch. I would of course be thankful if @bitinn or @zkat would like to weigh in and offer their opinions here as well. 1. Accessing the local filesystem eases security breaches.Take @jimmywarting's (admittedly anecdotal and contrived) use case:
Really, I cannot believe many people are using node-fetch as a proxy (I might be wrong of course, but IMO there are many much better ways to create a proxy in Node.js). But I find it easy to imagine people who are using a URL from an untrusted source (the network for example) and just plug it into @stevenvachon proposed using an option to allow file URL scheme. That may solve this issue, but see my response below. 2. There is no recognized standards for interpretation of file-scheme URLs.As outlined in sindresorhus/got#227 filed by @stevenvachon, WHATWG's Fetch Standard offers no discussion of how file URL schemes should be interpreted on different platforms. In fact, in the PR adding documentation for Node.js Now that file URL scheme is supported officially in Node.js, I foresee a de-facto standard being established at least in the Node.js community. But then in less than a month when Node.js 8.0.0 is released, users can use 3. There is no precedent for support of file-scheme URLs in Node.js community.None of the most popular URL-to-response libraries I can think of in under a minute do not support file-scheme URLs:
4. Browser
|
I still pretty against it. I don't see why we want to wrap around Plus the issue 2 in Timothy's reply: Fetch Spec gave us nothing to work with on |
How to enable file URLs - https://gist.github.com/joshua-gould/58e1b114a67127273eef239ec0af8989 |
@bitinn Could you please reconsider this and unix socket support? Interacting with things like the REST API on the Docker daemon socket are impossible without it. |
@masaeedu Still a nope on File URL. But if you really need UNIX socket support, try got, they are modelled after Fetch. https://github.com/sindresorhus/got#unix-domain-sockets |
Unfortunately I can't use |
This comment has been minimized.
This comment has been minimized.
@TimothyGu replying to your comment #75 (comment)
That's news to me. Yes, Chrome disallows As your comment is from 2017 and my reply from 2019 there is the potential that something has changed since. But I definitely have used fetch on |
so if I want to fetch a ftp:// file, how should I go around doing this? |
Every once in a while I have the misfortune of using Where is Node ran? server side. right? What do you have access to in the server side always, a filesystem, right?? right??? SO, why exactly does this not support local filesystems? Why does this have to work like a browser? Who cares if this works like a browser?? If I wanted browser behavior, I would not have pulled a package called NODE-fetch. I would have pulled something called browser-fetch or something else. And what security implications exactly does this lack of support refer to? People like to throw this word around like a magic statement that makes bad decisions and architecture go away. If you can't point exactly what security implication you're trying to address, you are addressing absolutely nothing, just a waste of everyone's time. This is not even the only GH issue on this for this package. Clearly lots of people are suffering from this problem. On another thread a maintainer cited the reason for this due to "servers not having an origin URL". Excuse me, but then what is |
I understand that you're frustrated @3p3r and have had a difficult experience with node-fetch and just want to vent a little bit. However, it's important to keep a respectful tone when discussing issues with others on open source projects. Regarding your question about why node-fetch have decided not to support the such as in the example of building CORS proxies \w the fetch api. While I understand that you would like node-fetch to support the file:// scheme, it's important to consider the potential risks and drawbacks of doing so also. you can get around this with a simple wrapper around import fetch, { Response, blobFrom } from 'node-fetch'
import fs from 'node:fs'
import process from 'node:process'
const base = process.cwd()
export default async function fetchWrapper(url, options) {
const parsedUrl = new URL(url, base)
if (parsedUrl.protocol === 'file:') {
// If the URL starts with 'file://', read the file from the local file system
const filePath = decodeURIComponent(parsedUrl.pathname)
if (!fs.existsSync(filePath)) {
return new Response(null, { status: 404, statusText: 'NOT FOUND' })
}
const blob = await blobFrom(filePath, { type: 'application/octet-stream' })
return new Response(blob, { status: 200, statusText: 'OK', url })
} else {
// For all other requests, use node-fetch as usual
return fetch(url, options)
}
} |
I am absolutely not trying to bash the maintainers. I am so insignificant in the pool of 36M+ people that download this package that whatever I say makes no dent in anything. I am trying to point out a design flaw here. In a server environment where this package is used 99% of the time (and should be 100% of the time!), accessing resources over a network is most definitely less secure than accessing the local filesystem. You cannot argue that fetching network resources in Node is safer than accessing local filesystem. This behavior only makes sense in browser, because there are further safeguards for networks access. The browser platform fundamentally does not allow file access and its networking stack is strict due to all layers of standards that went into it all these years. But those networking safeguards do not exist in Node, so arguing that accessing network is safer in Node vs accessing local filesystem is moot. A CORS proxy built with node-fetch, hosted somewhere online should not have sensitive files next to it. Or it should not have filesystem permission setup in a way that you can fetch That won't make this package any less secure. If I could use node-fetch to cause stack overflow to execute arbitrary shell commands, now that would make this package insecure. |
Chrome (Chromium-based browsers) extensions support
|
Keep in mind Chromium-based browser supports |
Looks like > var scriptText = await (await fetch("file:///home/user/exports.js")).text();
undefined
> scriptText
"const fn = () => 123;\nexport {fn};"
> const mod = await import(URL.createObjectURL(new Blob([scriptText], {type:"text/javascript"})));
undefined
> mod
[Module: null prototype] { fn: [Function: fn] }
> mod.fn()
123 |
time to migrate to built in fetch |
As you may already know Some of all the original members and authors have all agreed to not support file: cuz of security concerns. |
It makes sense that it is more "secure" to fetch local files than to expose the entire system to remote networking requests.
Handle the request like any other protocol. The developer should be in control over the API surface. The user determines what is "secure" and what is "insecure", not some external specification controlled by a handful of individuals who are not in control of the users' machine.
That's possible anyway. In the browser using an extension. Outside of the browser using a proxy. Again, the user makes that decision on their own machine. Full-duplex streaming is a little more involved specification-wise, though shouldn't/doesn't have to be. Folks have implemented full-duplex streaming to the browser. Specification language is W.I.P. by certain parties. Both My point is I'm not a proponent of censorship. Everybody thinks their own ideas are great.
|
Makes no sense to me. cuz why would you exposing a file system? sure i may be more secure in a way that you can not list, delete or edit files but still...
why not throw? and what about the response? should it contain content-type, content-length, last modified date, accept byte range? how much should it expose?
I know that deno supports file, and it have protections agains redirect attacks like https:// -> file:// and if we can implement something like a permission control to allow TimothyGu have already gone trough why supporting file:// would be bad Why are we talking about duplex streams? it's have nothing to do with file: And you can say whatever you like, but plz do it elsewhere so that the person dose not get offended by it. |
I haven't tested node v22.0.0-nightly202311151d8483e713 cuz it's not released? So i have my doubts if it's working |
Then you can not fetch Extending your own personal preferences to an entire API is selfish and to me denotes you want to control what users do on their own machine if they use your library. Why? You are not responsible for what users' fetch on their own machines. Relevant specification issue w3c/webappsec-secure-contexts#66.
You can throw or not throw. Whatever. The point is arbitrarily barring fetching
It is released. Fetch Node.js nightly archive and test for yourself to resolve any doubts you might have.
I'm just saying the reason for
I think maintainers need to learn how to handle constructive feedback. You have done great work in the open source software domain. Editing honest user feedback is problematic on many levels. IMO you would have done better to just mark the comment as off-topic rather than editing the users' comment because you felt "offended". I don't think the author of the comment was trying to throw shade. Anyway, I ain't trying to give you grief. I was startled by you editing a users' comments here and conveyed my opinion on that act. Have a great day, cheers. |
I mean, yeah, at least, disable it by default, but write options to enable it if you want, doesn't node has permissions now? if you want to be safe you can use those permissions. |
If you are using I didn't use any special flags when I tested. I fetch the Node.js nightly archive every day or so though, so the example I posted is using the Isn't the whole point of polyfills that native support is not shipped? |
Node-fetch was created long before when fetch was only a browser thing and was very new we created a server side implementation first and our goal where to give browser fetch to the server. And prefixed it with node cuz it was specifically built for node:http in mind |
I see. So |
Looks like fetching |
For anybody interested in making this happen for Undici |
at new URL (node:internal/url:804:36) hmm maybe somthing that can be patched? |
@Kreijstal A few ways to do this for Node.js' Undici At https://github.com/nodejs/undici/blob/main/lib/web/fetch/index.js#L129 add the following to short-circuit for
Using
GNU Coreutils
|
Are there plans to support local filesystem URLs?
The text was updated successfully, but these errors were encountered: