Http2: Fix redirect-handling

The redirect handling for http2 was a little simple. E.g. not handling
relative URLs.
Fix this using the redirect response parsing function which the http1
protocol handler already uses.

Fixes: QTBUG-100651
Change-Id: Ic0cec4cacc92707e7a7fde1f4665f80995a6057e
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
(cherry picked from commit db5b8bbea3f3cf1675d2ddd449359b6fbedc523e)

4 files changed, 99 insertions, 7 deletions

diff --git a/src/network/access/qhttp2protocolhandler.cpp b/src/network/access/qhttp2protocolhandler.cpp
index c179546dbc..3409224f20 100644
--- a/src/network/access/qhttp2protocolhandler.cpp
+++ b/src/network/access/qhttp2protocolhandler.cpp
@@ -1135,8 +1135,6 @@ void QHttp2ProtocolHandler::updateStream(Stream &stream, const HPack::HttpHeader
     // moment and we are probably not done yet. So we extract url and set it
     // here, if needed.
     int statusCode = 0;
-    QUrl redirectUrl;
-
     for (const auto &pair : headers) {
         const auto &name = pair.name;
         auto value = pair.value;
@@ -1159,8 +1157,6 @@ void QHttp2ProtocolHandler::updateStream(Stream &stream, const HPack::HttpHeader
             if (ok)
                 httpReply->setContentLength(length);
         } else {
-            if (name == "location")
-                redirectUrl = QUrl::fromEncoded(value);
             QByteArray binder(", ");
             if (name == "set-cookie")
                 binder = "\n";
@@ -1225,8 +1221,20 @@ void QHttp2ProtocolHandler::updateStream(Stream &stream, const HPack::HttpHeader
         }
     }
 
-    if (QHttpNetworkReply::isHttpRedirect(statusCode) && redirectUrl.isValid())
-        httpReply->setRedirectUrl(redirectUrl);
+    if (QHttpNetworkReply::isHttpRedirect(statusCode) && httpRequest.isFollowRedirects()) {
+        QHttpNetworkConnectionPrivate::ParseRedirectResult result =
+                m_connection->d_func()->parseRedirectResponse(httpReply);
+        if (result.errorCode != QNetworkReply::NoError) {
+            auto errorString = m_connection->d_func()->errorDetail(result.errorCode, m_socket);
+            finishStreamWithError(stream, result.errorCode, errorString);
+            sendRST_STREAM(stream.streamID, INTERNAL_ERROR);
+            markAsReset(stream.streamID);
+            return;
+        }
+
+        if (result.redirectUrl.isValid())
+            httpReply->setRedirectUrl(result.redirectUrl);
+    }
 
     if (httpReplyPrivate->isCompressed() && httpRequest.d->autoDecompress)
         httpReplyPrivate->removeAutoDecompressHeader();
diff --git a/tests/auto/network/access/http2/http2srv.cpp b/tests/auto/network/access/http2/http2srv.cpp
index e2f039e2f7..ab760dd2b0 100644
--- a/tests/auto/network/access/http2/http2srv.cpp
+++ b/tests/auto/network/access/http2/http2srv.cpp
@@ -125,6 +125,12 @@ void Http2Server::setAuthenticationHeader(const QByteArray &authentication)
     authenticationHeader = authentication;
 }
 
+void Http2Server::setRedirect(const QByteArray &url, int count)
+{
+    redirectUrl = url;
+    redirectCount = count;
+}
+
 void Http2Server::emulateGOAWAY(int timeout)
 {
     Q_ASSERT(timeout >= 0);
@@ -855,7 +861,10 @@ void Http2Server::sendResponse(quint32 streamID, bool emptyBody)
         const QString url("%1://localhost:%2/");
         header.push_back({"location", url.arg(isClearText() ? QStringLiteral("http") : QStringLiteral("https"),
                                               QString::number(targetPort)).toLatin1()});
-
+    } else if (redirectCount > 0) { // Not redirecting while reading, unlike above
+        --redirectCount;
+        header.push_back({":status", "308"});
+        header.push_back({"location", redirectUrl});
     } else if (!authenticationHeader.isEmpty() && !hasAuth) {
         header.push_back({ ":status", "401" });
         header.push_back(HPack::HeaderField("www-authenticate", authenticationHeader));
diff --git a/tests/auto/network/access/http2/http2srv.h b/tests/auto/network/access/http2/http2srv.h
index 013af86cc8..5f8c5aa77e 100644
--- a/tests/auto/network/access/http2/http2srv.h
+++ b/tests/auto/network/access/http2/http2srv.h
@@ -88,6 +88,9 @@ public:
     void setResponseBody(const QByteArray &body);
     // No authentication data is generated for the method, the full header value must be set
     void setAuthenticationHeader(const QByteArray &authentication);
+    // Set the redirect URL and count. The server will return a redirect response with the url
+    // 'count' amount of times
+    void setRedirect(const QByteArray &redirectUrl, int count);
     void emulateGOAWAY(int timeout);
     void redirectOpenStream(quint16 targetPort);
 
@@ -219,6 +222,9 @@ private:
     QAtomicInt interrupted;
 
     QByteArray authenticationHeader;
+
+    QByteArray redirectUrl;
+    int redirectCount = 0;
 protected slots:
     void ignoreErrorSlot();
 };
diff --git a/tests/auto/network/access/http2/tst_http2.cpp b/tests/auto/network/access/http2/tst_http2.cpp
index 0610d0a301..bf8c779909 100644
--- a/tests/auto/network/access/http2/tst_http2.cpp
+++ b/tests/auto/network/access/http2/tst_http2.cpp
@@ -115,6 +115,9 @@ private slots:
     void authenticationRequired_data();
     void authenticationRequired();
 
+    void redirect_data();
+    void redirect();
+
 protected slots:
     // Slots to listen to our in-process server:
     void serverStarted(quint16 port);
@@ -955,6 +958,72 @@ void tst_Http2::authenticationRequired()
     QTRY_VERIFY(serverGotSettingsACK);
 }
 
+void tst_Http2::redirect_data()
+{
+    QTest::addColumn<int>("maxRedirects");
+    QTest::addColumn<int>("redirectCount");
+    QTest::addColumn<bool>("success");
+
+    QTest::addRow("1-redirects-none-allowed-failure") << 0 << 1 << false;
+    QTest::addRow("1-redirects-success") << 1 << 1 << true;
+    QTest::addRow("2-redirects-1-allowed-failure") << 1 << 2 << false;
+}
+
+void tst_Http2::redirect()
+{
+    QFETCH(const int, maxRedirects);
+    QFETCH(const int, redirectCount);
+    QFETCH(const bool, success);
+    const QByteArray redirectUrl = "/b.html";
+
+    clearHTTP2State();
+    serverPort = 0;
+
+    ServerPtr targetServer(newServer(defaultServerSettings, defaultConnectionType()));
+    targetServer->setRedirect(redirectUrl, redirectCount);
+
+    QMetaObject::invokeMethod(targetServer.data(), "startServer", Qt::QueuedConnection);
+    runEventLoop();
+
+    QVERIFY(serverPort != 0);
+
+    nRequests = 1 + maxRedirects;
+
+    auto originalUrl = requestUrl(defaultConnectionType());
+    auto url = originalUrl;
+    url.setPath("/index.html");
+    QNetworkRequest request(url);
+    request.setAttribute(QNetworkRequest::Http2AllowedAttribute, QVariant(true));
+    request.setAttribute(QNetworkRequest::RedirectPolicyAttribute, QNetworkRequest::NoLessSafeRedirectPolicy);
+    request.setMaximumRedirectsAllowed(maxRedirects);
+
+    QScopedPointer<QNetworkReply> reply;
+    reply.reset(manager->get(request));
+
+    if (success) {
+        connect(reply.get(), &QNetworkReply::finished, this, &tst_Http2::replyFinished);
+    } else {
+        connect(reply.get(), &QNetworkReply::errorOccurred, this,
+                &tst_Http2::replyFinishedWithError);
+    }
+
+    // Since we're using self-signed certificates,
+    // ignore SSL errors:
+    reply->ignoreSslErrors();
+
+    runEventLoop();
+    STOP_ON_FAILURE
+
+    if (success) {
+        QCOMPARE(reply->error(), QNetworkReply::NoError);
+        QCOMPARE(reply->url().toString(),
+                 originalUrl.resolved(QString::fromLatin1(redirectUrl)).toString());
+    } else if (maxRedirects < redirectCount) {
+        QCOMPARE(reply->error(), QNetworkReply::TooManyRedirectsError);
+    }
+    QTRY_VERIFY(serverGotSettingsACK);
+}
+
 void tst_Http2::serverStarted(quint16 port)
 {
     serverPort = port;