Implement Peer ID Client column for Peers tab

[HanabishiRecca]

It is a fairly useful feature. Implemented both for desktop and WebUI.

Originally introduced in qBittorrent Enhanced Edition, initial source is ported from there.

Screenshot

[Chocobo1]

It is a fairly useful feature.

Please elaborate.

[HanabishiRecca]

Please elaborate.

Well, it's just "more info is better". Clients can omit user-agent (Client column contains Unknown in this case) or fake it. But peer id is always present, as far as I'm concerned. Of course technically it can be faked too, but still better than nothing.

And I don't see reasons for not having it. Columns are customizable and can be hidden for those who don't want to see em.

[Chocobo1]

Please elaborate.

Well, it's just "more info is better".

This does not correlate with what you said in the opening post: "It is a fairly useful feature".
It doesn't even have a real world use case associated. Sounds like false advertising to me.

But peer id is always present, as far as I'm concerned.

IIRC Client column is parsed from peer_id. So effectively these 2 columns point to the same data but different representation.

While I don't strongly oppose this PR, I'm not going to back it up either.

[HanabishiRecca]

So effectively these 2 columns point to the same data but different representation.

I'm not really sure. Libtorrent does resolve some clients this way:
https://github.com/arvidn/libtorrent/blob/64817e0e8793d0875fc10245de52ffb2540a223d/src/identify_client.cpp#L146

But I clearly can see clients that are not on the list:

And peer ids here obviously can not represent such precise build numbers (like 5.26.13983).
But maybe I got it wrong and this PR is actually useless. Definitely need @arvidn help here.

[ghost]

Client column is the advertised user agent while the peer_id is completely different thing. Peer id of two clients will not match(in small swarms) but user agents could be same. I guess the peer_id shown here is truncated as the actual peer_id is 20 bytes. Here only the non random bytes are being displayed.

[HanabishiRecca]

Yeah. In fact even on my initial screenshot you can see a bunch of UW peers advertised themselfs as libtorrent/1.2.2.0, which is not very informative. But looking up by the peer id you can identify it as µTorrent Web + see the actual client version.
More comperhensive list can be found e.g. here.

[Chocobo1]

Client column is the advertised user agent while the peer_id is completely different thing.

Seems you are right on this. Here is the spec: https://www.bittorrent.org/beps/bep_0010.html
And interestingly from that spec:

v	Client name and version (as a utf-8 string). This is a much more reliable way of identifying the client than relying on the peer id encoding.

So if the peer doesn't support this Extension Protocol then Client field will be determined from peer_id.
When peer support Extension Protocol: https://github.com/arvidn/libtorrent/blob/550d3c7d9faeb3c59584b16fd0fc90a05e047ce5/src/bt_peer_connection.cpp#L2037
When it doesn't: https://github.com/arvidn/libtorrent/blob/550d3c7d9faeb3c59584b16fd0fc90a05e047ce5/src/bt_peer_connection.cpp#L3533

[arvidn]

I'll make a bunch of statements, and maybe some questions will be answered.

The peer_id is just a unique identifier to avoid multiple connections to the same peer, as well as avoiding connecting to oneself.
Very early on, clients started advertising themselves and their version, as a prefix to the peer_id, and the mainline (BitTorrent Inc.) client followed suit.

The peer_id, as it was originally intended (I believe), ended up not being very useful. You can't trust peers to use the same peer ID if they open multiple connections to you, so it's primarily helpful in identifying connections to yourself. The one other (subtle) use is if two clients connect to each other at the same time, the peer ID is used to determine which connection to disconnect and which one to keep. Both clients need to agree on this, otherwise they might also decide to close both connections (one each).

There are 4 places where a bittorrent client advertises a "user agent".

1. The actual UserAgent field in the HTTP tracker request. Advertised to trackers.
2. The peer ID field in the peer handshake. Advertised to other peers.
3. The version field in the peer extension handshake. Advertised to other peers.
4. The version field in DHT messages. Advertised to other DHT nodes.

I believe only (2) and maybe (3) is relevant for this ticket.

Presumably, in the screenshot above, where the Client column disagrees with the PeerID column, the peer advertised a different version than its peer_id suggested (e.g. peer ID UW and client version: libtorrent/2.2.0).

[HanabishiRecca]

Changes:

1. Sanitize peer id to only printable ASCII characters.
2. Return full string (arguable).

[HanabishiRecca]

Now I see a few possible ways:

1. Keep showing the full string. Usefulness of the random part is questionable.
2. Trim to 8 chars unconditionally. Kinda works 99% of the time.
3. Trim to 8 chars if printable else show the full string. But do we really want to see it?
4. Trim to 8 chars if printable else show nothing. Very high chance to show only sensible data and hide random noise. Sounds best to me.
5. Implement some more complicated parsing. But there is not much real cases when it can be useful. Also natural randomness can still interfere.

Will be good to hear more thoughts about it.

[glassez]

4. Trim to 8 chars if printable else show nothing. Very high chance to show only sensible data and hide random noise. Sounds best to me.

In case it is really needed I would prefer this option. Also I would show some placeholder in case of non printable id.

[HanabishiRecca]

Ok, will stick with this variant for now.

[ghost]

I think this column should be hidden by default.

[HanabishiRecca]

I think this column should be hidden by default.

Agree. But I can't figure out how to do it for desktop UI.
Easy for WebUI though, it has a dedicated column flag for that.

[HanabishiRecca]

Nvm, I made it.

[Chocobo1]

@HanabishiRecca
You should also increment the PATCH version number of API_VERSION in webapplication.h.

[Chocobo1]

LGTM

[HanabishiRecca]

Current state, just in case:

[Chocobo1]

@HanabishiRecca
Thank you!