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

file:// protocol support #227

Closed
stevenvachon opened this issue Oct 15, 2016 · 11 comments
Closed

file:// protocol support #227

stevenvachon opened this issue Oct 15, 2016 · 11 comments

Comments

@stevenvachon
Copy link
Contributor

stevenvachon commented Oct 15, 2016

It'd be convenient to not have to handle this ourselves each time we need it.

Use cases:

  • Requesting data from URLs in an HTML document where such a protocol is most likely to occur
@sindresorhus
Copy link
Owner

sindresorhus commented Oct 15, 2016

Why? Please include a issue description.

@stevenvachon
Copy link
Contributor Author

Original comment updated.

@sindresorhus sindresorhus changed the title file:// support file:// protocol support Oct 17, 2016
@sindresorhus
Copy link
Owner

Haven't had a chance to think this through yet, but sounds like a pretty good venue for security issues.

fetch() intentionally doesn't support file://.

@stevenvachon
Copy link
Contributor Author

file:// is part of the fetch() spec: https://fetch.spec.whatwg.org/#basic-fetch

@sindresorhus
Copy link
Owner

Please read the actual text there about it.

@stevenvachon
Copy link
Contributor Author

stevenvachon commented Oct 18, 2016

Make it optional, then. That way, we could do something like:

const baseUrl = new URL("http://domain/")
const url = new URL("file://dir/file", baseUrl)
got(url, { supportFileProtocol: baseUrl.protocol==="file:" })  // origin error

@stevenvachon
Copy link
Contributor Author

Now that I think of it, a separate package should be written to handle this as it can then be used with any similar request library. If we want, we can document its use in this package's readme.

@tommedema
Copy link

@stevenvachon are you aware of any such package?

@stevenvachon
Copy link
Contributor Author

@tommedema no, but I also haven't begun to use this library yet either.

@sindresorhus
Copy link
Owner

Going to pass on this. See node-fetch/node-fetch#75 (comment) for reasoning.

@szmarczak
Copy link
Collaborator

I don't think it's a good idea, it could lead to potential security issues.

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

4 participants