diff --git a/src/core/NetworkRequest.cpp b/src/core/NetworkRequest.cpp index 22f93634c..c16a9991e 100644 --- a/src/core/NetworkRequest.cpp +++ b/src/core/NetworkRequest.cpp @@ -38,7 +38,6 @@ namespace void NetworkRequest::fetch(const QUrl& url) { - m_requested_url = url; m_finished = false; QNetworkRequest request(url); @@ -82,9 +81,12 @@ void NetworkRequest::fetchFinished() redirectTarget = url.resolved(redirectTarget); } // Request the redirect target + qDebug() << "Following redirect to" << redirectTarget; + m_redirects += 1; fetch(redirectTarget); return; } else { + qDebug() << "Too many redirects"; emit failure(); return; } diff --git a/src/core/NetworkRequest.h b/src/core/NetworkRequest.h index e1cab5357..1746add65 100644 --- a/src/core/NetworkRequest.h +++ b/src/core/NetworkRequest.h @@ -75,6 +75,9 @@ public: void cancel(); + /** + * @return The URL of the original request. Not updated after redirects. Use reply()->url() for the final URL including redirects. + */ QUrl URL() const; /** diff --git a/tests/TestNetworkRequest.cpp b/tests/TestNetworkRequest.cpp index a0523ebf3..026dcf492 100644 --- a/tests/TestNetworkRequest.cpp +++ b/tests/TestNetworkRequest.cpp @@ -176,5 +176,73 @@ void TestNetworkRequest::testNetworkRequestRedirects() { // Should respect max number of redirects // Headers, Reply, etc. should reflect final request - // TODO + + QFETCH(int, numRedirects); + QFETCH(int, maxRedirects); + const bool expectError = numRedirects > maxRedirects; + + const auto requestedURL = QUrl("https://example.com"); + const auto expectedUserAgent = QString("KeePassXC"); + + // Create mock reply + // Create and configure the mocked network access manager + MockNetworkAccess::Manager<QNetworkAccessManager> manager; + + QStringList requestedUrls; + + auto* reply = &manager + .whenGet(requestedURL) + // Has right user agent? + .has(MockNetworkAccess::Predicates::HeaderMatching(QNetworkRequest::UserAgentHeader, + QRegularExpression(expectedUserAgent))) + .reply(); + + for(int i = 0; i < numRedirects; ++i) { + auto redirectTarget = QUrl("https://example.com/redirect" + QString::number(i)); + reply->withRedirect(redirectTarget); + reply = &manager.whenGet(redirectTarget) + // Has right user agent? + .has(MockNetworkAccess::Predicates::HeaderMatching(QNetworkRequest::UserAgentHeader, + QRegularExpression(expectedUserAgent))) + .reply(); + } + reply->withBody(QString{"test-content"}.toUtf8()); + + // Create request + NetworkRequest request = createRequest(requestedURL, maxRedirects, std::chrono::milliseconds{5000}, QList<QPair<QString, QString>>{}, &manager); + + bool didSucceed = false, didError = false; + // Check request + QSignalSpy spy(&request, &NetworkRequest::success); + connect(&request, &NetworkRequest::success, [&didSucceed](const QByteArray&) { + didSucceed = true; + }); + + QSignalSpy errorSpy(&request, &NetworkRequest::failure); + connect(&request, &NetworkRequest::failure, [&didError]() { didError = true; }); + + + QTest::qWait(3*100); + + QTEST_ASSERT(didError || didSucceed); + // Ensures that predicates match - i.e., the header was set correctly + QCOMPARE(didSucceed, !expectError); + QCOMPARE(didError, expectError); + if(didSucceed) { + QCOMPARE(manager.matchedRequests().length(), numRedirects + 1); + QCOMPARE(request.URL(), requestedURL); + } } + +void TestNetworkRequest::testNetworkRequestRedirects_data() +{ + QTest::addColumn<int>("numRedirects"); + QTest::addColumn<int>("maxRedirects"); + + QTest::newRow("all good (0)") << 0 << 5; + QTest::newRow("all good (1)") << 1 << 5; + QTest::newRow("all good (2)") << 2 << 5; + QTest::newRow("no good (1, 0)") << 1 << 0; + QTest::newRow("no good (2, 1)") << 2 << 1; + QTest::newRow("no good (3, 2)") << 3 << 2; +} \ No newline at end of file diff --git a/tests/TestNetworkRequest.h b/tests/TestNetworkRequest.h index 8c92cbf68..140c6a0f9 100644 --- a/tests/TestNetworkRequest.h +++ b/tests/TestNetworkRequest.h @@ -16,6 +16,7 @@ private slots: void testNetworkRequestTimeout_data(); void testNetworkRequestRedirects(); + void testNetworkRequestRedirects_data(); }; #endif // KEEPASSXC_TESTNETWORKREQUEST_HPP