Let page interceptor process requests with extra headers

If a QWebEngineUrlRequestInterceptor has added extra headers to
the request on profile level, later the request was not processed
by the page interceptor. Relax this rule and only prevent the
interception of blocked or redirected requests.

Add auto test and extend documentation.

Pick-to: 6.6
Task-number: QTBUG-117752
Change-Id: I503bfc256a5e874a678be64c597c74e04e7ce966
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

4 files changed, 71 insertions, 13 deletions

diff --git a/src/core/api/qwebenginepage.cpp b/src/core/api/qwebenginepage.cpp
index 64343660b..d3121990b 100644
--- a/src/core/api/qwebenginepage.cpp
+++ b/src/core/api/qwebenginepage.cpp
@@ -1707,7 +1707,9 @@ void QWebEnginePagePrivate::visibleChanged(bool visible)
 
     The page does not take ownership of the pointer. This interceptor is called
     after any interceptors on the profile, and unlike profile interceptors, only
-    URL requests from this page are intercepted.
+    URL requests from this page are intercepted. If the original request was
+    already blocked or redirected by the profile interceptor, it will not be
+    intercepted by this.
 
     To unset the request interceptor, set a \c nullptr.
 
diff --git a/src/core/api/qwebengineurlrequestinfo.cpp b/src/core/api/qwebengineurlrequestinfo.cpp
index 448231781..152cf4dd0 100644
--- a/src/core/api/qwebengineurlrequestinfo.cpp
+++ b/src/core/api/qwebengineurlrequestinfo.cpp
@@ -302,8 +302,6 @@ void QWebEngineUrlRequestInfo::block(bool shouldBlock)
 
 void QWebEngineUrlRequestInfo::setHttpHeader(const QByteArray &name, const QByteArray &value)
 {
-    d_ptr->changed = true;
-
     // Headers are case insentive, so we need to compare manually
     for (auto it = d_ptr->extraHeaders.begin(); it != d_ptr->extraHeaders.end(); ++it) {
         if (it.key().compare(name, Qt::CaseInsensitive) == 0) {
diff --git a/src/core/net/proxying_url_loader_factory_qt.cpp b/src/core/net/proxying_url_loader_factory_qt.cpp
index 7fd7a866d..3a83ed7ea 100644
--- a/src/core/net/proxying_url_loader_factory_qt.cpp
+++ b/src/core/net/proxying_url_loader_factory_qt.cpp
@@ -364,19 +364,18 @@ void InterceptedRequest::ContinueAfterIntercept()
         const auto scoped_request_info = std::move(request_info_);
         QWebEngineUrlRequestInfoPrivate &info = *scoped_request_info->d_ptr;
 
+        for (auto header = info.extraHeaders.constBegin(); header != info.extraHeaders.constEnd(); ++header) {
+            std::string h = header.key().toStdString();
+            if (base::EqualsCaseInsensitiveASCII(h, "referer"))
+                request_.referrer = GURL(header.value().toStdString());
+            else
+                request_.headers.SetHeader(h, header.value().toStdString());
+        }
+
         if (info.changed) {
             if (info.shouldBlockRequest)
                 return SendErrorAndCompleteImmediately(net::ERR_BLOCKED_BY_CLIENT);
 
-            for (auto header = info.extraHeaders.constBegin(); header != info.extraHeaders.constEnd(); ++header) {
-                std::string h = header.key().toStdString();
-                if (base::EqualsCaseInsensitiveASCII(h, "referer")) {
-                    request_.referrer = GURL(header.value().toStdString());
-                } else {
-                    request_.headers.SetHeader(h, header.value().toStdString());
-                }
-            }
-
             if (info.shouldRedirectRequest) {
                 net::RedirectInfo::FirstPartyURLPolicy first_party_url_policy =
                         request_.update_first_party_url_on_redirect ? net::RedirectInfo::FirstPartyURLPolicy::UPDATE_URL_ON_REDIRECT
diff --git a/tests/auto/core/qwebengineurlrequestinterceptor/tst_qwebengineurlrequestinterceptor.cpp b/tests/auto/core/qwebengineurlrequestinterceptor/tst_qwebengineurlrequestinterceptor.cpp
index 5c983a210..76578f5ec 100644
--- a/tests/auto/core/qwebengineurlrequestinterceptor/tst_qwebengineurlrequestinterceptor.cpp
+++ b/tests/auto/core/qwebengineurlrequestinterceptor/tst_qwebengineurlrequestinterceptor.cpp
@@ -53,6 +53,8 @@ private Q_SLOTS:
     void multipleRedirects();
     void postWithBody_data();
     void postWithBody();
+    void profilePreventsPageInterception_data();
+    void profilePreventsPageInterception();
 };
 
 tst_QWebEngineUrlRequestInterceptor::tst_QWebEngineUrlRequestInterceptor()
@@ -139,7 +141,7 @@ public:
 
         // MEMO avoid unintentionally changing request when it is not needed for test logic
         //      since api behavior depends on 'changed' state of the info object
-        Q_ASSERT(info.changed() == (block || redirect || !headers.empty()));
+        Q_ASSERT(info.changed() == (block || redirect));
     }
 
     bool shouldSkipRequest(const RequestInfo &requestInfo)
@@ -1059,5 +1061,62 @@ void tst_QWebEngineUrlRequestInterceptor::postWithBody()
     QVERIFY(interceptor.isCalled);
 }
 
+class PageOrProfileInterceptor : public QWebEngineUrlRequestInterceptor
+{
+public:
+    PageOrProfileInterceptor(const QString &profileAction)
+        : profileAction(profileAction)
+    {
+    }
+
+    void interceptRequest(QWebEngineUrlRequestInfo &info) override
+    {
+        if (profileAction == "block")
+            info.block(true);
+        else if (profileAction == "redirect")
+            info.redirect(QUrl("data:text/html,<p>redirected"));
+        else if (profileAction == "add header")
+            info.setHttpHeader("Custom-Header", "Value");
+        else
+            QVERIFY(info.httpHeaders().contains("Custom-Header"));
+        ran = true;
+    }
+
+    QString profileAction;
+    bool ran = false;
+};
+
+void tst_QWebEngineUrlRequestInterceptor::profilePreventsPageInterception_data()
+{
+    QTest::addColumn<QString>("profileAction");
+    QTest::addColumn<bool>("interceptInProfile");
+    QTest::addColumn<bool>("interceptInPage");
+    QTest::newRow("block") << "block" << true << false;
+    QTest::newRow("redirect") << "redirect" << true << false;
+    QTest::newRow("add header") << "add header" << true << true;
+}
+
+void tst_QWebEngineUrlRequestInterceptor::profilePreventsPageInterception()
+{
+    QFETCH(QString, profileAction);
+    QFETCH(bool, interceptInProfile);
+    QFETCH(bool, interceptInPage);
+
+    QWebEngineProfile profile;
+    PageOrProfileInterceptor profileInterceptor(profileAction);
+    profile.setUrlRequestInterceptor(&profileInterceptor);
+    profile.settings()->setAttribute(QWebEngineSettings::ErrorPageEnabled, false);
+
+    QWebEnginePage page(&profile);
+    PageOrProfileInterceptor pageInterceptor("");
+    page.setUrlRequestInterceptor(&pageInterceptor);
+    QSignalSpy loadSpy(&page, SIGNAL(loadFinished(bool)));
+
+    page.load(QUrl("qrc:///resources/index.html"));
+    QTRY_COMPARE(loadSpy.size(), 1);
+    QCOMPARE(profileInterceptor.ran, interceptInProfile);
+    QCOMPARE(pageInterceptor.ran, interceptInPage);
+}
+
 QTEST_MAIN(tst_QWebEngineUrlRequestInterceptor)
 #include "tst_qwebengineurlrequestinterceptor.moc"