-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
feat(gw): /ipfs/ipfs/{cid} → /ipfs/{cid} #7930
Conversation
This will try to recover from invalid paths like /ipfs/ipfs/{cid} and redirect to proper one, when possible.
This seems weird. This will magically make some invalid URLs "just work". Those invalid URLs will then propagate and become "normal" (requiring support). Instead of patching go-ipfs to fix URLs like this for people, we should provide clear error messages describing what may/may not have gone wrong. |
@Stebalien tbh we already have pretty clear error message:
I don't think we can make things more clear than "invalid path /ipfs/ipfs". You have a valid point that silently fixing this == enabling misconfiguration to go unnoticed. <!DOCTYPE html><html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8" /><meta http-equiv="refresh" content="5;url={intendedURL}" /></head><body>invalid ipfs path: invalid path "{urlPath}": invalid CID: selected encoding not supported</body></html> This way broken links still load (after a delay) + delay and error are enough signal for dev to fix the address in their NFT contract etc. |
Yeah, that seems reasonable. |
This implements error page that does not hide the problem, but still redirects to a valid path after short delay: #7930 (comment)
@Stebalien implemented, looks like this (5s was IMO not enough to read the message): |
It may be nicer to replace the entire error message explaining what the issue is, but either way works for me. "/ipfs/ipfs/Qm... should be /ipfs/Qm..." would make more sense than "invalid CID". |
- moved to separate utility function - return Bad Request error - improved escaping of values passed via URL path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm. One more thing.
This ensures we dont contribute to problem described in: ipfs/kubo#7930
This ensures we dont contribute to problem described in: ipfs/kubo#7930
This ensures we dont contribute to problem described in: ipfs/kubo#7930
This ensures we dont contribute to problem described in: ipfs/kubo#7930
This ensures we dont contribute to problem described in: ipfs/kubo#7930
This ensures we dont contribute to problem described in: ipfs/kubo#7930
This ensures we dont contribute to problem described in: ipfs/kubo#7930
This implements error page that does not hide the problem, but still redirects to a valid path after short delay: ipfs/kubo#7930 (comment) This commit was moved from ipfs/kubo@dae7387
ipfs/kubo#7930 (comment) This commit was moved from ipfs/kubo@450baef
ipfs/kubo#7930 (comment) This commit was moved from ipfs/kubo@a35ffee
ipfs/kubo#7930 (comment) This commit was moved from ipfs/kubo@763a120
feat(gw): /ipfs/ipfs/{cid} → /ipfs/{cid} This commit was moved from ipfs/kubo@d30f1e5
This PR enables Gateways to recover from invalid paths like
/ipfs/ipfs/{cid}
and redirect them to proper one, when possible.raison d'être is to
image
tag here (nft) 🐈🏳️🌈ipfs://
URI to address bar)cc @autonome