QWebEngineUrlRequestJob: QUrl("null") for unique initiator origins

The empty URL is used both for representing a missing origin (browser-initiated
navigation request) and a unique/opaque origin. This is problematic since the
security implications are very different in these two cases: browser-initiated
requests usually should have high security clearance, while requests from unique
origins should be restricted.

Task-number: QTBUG-69372
Change-Id: Iff73fd1c9a29f1c5c281a8945536333081ff2d6b
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

3 files changed, 59 insertions, 4 deletions

diff --git a/src/core/api/qwebengineurlrequestjob.cpp b/src/core/api/qwebengineurlrequestjob.cpp
index c028a1167..f741b88e8 100644
--- a/src/core/api/qwebengineurlrequestjob.cpp
+++ b/src/core/api/qwebengineurlrequestjob.cpp
@@ -115,9 +115,24 @@ QByteArray QWebEngineUrlRequestJob::requestMethod() const
 
 /*!
     \since 5.11
-    Returns the origin URL of the content that initiated the request. If the
-    request was not initiated by web content the function will return an
-    empty QUrl.
+    Returns the serialized origin of the content that initiated the request.
+
+    Generally, the origin consists of a scheme, hostname, and port. For example,
+    \c "http://localhost:8080" would be a valid origin. The port is omitted if
+    it is the scheme's default port (80 for \c http, 443 for \c https). The
+    hostname is omitted for non-network schemes such as \c file and \c qrc.
+
+    However, there is also the special value \c "null" representing a unique
+    origin. It is, for example, the origin of a sandboxed iframe. The purpose of
+    this special origin is to be always different from all other origins in the
+    same-origin check. In other words, content with a unique origin should never
+    have privileged access to any other content.
+
+    Finally, if the request was not initiated by web content, the function will
+    return an empty QUrl. This happens, for example, when you call \l
+    QWebEnginePage::setUrl().
+
+    This value can be used for implementing secure cross-origin checks.
 */
 QUrl QWebEngineUrlRequestJob::initiator() const
 {
diff --git a/src/core/net/url_request_custom_job_proxy.cpp b/src/core/net/url_request_custom_job_proxy.cpp
index 6c9824bb9..526ac2f8b 100644
--- a/src/core/net/url_request_custom_job_proxy.cpp
+++ b/src/core/net/url_request_custom_job_proxy.cpp
@@ -158,7 +158,7 @@ void URLRequestCustomJobProxy::initialize(GURL url, std::string method, base::Op
 
     QUrl initiatorOrigin;
     if (initiator.has_value())
-        initiatorOrigin = toQt(initiator.value().GetURL());
+        initiatorOrigin = QUrl::fromEncoded(QByteArray::fromStdString(initiator.value().Serialize()));
 
     QWebEngineUrlSchemeHandler *schemeHandler = 0;
     QSharedPointer<const BrowserContextAdapter> browserContext = m_adapter.toStrongRef();
diff --git a/tests/auto/widgets/qwebengineprofile/tst_qwebengineprofile.cpp b/tests/auto/widgets/qwebengineprofile/tst_qwebengineprofile.cpp
index 123cb7b32..3415b06c5 100644
--- a/tests/auto/widgets/qwebengineprofile/tst_qwebengineprofile.cpp
+++ b/tests/auto/widgets/qwebengineprofile/tst_qwebengineprofile.cpp
@@ -56,6 +56,7 @@ private Q_SLOTS:
     void httpAcceptLanguage();
     void downloadItem();
     void changePersistentPath();
+    void initiator();
 };
 
 void tst_QWebEngineProfile::init()
@@ -534,5 +535,44 @@ void tst_QWebEngineProfile::changePersistentPath()
     QVERIFY(newPath.endsWith(QStringLiteral("Test2")));
 }
 
+class InitiatorSpy : public QWebEngineUrlSchemeHandler
+{
+public:
+    QUrl initiator;
+    void requestStarted(QWebEngineUrlRequestJob *job) override
+    {
+        initiator = job->initiator();
+        job->fail(QWebEngineUrlRequestJob::RequestDenied);
+    }
+};
+
+void tst_QWebEngineProfile::initiator()
+{
+    InitiatorSpy handler;
+    QWebEngineProfile profile;
+    profile.installUrlSchemeHandler("foo", &handler);
+    QWebEnginePage page(&profile);
+    QSignalSpy loadFinishedSpy(&page, SIGNAL(loadFinished(bool)));
+
+    // about:blank has a unique origin, so initiator should be QUrl("null")
+    evaluateJavaScriptSync(&page, "window.location = 'foo:bar'");
+    QVERIFY(loadFinishedSpy.wait());
+    QCOMPARE(handler.initiator, QUrl("null"));
+
+    page.setHtml("", QUrl("http://test:123/foo%20bar"));
+    QVERIFY(loadFinishedSpy.wait());
+
+    // baseUrl determines the origin, so QUrl("http://test:123")
+    evaluateJavaScriptSync(&page, "window.location = 'foo:bar'");
+    QVERIFY(loadFinishedSpy.wait());
+    QCOMPARE(handler.initiator, QUrl("http://test:123"));
+
+    // Directly calling load/setUrl should have initiator QUrl(), meaning
+    // browser-initiated, trusted.
+    page.load(QUrl("foo:bar"));
+    QVERIFY(loadFinishedSpy.wait());
+    QCOMPARE(handler.initiator, QUrl());
+}
+
 QTEST_MAIN(tst_QWebEngineProfile)
 #include "tst_qwebengineprofile.moc"