qbittorrent / qBittorrent

qBittorrent BitTorrent client
https://www.qbittorrent.org
Other
27.62k stars 3.91k forks source link

Qt GUI tracklist filter widget may display wrong top-level domain for tlds like `.co.uk` #19035

Closed ttys3 closed 1 year ago

ttys3 commented 1 year ago

qBittorrent & operating system versions

qBittorrent v4.5.2 Linux Fedora 38 Qt: 6.4.2 Libtorrent: 2.0.8.0 Boost: 1.78.0 OpenSSL: 3.0.8 zlib: 1.2.13

What is the problem?

found the issue during this PR: https://github.com/qbittorrent/qBittorrent/pull/18190

the GUI version has a bug dealing with hostname.

in QString getHost(const QString &url) (https://github.com/qbittorrent/qBittorrent/blob/ecc08dee0969b69330ca32b762df1a847ca2cb89/src/gui/transferlistfilters/trackersfilterwidget.cpp#L76)

it simply do host.section(u'.', -2, -1), which will cause https://www.google.co.uk become co.uk, not the right google.co.uk

Steps to reproduce

just copy the QString getHost(const QString &url) func out, and do some tests:

#include <QCoreApplication>
#include <QDebug>
#include <QtGlobal>
#include <QtNetwork>

QString getHost(const QString &url)
{
    // We want the domain + tld. Subdomains should be disregarded
    // If failed to parse the domain or IP address, original input should be returned

    const QString host = QUrl(url).host();
    if (host.isEmpty())
        return url;

    // host is in IP format
    if (!QHostAddress(host).isNull())
        return host;

    return host.section(u'.', -2, -1);
}

int main(int argc, char *argv[]) {
    QCoreApplication a(argc, argv);
    qInfo() << "tests: ";

    // QString array
    QStringList list;
    list << "https://www.google.com";
    list << "https://www.google.co.uk";
    list << "https://www.google.jp";
    list << "https://tracker.example.com/?passkey=xxxxx";

    // loop through the array
    for (int i = 0; i < list.size(); ++i) {
        qInfo() << getHost(list.at(i));
    }
// output:
//    "google.com"
//    "co.uk"
//    "google.jp"
//    "example.com"

    return QCoreApplication::exec();
}

Additional context

No response

Log(s) & preferences file(s)

No response

ttys3 commented 1 year ago

or, maybe take this comment ? https://github.com/qbittorrent/qBittorrent/pull/18190#issuecomment-1567697482

Chocobo1 commented 1 year ago

or, maybe take this comment ? https://github.com/qbittorrent/qBittorrent/pull/18190#issuecomment-1567697482

If my memory serves me right, we did try that but was quickly reverted due to users backlash, I'm not really sure. I don't mind if someone wants to try it. 😉

stalkerok commented 1 year ago

Maybe you can make it so that if this is still one tracker, then the user himself could combine it into one?

glassez commented 1 year ago

Maybe you can make it so that if this is still one tracker, then the user himself could combine it into one?

It looks like it could be implemented through something like tracker aliases, e.g. if given example.com=tracker1.example.com;tracker2.example.com, then they are all counted as example.com.

ttys3 commented 1 year ago

good to see https://github.com/qbittorrent/qBittorrent/pull/19062

tearfur commented 1 year ago

If my memory serves me right, we did try that but was quickly reverted due to users backlash, I'm not really sure. I don't mind if someone wants to try it.

I did some tracing, here is the history of the relevant code in reverse chronological order (as far as I can tell). It doesn't look like there was any backlash/revert of sorts, so I think it is worth a try to try use hostnames instead.

Date Commit PR Change
2023-04-03 0dcb65bb7cc2f4f65405712d6159d429abb5a3a5 #18801 Code was moved from src/gui/transferlistfilterswidget.cpp to src/gui/transferlistfilters/trackersfilterwidget.cpp
2022-03-18 802ec5a14e8eb659549cc606b4a1920cdbf5c6d1 #16652 Changed from ordinary string literals to QString literals
2020-12-06 9f0429ca6ffc7f18b88bc215f006a7bdead75674 #13912 Moved getHost() from TrackerFiltersList to anonymous namespace
2020-06-04 15b2811fa53774c7a7ee3d1c814703f0d2f8a292 #12969 Moved from using QUrl::topLevelDomain() to the current implementation. It doesn't look there was any (intentional) change in behaviour
2015-03-28 f0d5ce4b98428d160acbbda2ad4c52364242bb1a #2725 The tracker side panel was first implemented
stalkerok commented 11 months ago

Maybe you can make it so that if this is still one tracker, then the user himself could combine it into one?

It looks like it could be implemented through something like tracker aliases, e.g. if given example.com=tracker1.example.com;tracker2.example.com, then they are all counted as example.com.

@glassez, if you can find the time to implement this, that would be very nice. Thanks.

glassez commented 11 months ago

@stalkerok Is there really a lot of such trackers (having multiple subdomains) in the wild?

stalkerok commented 11 months ago

Quite a lot. For example

http://tr0.torrent4me.com/ann?uk=
http://tr1.torrent4me.com/ann?uk=
http://tr2.torrent4me.com/ann?uk=
http://tr3.torrent4me.com/ann?uk=
http://tr4.torrent4me.com/ann?uk=
http://tr5.torrent4me.com/ann?uk=

or

http://tr0.tor4me.info/ann?uk=
http://tr1.tor4me.info/ann?uk=
http://tr2.tor4me.info/ann?uk=
http://tr3.tor4me.info/ann?uk=
http://tr4.tor4me.info/ann?uk=
http://tr5.tor4me.info/ann?uk=
http://tr6.tor4me.info/ann?uk=
http://tr10.tor4me.info/ann?uk=
http://tr100.tor4me.info/ann?uk=
http://tr1000.tor4me.info/ann?uk=

It is the same tracker. You can put any digit instead of the tracker number and it will work. More examples

https://bt.t-ru.org/ann?pk=
https://bt2.t-ru.org/ann?pk=
https://bt3.t-ru.org/ann?pk=
https://bt4.t-ru.org/ann?pk=
http://bt01.nnm-club.cc:2710/ann?uk=
http://bt02.nnm-club.cc:2710/ann?uk=
http://bt01.nnm-club.info:2710/ann?uk=
http://bt02.nnm-club.info:2710/ann?uk=
http://bt01.ipv6.nnm-club.cc:2710/ann?uk=
http://bt02.ipv6.nnm-club.cc:2710/ann?uk=
http://bt01.ipv6.nnm-club.info:2710/ann?uk=
http://bt02.ipv6.nnm-club.info:2710/ann?uk=
http://[2a01:d0:a580:1::2]:2710/ann?uk=

There are only 4 trackers in the screenshot 2023-10-31_152906

glassez commented 11 months ago

I wonder what is the point in such subdomains...