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

Support Web API navigator.registerProtocolHandler #7686

Closed
lidel opened this issue Sep 18, 2020 · 3 comments · Fixed by #7802
Closed

Support Web API navigator.registerProtocolHandler #7686

lidel opened this issue Sep 18, 2020 · 3 comments · Fixed by #7802
Labels
effort/hours Estimated to take one or several hours exp/beginner Can be confidently tackled by newcomers kind/bug A bug in existing code (including security flaws) kind/enhancement A net-new feature or improvement to an existing feature need/community-input Needs input from the wider community need/maintainers-input Needs input from the current maintainer(s) P2 Medium: Good to have, but can wait until someone steps up topic/gateway Topic gateway

Comments

@lidel
Copy link
Member

lidel commented Sep 18, 2020

What

I propose we add support for ?uri= which would decode passed URI and return HTTP 301 redirect to proper /ipfs/ or /ipns/ path.

This is pretty trivial to add, but wanted to agree on ?url= (or other means) before I submit PR.

cc @autonome @Gozala @aschmahmann @achingbrain

Why

Now that Chromium 86 safelisted ipfs://,ipns:// and dweb: we should support requests produced by navigator.registerProtocolHandler on gateways.

Namely, if someone registers dweb.link as URI handler for ipfs://:

navigator.registerProtocolHandler('ipfs', 'https://dweb.link/?uri=%s', 'ipfs resolver')

Then opening ipfs://QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR Chromium will produce HTTP GET with URI-encoded URI:

https://dweb.link/?uri=ipfs%3A%2F%2FQmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR

Using URL parameter ?uri= seems safer than /uri/%s because it does not clash with path-based logic already present in gateway code, but I am open for suggestions, if there is a better way to represent this.

@lidel lidel added kind/bug A bug in existing code (including security flaws) kind/enhancement A net-new feature or improvement to an existing feature topic/gateway Topic gateway need/community-input Needs input from the wider community P2 Medium: Good to have, but can wait until someone steps up need/maintainers-input Needs input from the current maintainer(s) exp/beginner Can be confidently tackled by newcomers effort/hours Estimated to take one or several hours labels Sep 18, 2020
@dennis-tra
Copy link
Collaborator

Hi @lidel

I would like to make my first contribution. Do you think this issue is ready to be implemented?

I've already had look around the codebase and saw that prometheus has already registered a handler for the top level root / path - metrics.go:L91. Registering another handler for / would result in a panic. Now two ways forward come to my mind:

  1. Change the prometheus code and add another handler that parses the uri query parameter
  2. Since one must call registerProtocolHandler for every protocol (ipfs, ipns, etc.) we could use the URLs https://dweb.link/ipfs/?uri=%s and https://dweb.link/ipns/?uri=%s. Doing it this way would make it possible to implement the logic in gateway_handler.go which has already registered the paths /ipfs and /ipns.

Let me know what you think.

@lidel
Copy link
Member Author

lidel commented Dec 4, 2020

Hi @dennis-tra, yes, its ok for you to take a stab at this!
I think the approach you proposed in (2) is sensible (fair point that ipfs:// and ipns:// will require separate registration anyway) – ping me when you have a PR :)

@dennis-tra
Copy link
Collaborator

dennis-tra commented Dec 5, 2020

@lidel I just filed the PR #7802 it would be great if you could take a look :)

@lidel lidel changed the title Endpoint for Web API navigator.registerProtocolHandler feat: URI router for Web API navigator.registerProtocolHandler Jan 14, 2021
@lidel lidel changed the title feat: URI router for Web API navigator.registerProtocolHandler URI router for Web API navigator.registerProtocolHandler Jan 14, 2021
@lidel lidel changed the title URI router for Web API navigator.registerProtocolHandler Support Web API navigator.registerProtocolHandler Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/hours Estimated to take one or several hours exp/beginner Can be confidently tackled by newcomers kind/bug A bug in existing code (including security flaws) kind/enhancement A net-new feature or improvement to an existing feature need/community-input Needs input from the wider community need/maintainers-input Needs input from the current maintainer(s) P2 Medium: Good to have, but can wait until someone steps up topic/gateway Topic gateway
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants