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.

Sign up for GitHub

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

Jump to bottom

feat(gw): URI router for Web API navigator.registerProtocolHandler #7802

Merged
merged 5 commits into from
Jan 14, 2021
Merged

feat(gw): URI router for Web API navigator.registerProtocolHandler #7802

merged 5 commits into from
Jan 14, 2021

Conversation

dennis-tra
Copy link
Contributor

@dennis-tra dennis-tra commented Dec 5, 2020 •
edited by lidel
Loading

@lidel This PR attempts to close #7686

This commit adds support for requests produced by navigator.registerProtocolHandler on gateways. Now one can register dweb.link as an URI handler for ipfs://:

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

Then opening ipfs://QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR will produce an HTTP GET call to:

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

The query parameter uri will now be parsed and the given content identifier resolved via:

https://dweb.link/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR

Note:

  1. If the path parameter ipfs and scheme of the uri param value don't match we'll redirect to the uri param value. E.g. A call to https://dweb.link/ipfs/?uri=ipns://content-identifier will be redirected to https://dweb.link/ipns/content-identifier
  2. I didn't test path combinations where X-Ipfs-Gateway-Prefix is set due to the discussion in Removing support for X-Ipfs-Gateway-Prefix #7702.

@welcome

This comment has been minimized.

Sign in to view
@dennis-tra dennis-tra mentioned this pull request Dec 5, 2020
Closed
@lidel
Copy link
Member

lidel commented Dec 11, 2020

@dennis-tra I'm sorry I did not review this yet, been busy with 0.8.0, will review as soon I can.

lidel
lidel approved these changes Dec 12, 2020
View reviewed changes
Copy link
Member

@lidel lidel left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dennis-tra LGTM, thank you!

I've added small tweaks:

  • URI-decoding step (because navigator.registerProtocolHandler will always URL-encode entire URI before passing it to the endpoint)
  • support for query params to enable things like ipfs://QmFoo?filename=cat.jpg
  • tests with longer path that includes unicode characters to confirm Location header is properly url-encoded

@aschmahmann this is kinda small, any chance to review and include it in 0.8.0 RC2?

Would simplify some demos Igalia wants to do with IPFS around navigator.registerProtocolHandler in browsers (ongoing work in https://chromium-review.googlesource.com/c/chromium/src/+/2287304 etc)

@lidel lidel added need/review Needs a review topic/gateway Topic gateway labels Dec 12, 2020
@dennis-tra
Copy link
Contributor Author

@lidel thanks for your review and the improvements :) would be great to already see it in 0.8.0 ✌️

@lidel lidel mentioned this pull request Jan 7, 2021
Merged
4 tasks
@dennis-tra
Copy link
Contributor Author

Hi @lidel,

I just rebased on master to trigger the CI again since there was a failing check. Everything looks good now - any chance to get this merged?

@lidel
Copy link
Member

lidel commented Jan 13, 2021 •
edited
Loading

@dennis-tra thanks for rebase! I've added sharness tests just to be extra sure it won't get broken by a mistake in the future.

I talked with @aschmahmann and we should be able to include this in 0.8.0-rc2, just need to block some time for the review this month.

I bumped priority for this, because when this ships, we will ask browser vendors with upcoming built-in URI support such as Brave and Opera to switch to this ?uri= router, removing the complexity from their end.

@lidel lidel changed the title feat: support requests from 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 feat(gw): URI router for Web API navigator.registerProtocolHandler Jan 14, 2021
dennis-tra and others added 5 commits January 14, 2021 20:52
@dennis-tra @lidel
feat: support requests from registerProtocolHandler
36368ee 
This commit adds support for requests produced by navigator.registerProtocolHandler on gateways. Now one can register `dweb.link` as an URI handler for `ipfs://`:

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

Then opening `ipfs://QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR` will produce an HTTP GET call to:

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

The query parameter `uri` will now be parsed and the given content identifier resolved via:

`https://dweb.link/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR`
@lidel
fix: ?uri= url-decode and preserve query
3de5b14 
This makes ?uri= param able to process URIs passed by web browsers
https://developer.mozilla.org/en-US/docs/Web/API/Navigator/registerProtocolHandler
@lidel
refactor: remove redundant urlescape
abb25a1 
URL.Query() will already decode the query parameters
@lidel
test: sharness for ?uri= router
cdad394 
Additional tests to ensure there are no regressions, as this will be
used by browser vendors in the future.
@lidel
test: cover 2 remaining lines
a0f90d3
@aschmahmann aschmahmann merged commit b5079b0 into ipfs:master Jan 14, 2021
@dennis-tra dennis-tra deleted the issue-7686 branch January 15, 2021 07:53
@lidel lidel mentioned this pull request Jan 16, 2021
Closed
@lidel lidel mentioned this pull request Feb 16, 2021
Closed
lidel added a commit to ipfs/ipfs-companion that referenced this pull request May 10, 2021
@lidel
chore: native uri router
7c894a3 
This replaces JS-based router with URI router introduced in
ipfs/kubo#7802

Every gateway running go-ipfs 0.8.0 or later has it.
It not only removes the need for JS-based router, but makes it
future-proof, as gateway takes care of redirects to subdomains and/or
base conversion, if needed.
@lidel lidel mentioned this pull request May 10, 2021
Merged
lidel added a commit to ipfs/ipfs-companion that referenced this pull request May 10, 2021
@lidel
chore: native uri router (#999)
41171b3 
This replaces JS-based router with URI router introduced in
ipfs/kubo#7802

Every gateway running go-ipfs 0.8.0 or later has it.
It not only removes the need for JS-based router, but makes it
future-proof, as gateway takes care of redirects to subdomains and/or
base conversion, if needed.
@Arqu Arqu mentioned this pull request Feb 17, 2023
Open
hacdias pushed a commit to ipfs/boxo that referenced this pull request Jan 27, 2023
@aschmahmann
Merge pull request ipfs/kubo#7802 from dennis-tra/issue-7686
7a56ba7 
feat(gw): URI router for Web API navigator.registerProtocolHandler

This commit was moved from ipfs/kubo@b5079b0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lidel lidel lidel approved these changes

Assignees
No one assigned
Labels
need/review Needs a review topic/gateway Topic gateway
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support Web API navigator.registerProtocolHandler
3 participants
@dennis-tra @lidel @aschmahmann