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

Only show used candidates on https://webtorrent.io/ #578

Closed
KaptenJansson opened this issue Jan 18, 2016 · 11 comments
Closed

Only show used candidates on https://webtorrent.io/ #578

KaptenJansson opened this issue Jan 18, 2016 · 11 comments
Labels
bug

Comments

@KaptenJansson
Copy link

@KaptenJansson KaptenJansson commented Jan 18, 2016

Currently https://webtorrent.io/ shows local host candidates with private IP addresses which I will never be able to access which indicates it displays the "first" (or some other type of selection) candidates seen.

Instead it should show reachable candidates that are actually used instead.

@DiegoRBaquero

This comment has been minimized.

Copy link
Member

@DiegoRBaquero DiegoRBaquero commented Jan 19, 2016

The private IPs are actually connected wires which transfer data to you. If not private, they are peers using public IPs.

@KaptenJansson

This comment has been minimized.

Copy link
Author

@KaptenJansson KaptenJansson commented Jan 19, 2016

Well ofc they are sending data, however it does not make sense showing host candidates that I can never connect to directly to (no one else on this network is visiting that page besides it displays private IP's on different subnets) thus the demo is not correctly conveying which endpoint I have established a peerconnection to.

@DiegoRBaquero

This comment has been minimized.

Copy link
Member

@DiegoRBaquero DiegoRBaquero commented Jan 19, 2016

Those are not candidates, else there would be more than 30 instantly. The shown peers are connected peers, not candidates.

On Jan 19, 2016, at 11:52 AM, Christoffer Jansson notifications@github.com wrote:

Well ofc they are sending data, however it does not make sense showing host candidates that I can never connect to directly thus the demo is not correctly conveying which endpoint I have established a peerconnection to.


Reply to this email directly or view it on GitHub.

@KaptenJansson

This comment has been minimized.

Copy link
Author

@KaptenJansson KaptenJansson commented Jan 19, 2016

Right, what I meant is that I cannot possibly connect to private IP's from my network (unless they are on my network) hence it looks like there something wrong with demo. As you mentioned, they are connected peers hence it makes sense displaying the IP address that I'm actually connected to.

It looks like you are using "any" candidate as a peer identifier in the demo rather than the actual connected IP address.

Does that make sense?

@feross

This comment has been minimized.

Copy link
Member

@feross feross commented Feb 11, 2016

@KaptenJansson Thanks for opening this issue. You are correct. Sometimes local IP address candidates are mistakenly shown on https://webtorrent.io. This is a bug in simple-peer.

I was able to fix this bug in Chrome, but it still exists in Firefox, as you can see here: feross/simple-peer#63

If you're interested, please contribute a fix!

(I'm going to close this issue here, since it's already being tracked on the simple-peer repo.)

@feross feross closed this Feb 11, 2016
@feross feross added the bug label Feb 11, 2016
@feross

This comment has been minimized.

Copy link
Member

@feross feross commented Feb 12, 2016

This is fixed now! Released in simple-peer 5.12.0.

feross added a commit to feross/simple-peer that referenced this issue Feb 12, 2016
@KaptenJansson

This comment has been minimized.

Copy link
Author

@KaptenJansson KaptenJansson commented Feb 15, 2016

Good stuff ;) When will it be pushed to webtorrent.io?

@feross

This comment has been minimized.

Copy link
Member

@feross feross commented Feb 18, 2016

It's published to webtorrent.io now.

@KaptenJansson

This comment has been minimized.

Copy link
Author

@KaptenJansson KaptenJansson commented Feb 19, 2016

Just tested it and now I just see public-ally accessible IP addresses, awesome!

@feross

This comment has been minimized.

Copy link
Member

@feross feross commented Feb 19, 2016

@KaptenJansson thanks!

@lock

This comment has been minimized.

Copy link

@lock lock bot commented May 4, 2018

This thread has been automatically locked because it has not had recent activity. To discuss futher, please open a new issue.

@lock lock bot locked as resolved and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.