Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

feat: add HTTP Gateway support for /ipns/ paths #2020

Merged
merged 8 commits into from
Jul 4, 2019

Conversation

lidel
Copy link
Member

@lidel lidel commented May 7, 2019

Part of an effort to run embedded js-ipfs in Brave 🦁 ipfs/ipfs-companion#716
Fixes #1918

This PR will add support for /ipns/ paths at HTTP Gateway.
Smoke test: /ipns/tr.wikipedia-on-ipfs.org (IPNS+DNSLink+HAMT-sharded website)

Do not merge it yet

This PR requires below PRs to be merged first:

@ghost ghost assigned lidel May 7, 2019
@ghost ghost added the status/in-progress In progress label May 7, 2019
@lidel lidel changed the title feat: add HTTP Gateway support for /ipns/ paths [WIP] feat: add HTTP Gateway support for /ipns/ paths May 7, 2019
@hugomrdias hugomrdias mentioned this pull request May 7, 2019
15 tasks
lidel added a commit to ipfs/ipfs-companion that referenced this pull request May 8, 2019
tl;dr opening /ipns/tr.wikipedia-on-ipfs.org/wiki/Mars.html works

Switched to js-ipfs with cherry-picked changes from:
ipfs/js-ipfs#2020
ipfs/js-ipfs#1989
ipfs/js-ipfs#2002
ipfs/js-ipfs-http-response#22
ipfs-inactive/js-ipfs-mfs#48
@lidel lidel force-pushed the feat/ipns-on-gateway branch from 571eb4b to c9fe577 Compare May 13, 2019 11:59
It requires below to PRs to land first:

#2002
ipfs/js-ipfs-http-response#19
ipfs-inactive/js-ipfs-mfs#48

License: MIT
Signed-off-by: Marcin Rataj <lidel@lidel.org>
@lidel lidel force-pushed the feat/ipns-on-gateway branch from c9fe577 to 97d03fc Compare June 27, 2019 07:21
@lidel
Copy link
Member Author

lidel commented Jun 27, 2019

I rebased this PR and switched to released versions of ipfs-mfs and ipfs-http-response.
Cool test link for HTTP Gateway (IPNS+DNSLink+HAMT):

Note: This PR resolves /ipns/ to /ipfs/ before passing it to ipfs-http-response as a workaround until ipfs/js-ipfs-http-response#22 is solved in more generic way.

@alanshaw this is a fairly small change, do you want to merge this to compliment other IPNS-related backend PRs made by @hugomrdias?

(Travis fails due to recent changes in master, not related to this PR: https://travis-ci.com/ipfs/js-ipfs/jobs/211405524#L3104)

@lidel lidel changed the title [WIP] feat: add HTTP Gateway support for /ipns/ paths feat: add HTTP Gateway support for /ipns/ paths Jun 27, 2019
@lidel lidel requested review from alanshaw and hugomrdias June 27, 2019 07:30
src/http/gateway/routes/index.js Outdated Show resolved Hide resolved
src/http/gateway/resources/gateway.js Outdated Show resolved Hide resolved
src/http/gateway/resources/gateway.js Outdated Show resolved Hide resolved
src/http/gateway/routes/gateway.js Outdated Show resolved Hide resolved
},
{
method: '*',
path: '/ipns/{mutableId*}',
Copy link
Member

Choose a reason for hiding this comment

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

we should define what we should call this id, libp2p-key, ipns-key-id, even ipns-cid (content been the hash of the pubkey) and we should PR to the spec https://github.com/ipfs/specs/tree/master/naming this new name.

Copy link
Member Author

@lidel lidel Jul 3, 2019

Choose a reason for hiding this comment

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

We have libp2p-key multicodec now for presenting IPNS names as base32 CIDv1, but it can also be FQDN with DNSLink, so I renamed it to libp2pKeyOrFqdn.

I did not touch mentioned spec as /ipns/base32(<HASH>) mentioned there has direct implication on how things are implemented: https://github.com/ipfs/js-ipns/blob/v0.5.2/src/index.js#L214 (IPNS does not use <libp2p-key-in-cidv1b32> but raw concatenation of /ipns/ and key buffer, and now they move to just buffer)

lidel added 4 commits July 2, 2019 21:12
License: MIT
Signed-off-by: Marcin Rataj <lidel@lidel.org>
License: MIT
Signed-off-by: Marcin Rataj <lidel@lidel.org>
- remove custom arg passing and replace it with native Joi validation
- decode escaped unicode in paths before passing to ipfs resolver
- add tests for /ipns/ path (file and directory)

License: MIT
Signed-off-by: Marcin Rataj <lidel@lidel.org>
License: MIT
Signed-off-by: Marcin Rataj <lidel@lidel.org>
src/http/gateway/resources/gateway.js Outdated Show resolved Hide resolved
src/http/gateway/resources/gateway.js Outdated Show resolved Hide resolved
src/http/gateway/resources/gateway.js Outdated Show resolved Hide resolved
src/http/gateway/resources/gateway.js Outdated Show resolved Hide resolved
src/http/gateway/resources/gateway.js Outdated Show resolved Hide resolved
src/http/gateway/resources/gateway.js Outdated Show resolved Hide resolved
lidel added 3 commits July 3, 2019 17:26
License: MIT
Signed-off-by: Marcin Rataj <lidel@lidel.org>
License: MIT
Signed-off-by: Marcin Rataj <lidel@lidel.org>
License: MIT
Signed-off-by: Marcin Rataj <lidel@lidel.org>
Copy link
Member

@hugomrdias hugomrdias left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

Code changes LGTM, just one question.

const resources = require('../../gateway/resources')

module.exports = [
{
method: '*',
path: '/ipfs/{cid*}',
path: '/ipfs/{path*}',
Copy link
Member

Choose a reason for hiding this comment

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

Does this need an /ipns handler also?

Copy link
Member Author

@lidel lidel Jul 4, 2019

Choose a reason for hiding this comment

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

No, this is only for /webui and hardcoded /ipfs/{cid_of_webui} on API port.
(/ipns/ is only available on the Gateway port)

// QmaRdtkDark8TgXPdDczwBneadyF44JvFGbrKLTkmTUhHk
await http.api._ipfs.add([content('utf8/cat-with-óąśśł-and-أعظم._.jpg')])
// Publish QmW2WQi7j6c7UgJTarActp7tDNikE4B2qXtFCfLPdsgaTQ to IPNS using self key
await http.api._ipfs.name.publish('QmW2WQi7j6c7UgJTarActp7tDNikE4B2qXtFCfLPdsgaTQ', { resolve: false })
Copy link
Member

Choose a reason for hiding this comment

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

Note for the future: unless we're trying to validate particular content creates a particular CID we should add the content and store the returned CID for use in future tests. Hard coding CIDs like this is a maintenance nightmare when we want to change defaults.

Copy link
Member Author

@lidel lidel Jul 4, 2019

Choose a reason for hiding this comment

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

I am aware, but did not want to rewrite existing tests, so as a compromise I reused existing CID of cat-folder/cat.jpg (was already present a few lines above)

@alanshaw alanshaw mentioned this pull request Jul 4, 2019
54 tasks
@alanshaw alanshaw merged commit 43ac305 into master Jul 4, 2019
@alanshaw alanshaw deleted the feat/ipns-on-gateway branch July 4, 2019 16:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resolving DNSLink Paths: /ipns/<fqdn>
3 participants