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

/ipns/ should redirect to /ipfs/ and/or cache #3861

Closed
recmo opened this issue Apr 13, 2017 · 8 comments · Fixed by #8891
Closed

/ipns/ should redirect to /ipfs/ and/or cache #3861

recmo opened this issue Apr 13, 2017 · 8 comments · Fixed by #8891
Labels
need/community-input Needs input from the wider community not actionable topic/ipns Topic ipns

Comments

@recmo
Copy link

recmo commented Apr 13, 2017

Version information:

go-ipfs version: 0.4.3-
Repo version: 4
System version: amd64/linux
Golang version: go1.7

Type: Feature

Severity: Medium (performance issue)

Description:

Currently, /ipfs/ request have maximal caching implemented, which is great for performance as it allows offloading load to caches (reverse proxies, Cloudflare, browser, etc). The /ipns/ are mutable, so the same treatment would not work. Currently, they don't provide any form of caching, making every request go all the way back to the (comparatively slow) origin node.

Addendum:

There is currently a very low severity bug in the handling of the If-None-Match header for /ipns/ requests. It is incorrectly handled like the /ipfs/ requests, and returns 304 Not Modified if set to the ipns base name, even though the content may have changed. This is very low severity since /ipns/ never sets an ETag header to begin with.

Proposal:

I propose to implement the following:

  • On /ipns/ requests always set the ETag to the current /ipfs/ base name.

  • If the request contains a If-None-Match header, check the ETag and reply with 304 Not Modified if appropriate.

  • Add a configuration option GateWayconfig.IpnsRedirect bool.

  • If the boolean is set, reply to /ipns/ requests with a 307 redirect to the current /ipfs/ path and include a header for 5 minute caching. (ipns updates, AFAIK, have no global consistency guarantees, so this should not hurt. Even a slight caching header can be the difference between 10k requests per second and 1 request per minute.)

  • If the boolean is not set, reply as before, but also include the ETag header described above.

Relevant prior discussions:

Redirection has been mentioned earlier in issue 132:

  • should use relative 302 (or 303/307) redirects for resolving /ipns -> /ipfs

Issue 132 is closed, but this particular feature is currently not implemented. In issue 1234 it was suggested again, but at least one user had a problem with it in the context of dns links (something I don't understand).

Questions:
  • Does the config option make sense or is it too much and can I always enable redirects?
  • Should the redirect cache duration be configurable? What is a good default?
  • Is there a reason this is not implemented yet that I should know of?
  • How does this affect dns link?
@recmo recmo changed the title /ipns/ should 307 redirect to /ipfs/ /ipns/ should redirect to /ipfs/ Apr 13, 2017
@recmo recmo changed the title /ipns/ should redirect to /ipfs/ /ipns/ should redirect to /ipfs/ and/or cache Apr 13, 2017
@Kubuxu Kubuxu added need/community-input Needs input from the wider community topic/ipns Topic ipns not actionable labels Apr 17, 2017
@Kubuxu
Copy link
Member

Kubuxu commented Apr 17, 2017

IPNS system is up for major rewrite (introduction of Inter Planetary Record System), this would be a place where cache duration configuration would probably be implemented. Time of this rewrite isn't specified.

#132 was just a proposal how this feature could be implemented, it was implemented otherwise. Plain redirecting isn't great idea IMO as it will prevent the site from updating.

There is a bit of conflict here as some sites don't worry about downgrade or progress stop attacks but others do. Currently IPNS has no way to express that so it takes the most conservative path. IPRS hopefully will be able to express that and thus solves this issue.

Using ETag with no timeout is very neat idea. It might be worth creating separate actionable issue that asks for implementation like this. Lack of caching in IPFS is because we cannot get TTL of DNS record and we focus a lot on C in CAP theorem.

@wigy-opensource-developer

Plain redirecting isn't great idea IMO as it will prevent the site from updating.

@Kubuxu As I understand, the 307 HTTP status should keep the /ipns URI in the browser window, therefore bookmarking the mutable URI is still possible. Did I miss something?

@recmo
Copy link
Author

recmo commented Apr 18, 2017

@wigy-opensource-developer None of the redirects keep the address bar. You can try here: https://jigsaw.w3.org/HTTP/300/307.html The only way to maintain address bar server-side is to proxy the content—like go-ipfs is currently doing. (Client side you can use an <iframe>.)

@recmo
Copy link
Author

recmo commented Apr 18, 2017

@Kubuxu Thanks for the heads up on IPRS. Was not aware of this planned change, good to know! Looks like a solid improvement over IPNS.

There is a bit of conflict here as some sites don't worry about downgrade or progress stop attacks but others do.

Sorry, you lost me here. What is a downgrade attack and progress stop attack? I couldn't find a mention of them in the github repo's outside of this issue.

Using ETag with no timeout is very neat idea. It might be worth creating separate actionable issue that asks for implementation like this.

Thanks! This looks like an easy job, I might give it a try and submit a PR this week.

Lack of caching in IPFS is because we cannot get TTL of DNS record and we focus a lot on C in CAP theorem.

I don't fully understand how DNS come in to play, I'll read up on DNS-links.

@Kubuxu
Copy link
Member

Kubuxu commented Apr 18, 2017

Sorry, you lost me here. What is a downgrade attack and progress stop attack? I couldn't find a mention of them in the github repo's outside of this issue.

The key here is that currently IPNS prefers no record over oldish or expired record. This is what was decided long time ago and possibility of changing it is a separate matter.

I don't fully understand how DNS come in to play, I'll read up on DNS-links.

IPNS allows also for DNS resolution, /ipns/ipfs.io works like that. We unfortunately are not getting TTLs from those DNS requests but using just a plain ETag caching instead of always fetching assets should improve it greatly.

@makew0rld
Copy link
Contributor

What has happened with this so far?

@DavidBurela
Copy link

A common use case I use is to pin documentation sites so that I can browse them offline
ipfs pin add /ipns/docs.ipfs.io

The issue is that I cannot browse it offline as it refuses to resolve the IPNS. I need to manually note the hash when I pin, so I can browse to that instead.

@lidel
Copy link
Member

lidel commented Aug 12, 2021

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/community-input Needs input from the wider community not actionable topic/ipns Topic ipns
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants