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

ens-ipfs/setup.js prevents connecting to all websites under certain conditions #7418

Closed
eduadiez opened this issue Nov 14, 2019 · 4 comments · Fixed by #7419
Closed

ens-ipfs/setup.js prevents connecting to all websites under certain conditions #7418

eduadiez opened this issue Nov 14, 2019 · 4 comments · Fixed by #7419

Comments

@eduadiez
Copy link
Contributor

Describe the bug
With the current implementation of ens-ipfs/setup.js all requests are listened even of the type xmlhttprequest, websockets, ...

I think it should use webRequest.RequestFilter to filter only main_frame the request.

This issue is related to #5460 where the previous implementation also has the same problem.

To Reproduce
If you are connected to a DAppNode metamask redirect you to ens even it's able to resolver the website.

Expected behavior
Only redirect to https://app.ens.domains in case the request from main_frame fails.

Browser details (please complete the following information):

  • All browsers
@eduadiez
Copy link
Contributor Author

I just made a PR to fix this problem #7419

@danfinlay
Copy link
Contributor

Can you clarify the problem you're experiencing?

While websockets may not make sense to intercept, other web resources could very validly be pointed at by ENS, like images, html, js, css, etc.

It sounds like you want to make sure that some AJAX calls pass through to a local server you're hosting? Can you go in detail about what you want to happen?

It does look like this current implementation does not support that kind of resource loading though, because it loads the whole tab when an ens request is fired, but just trying to hone at the correct implementation, not just "only allow in main frame", which seems like it may limit future functionality.

@eduadiez
Copy link
Contributor Author

It sounds like you want to make sure that some AJAX calls pass through to a local server you're hosting? Can you go in detail about what you want to happen?

In summary, DAppNode has an internal DNS bind, an "ethforwarder", an IPFS and a fullnode of ethereum. The DNS is responsible for sending the .eth request to the ethforwarder and this module use the internal IPFS node and its ethereum fullnode to resolve the .eth domain request without the needed to query a 3rd party service.

With the current implementation, if it fails to load any of the resources that a website tries to load it is redirected directly since you use webRequest.onErrorOccurred. With that filter even if an error occurs when loading a resource like a font it will be redirected automatically.

I've seen how in some cases the web redirects you to downloading the resource (e.g. a font) instead of loading the site.

While websockets may not make sense to intercept, other web resources could very validly be pointed at by ENS, like images, html, js, css, etc.

Do you know any example that I can take a look?

It does look like this current implementation does not support that kind of resource loading though, because it loads the whole tab when an ens request is fired, but just trying to hone at the correct implementation, not just "only allow in main frame", which seems like it may limit future functionality.

I completely agree that in the future or on certain situations it may make sense to load ENS resources inside another one. But maybe if that is the objective, using webRequest.onErrorOccurred for the implementation may not be the most appropriate way to do it or a more precise filtering would have to be done, since it can lead to problems such as the one I described on downloading a resource due to an error.

BTW, Personally I miss being able to disable this functionality since for me it is somewhat a little invasive (It sounds a bit weird to query .onion domains using public gateways), and I also think it might be interesting to be able to choose what endpoint to use for IPFS, SWARM, ... not only for decentralization or privacy, but because if the gateway that your are using is down, for any reason, everything stops working.

@danfinlay
Copy link
Contributor

We merged your PR because clearly this implementation isn't perfect, and we're happy to fix this in the meanwhile, but I'd like us to find a good long term solution, too.

In summary, DAppNode has an internal DNS bind, an "ethforwarder", an IPFS and a fullnode of ethereum.

We have a pending PR that will let you configure your own IPFS gateway, would that satisfy your requirements?
#7362

Do you know any example that I can take a look?

Here is an example JS file on ens/ipfs: ipfs.snap.eth.

using webRequest.onErrorOccurred for the implementation may not be the most appropriate way to do it

I totally agree, and we intend to convert to another method, possibly onBeforeRequest.

it might be interesting to be able to choose what endpoint to use for IPFS, SWARM, ... not only for decentralization or privacy, but because if the gateway that your are using is down, for any reason, everything stops working.

I agree, that's why we're making it all increasingly configurable, and as snaps rolls out, eventually users may even be able to opt into preferred implementations of these configurable redirects.

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

Successfully merging a pull request may close this issue.

2 participants