Merge pull request #11995 from glassez/fix-magnet-redirect

Handle HTTP redirections manually

src/base/net/downloadmanager.cpp

@@ -52,8 +52,6 @@ namespace
     // Disguise as Firefox to avoid web server banning
     const char DEFAULT_USER_AGENT[] = "Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Firefox/68.0";
 
-    const int MAX_REDIRECTIONS = 20;  // the common value for web browsers
-
     class NetworkCookieJar : public QNetworkCookieJar
     {
     public:
@@ -123,9 +121,8 @@ namespace
         request.setRawHeader("Referer", request.url().toEncoded().data());
         // Accept gzip
         request.setRawHeader("Accept-Encoding", "gzip");
-
+        // Qt doesn't support Magnet protocol so we need to handle redirections manually
-        request.setAttribute(QNetworkRequest::RedirectPolicyAttribute, QNetworkRequest::UserVerifiedRedirectPolicy);
+        request.setAttribute(QNetworkRequest::RedirectPolicyAttribute, QNetworkRequest::ManualRedirectPolicy);
-        request.setMaximumRedirectsAllowed(MAX_REDIRECTIONS);
 
         return request;
     }
@@ -168,7 +165,7 @@ Net::DownloadHandler *Net::DownloadManager::download(const DownloadRequest &down
     const ServiceID id = ServiceID::fromURL(request.url());
     const bool isSequentialService = m_sequentialServices.contains(id);
 
-    auto downloadHandler = new DownloadHandlerImpl {downloadRequest, this};
+    auto downloadHandler = new DownloadHandlerImpl {this, downloadRequest};
     connect(downloadHandler, &DownloadHandler::finished, downloadHandler, &QObject::deleteLater);
     connect(downloadHandler, &QObject::destroyed, this, [this, id, downloadHandler]()
     {

src/base/net/private/downloadhandlerimpl.cpp

@@ -36,6 +36,8 @@
 #include "base/utils/gzip.h"
 #include "base/utils/misc.h"
 
+const int MAX_REDIRECTIONS = 20;  // the common value for web browsers
+
 namespace
 {
     bool saveToFile(const QByteArray &replyData, QString &filePath)
@@ -53,8 +55,9 @@ namespace
     }
 }
 
-DownloadHandlerImpl::DownloadHandlerImpl(const Net::DownloadRequest &downloadRequest, QObject *parent)
+DownloadHandlerImpl::DownloadHandlerImpl(Net::DownloadManager *manager, const Net::DownloadRequest &downloadRequest)
-    : DownloadHandler {parent}
+    : DownloadHandler {manager}
+    , m_manager {manager}
     , m_downloadRequest {downloadRequest}
 {
     m_result.url = url();
@@ -86,7 +89,6 @@ void DownloadHandlerImpl::assignNetworkReply(QNetworkReply *reply)
     if (m_downloadRequest.limit() > 0)
         connect(m_reply, &QNetworkReply::downloadProgress, this, &DownloadHandlerImpl::checkDownloadSize);
     connect(m_reply, &QNetworkReply::finished, this, &DownloadHandlerImpl::processFinishedDownload);
-    connect(m_reply, &QNetworkReply::redirected, this, &DownloadHandlerImpl::handleRedirection);
 }
 
 // Returns original url
@@ -113,6 +115,13 @@ void DownloadHandlerImpl::processFinishedDownload()
         return;
     }
 
+    // Check if the server ask us to redirect somewhere else
+    const QVariant redirection = m_reply->attribute(QNetworkRequest::RedirectionTargetAttribute);
+    if (redirection.isValid()) {
+        handleRedirection(redirection.toUrl());
+        return;
+    }
+
     // Success
     m_result.data = (m_reply->rawHeader("Content-Encoding") == "gzip")
                     ? Utils::Gzip::decompress(m_reply->readAll())
@@ -139,7 +148,7 @@ void DownloadHandlerImpl::checkDownloadSize(const qint64 bytesReceived, const qi
 
     if ((bytesTotal > m_downloadRequest.limit()) || (bytesReceived > m_downloadRequest.limit())) {
         m_reply->abort();
-        setError(tr("The file size is %1. It exceeds the download limit of %2.")
+        setError(tr("The file size (%1) exceeds the download limit (%2)")
                  .arg(Utils::Misc::friendlyUnit(bytesTotal)
                       , Utils::Misc::friendlyUnit(m_downloadRequest.limit())));
         finish();
@@ -148,6 +157,12 @@ void DownloadHandlerImpl::checkDownloadSize(const qint64 bytesReceived, const qi
 
 void DownloadHandlerImpl::handleRedirection(const QUrl &newUrl)
 {
+    if (m_redirectionCount >= MAX_REDIRECTIONS) {
+        setError(tr("Exceeded max redirections (%1)").arg(MAX_REDIRECTIONS));
+        finish();
+        return;
+    }
+
     // Resolve relative urls
     const QUrl resolvedUrl = newUrl.isRelative() ? m_reply->url().resolved(newUrl) : newUrl;
     const QString newUrlString = resolvedUrl.toString();
@@ -158,13 +173,21 @@ void DownloadHandlerImpl::handleRedirection(const QUrl &newUrl)
         qDebug("Magnet redirect detected.");
         m_result.status = Net::DownloadStatus::RedirectedToMagnet;
         m_result.magnet = newUrlString;
-        m_result.errorString = tr("Redirected to magnet URI.");
+        m_result.errorString = tr("Redirected to magnet URI");
 
         finish();
         return;
     }
 
-    emit m_reply->redirectAllowed();
+    auto redirected = static_cast<DownloadHandlerImpl *>(
+                m_manager->download(Net::DownloadRequest(m_downloadRequest).url(newUrlString)));
+    redirected->m_redirectionCount = m_redirectionCount + 1;
+    connect(redirected, &DownloadHandlerImpl::finished, this, [this](const Net::DownloadResult &result)
+    {
+        m_result = result;
+        m_result.url = url();
+        finish();
+    });
 }
 
 void DownloadHandlerImpl::setError(const QString &error)

src/base/net/private/downloadhandlerimpl.h

@@ -42,7 +42,7 @@ class DownloadHandlerImpl : public Net::DownloadHandler
     Q_DISABLE_COPY(DownloadHandlerImpl)
 
 public:
-    explicit DownloadHandlerImpl(const Net::DownloadRequest &downloadRequest, QObject *parent);
+    DownloadHandlerImpl(Net::DownloadManager *manager, const Net::DownloadRequest &downloadRequest);
     ~DownloadHandlerImpl() override;
 
     void cancel() override;
@@ -61,7 +61,9 @@ private:
 
     static QString errorCodeToString(QNetworkReply::NetworkError status);
 
+    Net::DownloadManager *m_manager = nullptr;
     QNetworkReply *m_reply = nullptr;
     const Net::DownloadRequest m_downloadRequest;
+    short m_redirectionCount = 0;
     Net::DownloadResult m_result;
 };