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

Improve dnslink resolution #69

Closed
Kubuxu opened this issue Feb 10, 2016 · 4 comments
Closed

Improve dnslink resolution #69

Kubuxu opened this issue Feb 10, 2016 · 4 comments
Assignees

Comments

@Kubuxu
Copy link
Member

Kubuxu commented Feb 10, 2016

This would prevent possible delay of loading the page when DNS is slow (on bad connections DNS resolution can take few seconds).

So start loading page via normal HTTP and start resolving dnslink, if dnslink is available terminate normal HTTP connection and redirect to local gateway.

@lidel
Copy link
Member

lidel commented Feb 10, 2016

Very good idea. Thank you!

Current SDK provides two HTTP events (same for the future one)
that we can use to hook into request pipeline before request is made:

  • http-on-modify-request Called as a http request is made. The channel is available to allow you to modify headers and such.
  • http-on-opening-request Similar to http-on-modify-request, but called earlier (synchronously during the channel's asyncOpen() call), and some channel atttributes (proxyInfo) may be missing. Use only if your observer must be called before asyncOpen returns.

https://developer.mozilla.org/en-US/docs/Observer_Notifications#HTTP_request

Currently, all the redirect logic of this addon is in http-on-modify-request handler.

I guess we could hook into earlier http-on-opening-request and only check if host is in cache.
If not, do an asynchronous dnslookup with a callback that puts the result in cache.
Then in http-on-modify-request we would just check the cache.
Result: no slowdown caused by synchronous lookups or slow DNS/network.

There is a risk of dnslink lookup taking longer than the delay between those two events, but it would be only for an initial cache miss (and we could save cache between browser restarts).
Not sure if we can do anything more with these APIs.

Should I give it a try?

@the8472
Copy link
Contributor

the8472 commented Feb 11, 2016

At the very least we could do an async XHR and suspend the nsIRequest instead of blocking the main thread waiting for a response and resume the request once we got a response. This way at least the UI stays responsive.

@lidel lidel self-assigned this Feb 11, 2016
@lidel lidel closed this as completed in 7d40446 Feb 13, 2016
lidel added a commit that referenced this issue Feb 13, 2016
- solves #69 without miss on first request
@lidel
Copy link
Member

lidel commented Feb 13, 2016

Thank you @the8472, it was a very good insight.

  • dnslink lookups are now executed asynchronously and only if DNS support is enabled, record was not found in cache and API is up
  • request is suspended until dnslink lookup callback is fired

Submitted v1.5.4 to AMO.

@lidel lidel modified the milestone: v1.6.0 Feb 13, 2016
@lidel
Copy link
Member

lidel commented Apr 26, 2017

Reopening: I feel it may be a good idea to revisit this in WebExtension rewrite and see if something similar can be introduced.

@lidel lidel reopened this Apr 26, 2017
@lidel lidel added this to the v2.0.0 milestone Apr 26, 2017
@lidel lidel removed this from the v2.0.0 milestone Sep 5, 2017
@lidel lidel changed the title Start loading HTTP page in parrarel with dnslink resolution Improve dnslink resolution Oct 2, 2017
lidel added a commit that referenced this issue Oct 6, 2017
This commit removes false-positive redirects for paths
that start with /ipns/{ipnsRoot} by following these steps:
1. is-ipfs test (may produce false-positives)
2. remove false-positives by checking if ipnsRoot is:
   - a valid CID (we check this first as its faster/cheaper)
   - or FQDN with a valid dnslin in DNS TXT record
    (expensive, but we reuse caching mechanism from dnslink experiment)

This means we now _automagically_ detect valid IPFS resources on any
website as long as path starts with /ipfs/ or /ipns/, removing problems
described in
#16 (comment)

This commit also closes #69 -- initial load is suspended until dnslink
is read via API, then it is cached so that all subsequent requests are
very fast.
@lidel lidel closed this as completed in 7d9a1f2 Oct 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants