client IPv6 support [BEP-7] #41
Conversation
There was a problem hiding this comment.
Is there ever a time when an IPv6 address won't have a : character? What is this check for?
There was a problem hiding this comment.
From my reading of the spec, the peers6 key should only contain ipv6 addresses.
peers6 contains address-port pairs where the addresses are all IPv6
And IPv6 addresses seem to always have : because that's the delimiter. So I can't figure out what this line is for.
I'll remove it for now, but let me know if it's important and we can re-add it.
There was a problem hiding this comment.
I was adding some tolerance in #40, and because the non-compact format essentially allows free-form hostnames, why not check if this is an IPv6 address at all?
It's not important to me and might lead interoperating people to assume a false level of flexibility in the HTTP tracker protocol.
There was a problem hiding this comment.
I see what you're saying. It's not hard to check if it's a domain, so let's do it.
|
Okay, this is all merged and published as 2.9.0 now! Thanks for all the awesome PRs, @astro! |
|
Also, I added you as a collaborator on this repo. Please read CONTRIBUTING.md and enjoy! |
There was a problem hiding this comment.
Whoa! Discovered a bigger bug. This should be data.peers6.forEach! Will fix. ;)
There was a problem hiding this comment.
Oopsie. Ideally, we'd cover this with tests...
There was a problem hiding this comment.
Once we support ipv6 on the server (#43) we can easily add a test that tests both sides of this. :)
2 participants
Depends on the next versions of the following modules:
You can test with torrents from Bitlove.org, the tracker always returns the IPv4 & IPv6 address of the prittorrent seeder.