Recreate response head objects on multiple redirect

The previous response head gets moved when redirecting, which lead to
dereferencing a null pointer on the next redirect.

Fixes: QTBUG-109357
Change-Id: Iaad1c46b8d4ca9720f1749980a9e06337ca0f3d8
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
(cherry picked from commit c6b2b5d8038b3ec0de6233c1e21df60ade11c81b)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

4 files changed, 73 insertions, 8 deletions

diff --git a/src/core/net/proxying_url_loader_factory_qt.cpp b/src/core/net/proxying_url_loader_factory_qt.cpp
index a4713150b..1dc1b1577 100644
--- a/src/core/net/proxying_url_loader_factory_qt.cpp
+++ b/src/core/net/proxying_url_loader_factory_qt.cpp
@@ -30,6 +30,18 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+namespace {
+    network::mojom::URLResponseHeadPtr createResponse(const network::ResourceRequest &request) {
+        const bool disable_web_security = base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableWebSecurity);
+        network::mojom::URLResponseHeadPtr response = network::mojom::URLResponseHead::New();
+        response->response_type = network::cors::CalculateResponseType(
+            request.mode, disable_web_security || (
+            request.request_initiator && request.request_initiator->IsSameOriginWith(url::Origin::Create(request.url))));
+
+        return response;
+    }
+}
+
 namespace QtWebEngineCore {
 
 ASSERT_ENUMS_MATCH(QWebEngineUrlRequestInfo::ResourceTypeMainFrame, blink::mojom::ResourceType::kMainFrame)
@@ -177,11 +189,7 @@ InterceptedRequest::InterceptedRequest(ProfileAdapter *profile_adapter,
     , weak_factory_(this)
 {
     const bool disable_web_security = base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableWebSecurity);
-    current_response_ = network::mojom::URLResponseHead::New();
-    current_response_->response_type = network::cors::CalculateResponseType(
-        request_.mode,
-        disable_web_security || (
-            request_.request_initiator && request_.request_initiator->IsSameOriginWith(url::Origin::Create(request_.url))));
+    current_response_ = createResponse(request_);
     // If there is a client error, clean up the request.
     target_client_.set_disconnect_handler(
             base::BindOnce(&InterceptedRequest::OnURLLoaderClientError, base::Unretained(this)));
@@ -350,9 +358,6 @@ void InterceptedRequest::ContinueAfterIntercept()
                         first_party_url_policy, request_.referrer_policy, request_.referrer.spec(),
                         net::HTTP_TEMPORARY_REDIRECT, toGurl(info.url), absl::nullopt,
                         false /*insecure_scheme_was_upgraded*/);
-
-                // FIXME: Should probably create a new header.
-                current_response_->encoded_data_length = 0;
                 request_.method = redirectInfo.new_method;
                 request_.url = redirectInfo.new_url;
                 request_.site_for_cookies = redirectInfo.new_site_for_cookies;
@@ -360,6 +365,11 @@ void InterceptedRequest::ContinueAfterIntercept()
                 request_.referrer_policy = redirectInfo.new_referrer_policy;
                 if (request_.method == net::HttpRequestHeaders::kGetMethod)
                     request_.request_body = nullptr;
+                // In case of multiple sequential rediredts, current_response_ has previously been moved to target_client_
+                // so we create a new one using the redirect url.
+                if (!current_response_)
+                    current_response_ = createResponse(request_);
+                current_response_->encoded_data_length = 0;
                 target_client_->OnReceiveRedirect(redirectInfo, std::move(current_response_));
                 return;
             }
diff --git a/tests/auto/core/qwebengineurlrequestinterceptor/CMakeLists.txt b/tests/auto/core/qwebengineurlrequestinterceptor/CMakeLists.txt
index 0f8d90a08..e03caa6d7 100644
--- a/tests/auto/core/qwebengineurlrequestinterceptor/CMakeLists.txt
+++ b/tests/auto/core/qwebengineurlrequestinterceptor/CMakeLists.txt
@@ -12,6 +12,8 @@ qt_internal_add_test(tst_qwebengineurlrequestinterceptor
 
 set(tst_qwebengineurlrequestinterceptor_resource_files
     "resources/content.html"
+    "resources/content2.html"
+    "resources/content3.html"
     "resources/favicon.html"
     "resources/firstparty.html"
     "resources/fontawesome.woff"
diff --git a/tests/auto/core/qwebengineurlrequestinterceptor/resources/content3.html b/tests/auto/core/qwebengineurlrequestinterceptor/resources/content3.html
new file mode 100644
index 000000000..84bf55036
--- /dev/null
+++ b/tests/auto/core/qwebengineurlrequestinterceptor/resources/content3.html
@@ -0,0 +1,6 @@
+<html>
+<head><link rel="icon" href="data:,"></head>
+<body>
+<a>Simple test page without favicon (meaning no separate request from http server)</a>
+</body>
+</html>
diff --git a/tests/auto/core/qwebengineurlrequestinterceptor/tst_qwebengineurlrequestinterceptor.cpp b/tests/auto/core/qwebengineurlrequestinterceptor/tst_qwebengineurlrequestinterceptor.cpp
index 99cf3f244..f79a80641 100644
--- a/tests/auto/core/qwebengineurlrequestinterceptor/tst_qwebengineurlrequestinterceptor.cpp
+++ b/tests/auto/core/qwebengineurlrequestinterceptor/tst_qwebengineurlrequestinterceptor.cpp
@@ -45,6 +45,7 @@ private Q_SLOTS:
     void replaceInterceptor_data();
     void replaceInterceptor();
     void replaceOnIntercept();
+    void multipleRedirects();
 };
 
 tst_QWebEngineUrlRequestInterceptor::tst_QWebEngineUrlRequestInterceptor()
@@ -177,6 +178,29 @@ public:
     }
 };
 
+class TestMultipleRedirectsInterceptor : public QWebEngineUrlRequestInterceptor {
+public:
+    QList<RequestInfo> requestInfos;
+    QMap<QUrl, QUrl> redirectPairs;
+    int redirectCount = 0;
+    void interceptRequest(QWebEngineUrlRequestInfo &info) override
+    {
+        QVERIFY(QThread::currentThread() == QCoreApplication::instance()->thread());
+        qCDebug(lc) << this << "Type:" << info.resourceType() << info.requestMethod() << "Navigation:" << info.navigationType()
+                    << info.requestUrl() << "Initiator:" << info.initiator();
+        auto redirectUrl = redirectPairs.constFind(info.requestUrl());
+        if (redirectUrl != redirectPairs.constEnd()) {
+          info.redirect(redirectUrl.value());
+          requestInfos.append(info);
+          redirectCount++;
+        }
+    }
+
+    TestMultipleRedirectsInterceptor()
+    {
+    }
+};
+
 class ConsolePage : public QWebEnginePage {
     Q_OBJECT
 public:
@@ -850,5 +874,28 @@ void tst_QWebEngineUrlRequestInterceptor::replaceOnIntercept()
     QCOMPARE(profileInterceptor.requestInfos.size(), pageInterceptor2.requestInfos.size());
 }
 
+void tst_QWebEngineUrlRequestInterceptor::multipleRedirects()
+{
+    HttpServer server;
+    server.setResourceDirs({ ":/resources" });
+    QVERIFY(server.start());
+
+    TestMultipleRedirectsInterceptor multiInterceptor;
+    multiInterceptor.redirectPairs.insert(QUrl(server.url("/content.html")), QUrl(server.url("/content2.html")));
+    multiInterceptor.redirectPairs.insert(QUrl(server.url("/content2.html")), QUrl(server.url("/content3.html")));
+
+    QWebEngineProfile profile;
+    profile.settings()->setAttribute(QWebEngineSettings::ErrorPageEnabled, false);
+    profile.setUrlRequestInterceptor(&multiInterceptor);
+    QWebEnginePage page(&profile);
+    QSignalSpy spy(&page, SIGNAL(loadFinished(bool)));
+
+    page.setUrl(server.url("/content.html"));
+
+    QTRY_COMPARE_WITH_TIMEOUT(spy.count(), 1, 20000);
+    QTRY_COMPARE(multiInterceptor.redirectCount, 2);
+    QTRY_COMPARE(multiInterceptor.requestInfos.size(), 2);
+}
+
 QTEST_MAIN(tst_QWebEngineUrlRequestInterceptor)
 #include "tst_qwebengineurlrequestinterceptor.moc"