-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
Adds support for the IPFS protocols: #8805
Conversation
dbf26ee
to
48e0cf2
Compare
Until we have docs and a few test cases for it, it is hard to judge if this works 100% correctly. |
Sure thing! For the testing, i suggest that the tests would only validate that - say - I don't think the tests should actually execute the web request! Regarding the documentation, what exactly do you think should be documented and where would be the best place for that? I can for example document the logic in detecting the gateway. |
I think a fair test would be to use an Is the gateway supposed to be anything ? I mean it accepts all protocols now and right now the code doesn't even check that it is a URL. |
Closed after two months with no feedback or action. Feel free to reopen when things move again. |
Hi @bagder, sorry for not being active! This is definitely on my list of things to work on further and complete this implementation. I'll update this pr soon. Definitely still this month! |
Could you re-open this PR? I apparently don't have the permission for it. |
Feel free to have a new fresh look at the latest patch. I rewrote the url rewriting part completely to make use of curl API methods. There is now no custom parsing anymore besides the check to see if the parsed protocol is ipfs or ipns to even enter this code path to begin with. What's still missing in the gateway detection part is a default fallback (when no gateway is found) to dweb.link. Just for reference, that site is maintained by ProtocolLabs (the creators of IPFS) and is allowed to be the fallback. Same is done on the ffmpeg side of things. |
The IPFS gateway gets to see all the IPFS traffic as there is no extra security layer on top of this. Using an IPFS gateway means you blindly trust that server and its operators and allow them to see and possibly even tamper with the data (and you can't tell). It's a really big deal. I don't think we should have any "random" site out there as a default gateway without very huge warning letters and flashing alarms going off. |
The intent of having this is to give an "ipfs always works" idea. As it first will want to find a local gateway and then uses dweb.link as fallback. On the ffmpeg side this concern is sort of mitigated by just printing a chunk of text as warning notifying the user that dweb.link is used and that the user really should set up a local node instead. Is such a solution acceptable for curl too? |
It makes me uncomfortable. The admins of that single gateway gets super powers and control over so many users' potential traffic when someone convinces ordinary users out there to run a curl command line using ipfs. I understand the point of why someone might want to do this. I'm just not sure I agree that the method is a very good one for users ultimately. Using a gateway to access ipfs in the first place is a shaky thing, and you really should trust the gateway before you use it. I don't see how we can tell the world to trust dweb.link by default. |
A wild idea - fail if no local node can be detected but point users to "dweb.link" that they can enable if they trust them. It could still be seen as an "endorsement" from curl though and I don't know if it's desired. |
I was thinking about something like that too @jzakrzewski What are your thoughts on that? |
To be honest, hardcoding a specific gateway in a low level tool like @bagder thoughts on opt-in options below? (A) No endpoint endorsement at all$ curl ipfs://bafkreicysg23kiwv34eg2d7qweipxwosdo2py4ldv42nbauguluen5v6am
Error: local gateway not found and/or IPFS_GATEWAY is not set
Learn how to run one: https://docs.ipfs.tech/install/command-line/
$ IPFS_GATEWAY=https://ipfs.io curl ipfs://bafkreicysg23kiwv34eg2d7qweipxwosdo2py4ldv42nbauguluen5v6am
hello (B) Small endorsement of ipfs.ioIf we want to show people "quick fix" option: $ curl ipfs://bafkreicysg23kiwv34eg2d7qweipxwosdo2py4ldv42nbauguluen5v6am
Error: local gateway not found and/or IPFS_GATEWAY is not set.
Try: IPFS_GATEWAY=https://ipfs.io
or run your own: https://docs.ipfs.tech/install/command-line/ |
Any update here? The things i know i'm waiting on:
I probably do still have to make some test cases. Not exactly sure how that's supposed to work but i'll figure that out of the functionality is agreed upon. Is there anything i'm missing? |
I would like any URLs mentioned for further documentation to go to curl.se rather than an externally controlled site/domain. That page may then refer to other sites etc, as then we can update accordingly in the future when we feel the need to.
I don't understand why you (as in the IPFS community) consider a simple HTTP 30x-redirect a "gateway" at all. That's just a bounce-off away to another host (that runs the real gateway), and doing that without users being fully aware of it feels borderline disingenuous. So no, I don't think curl should follow such redirects by default. It is almost exactly the same situation as with other HTTP redirects.
Docs. I think maybe |
Thank you for the feedback @lidel Will get that spell bot to play nice on it too ;) |
- ipfs://<cid> - ipns://<cid> This allows you tu use ipfs in curl like: curl ipfs://<cid> and curl ipns://<cid> For more information consult the readme at: https://curl.se/docs/ipfs.html
🎉 it is merged! Thanks for your hard work @markg85 |
This makes docs/IPFS.md from the source tree into the docs/ipfs.html page on the website. Requires curl/curl#8805
This makes docs/IPFS.md from the source tree into the docs/ipfs.html page on the website. Requires curl/curl#8805 Closes #298
That's so awesome @bagder ! :D Thank you! |
- ipfs://<cid> - ipns://<cid> This allows you tu use ipfs in curl like: curl ipfs://<cid> and curl ipns://<cid> For more information consult the readme at: https://curl.se/docs/ipfs.html Closes curl#8805
Does this make use of an env variable for the gateway URL? I couldn't see it on the docs site. If it did then it'd be useable in all the tools and scripts that use curl under the hood by simply setting the variable beforehand, and as there's 2 million commits on GitHub that reference curl so that's IPFS in a lot of places. Is there a standard variable for the IPFS gateway? |
Yes! |
Sorry! I was on my phone and find in page mustn't have been working. This is a great addition to curl, kudos! Next step wget, requests, http.client? :) |
@bitplane One of these: https://github.com/ipfs/integrations/issues |
It is said that the authenticity of the response can be verified by a hash. Does curl do this as well? |
No. Curl just gets the data from IPFS (through a gateway). If you use a public gateway then you don't have this guarantee because you don't know what's running behind the gateway. Then you're in CAR land where you can verify the data is as you'd expect it to be, this explains it. You should use a local gateway! |
Then why is there a "Verifiable responses" section? That sounds a lot like curl verifying that for more. Maybe you should write that clearly there? |
It literally says:
With that clickable link included to explain how to do just that. Care to elaborate what's not clear about that? |
Is there a reason this is not under the control of enable/disable switches? Not sure I've ever seen a protocol added to curl that is forced into build. |
No particular reason no, other than that this is a "protocol" only supported by the command line tool and special in the way how it is just a "translation" into HTTP(S). |
This is definitely a WIP branch.
It adds support for the IPFS protocols (
ipfs://
andipns://
).This code seems to be working already so now i'm looking for feedback to tidy it up for the eventual merger in curl.