Skip to content

Conversation

@astro
Copy link
Contributor

@astro astro commented Nov 23, 2014

BEP-3 states:

peers maps to a list of dictionaries corresponding to peers, each of which contains the keys peer id, ip, and port, which map to the peer's self-selected ID, IP address or dns name as a string, and port number, respectively.

The Theory.org Wiki agrees.

Is there any evidence of binary ip fields in the wild? Anyway, this code should support both.

@feross
Copy link
Member

feross commented Nov 26, 2014

Is there any evidence of binary ip fields in the wild?

Are you referring to compact style responses, where peers are identified by 6 bytes (4 for IP and 2 for port)? That's the most common way that trackers return responses, since it's more efficient. See:

compact: Setting this to 1 indicates that the client accepts a compact response. The peers list is replaced by a peers string with 6 bytes per peer. The first four bytes are the host (in network byte order), the last two bytes are the port (again in network byte order). It should be noted that some trackers only support compact responses (for saving bandwidth) and either refuse requests without "compact=1" or simply send a compact response unless the request contains "compact=0" (in which case they will refuse the request.)

In fact, the biggest trackers just listed on udp and disable http since it's less efficient and pretty much all clients support udp these days. Still, I want there to be an http implementation in here since it's simpler and sometimes that's all you want/need. :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this was not doing what it was supposed to. This code is written to handle the case where the server sends back a compact response, pulling out each individual byte (like compact2string does), which is already handled above. We just need the ip.toString() part only.

@feross
Copy link
Member

feross commented Nov 26, 2014

Fixed and published as 2.8.1.

@feross feross closed this Nov 26, 2014
@astro astro mentioned this pull request Nov 27, 2014
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 this pull request may close these issues.

2 participants