From 746d61c92279f479512e373a18ee95ac8ee7d88c Mon Sep 17 00:00:00 2001
From: Peter Varga <pvarga@inf.u-szeged.hu>
Date: Tue, 8 Sep 2020 14:36:36 +0200
Subject: Avoid to reject a certificate error twice in Quick

Widget API already ignores these cases.
This would cause a crash for fatal certificate errors after 83-based.

Change-Id: I3e80bb12f023f22e330218a5b20f3e267bb237dc
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
 .../api/qquickwebenginecertificateerror.cpp        |  6 ++++
 .../qmltests_ssl/data/tst_certificateError.qml     | 38 +++++++++++++++++-----
 .../certificateerror/tst_certificateerror.cpp      | 17 ++++++++++
 3 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/src/webengine/api/qquickwebenginecertificateerror.cpp b/src/webengine/api/qquickwebenginecertificateerror.cpp
index 63beb1bae..6d50ea851 100644
--- a/src/webengine/api/qquickwebenginecertificateerror.cpp
+++ b/src/webengine/api/qquickwebenginecertificateerror.cpp
@@ -121,6 +121,9 @@ void QQuickWebEngineCertificateError::ignoreCertificateError()
 {
     Q_D(QQuickWebEngineCertificateError);
 
+    if (d->answered)
+        return;
+
     d->answered = true;
 
     QSharedPointer<CertificateErrorController> strongRefCert = d->weakRefCertErrorController.toStrongRef();
@@ -137,6 +140,9 @@ void QQuickWebEngineCertificateError::rejectCertificate()
 {
     Q_D(QQuickWebEngineCertificateError);
 
+    if (d->answered)
+        return;
+
     d->answered = true;
 
     QSharedPointer<CertificateErrorController> strongRefCert = d->weakRefCertErrorController.toStrongRef();
diff --git a/tests/auto/quick/qmltests_ssl/data/tst_certificateError.qml b/tests/auto/quick/qmltests_ssl/data/tst_certificateError.qml
index 0629be175..4c252e210 100644
--- a/tests/auto/quick/qmltests_ssl/data/tst_certificateError.qml
+++ b/tests/auto/quick/qmltests_ssl/data/tst_certificateError.qml
@@ -38,15 +38,6 @@ TestWebEngineView {
     property bool deferError: false
     property bool acceptCertificate: false
 
-    onCertificateError: function(error) {
-        if (deferError)
-            error.defer()
-        else if (acceptCertificate)
-            error.ignoreCertificateError()
-        else
-            error.rejectCertificate()
-    }
-
     SignalSpy {
         id: spyError
         target: view
@@ -88,6 +79,16 @@ TestWebEngineView {
         function test_error(data) {
             view.deferError = data.deferError
             view.acceptCertificate = data.acceptCertificate
+            var handleCertificateError = function(error) {
+                if (deferError)
+                    error.defer()
+                else if (acceptCertificate)
+                    error.ignoreCertificateError()
+                else
+                    error.rejectCertificate()
+            }
+            view.certificateError.connect(handleCertificateError)
+
             view.url = Shared.HttpsServer.url()
 
             if (data.deferError) {
@@ -109,6 +110,25 @@ TestWebEngineView {
 
             compare(spyError.count, 1)
             compare(data.expectedContent, view.getBodyText())
+
+            view.certificateError.disconnect(handleCertificateError)
+        }
+
+        function test_fatalError() {
+            var handleCertificateError = function(error) {
+                verify(!error.overrideable);
+                // QQuickWebEngineViewPrivate::allowCertificateError() will implicitly reject
+                // fatal errors and it should not crash if already rejected in handler.
+                error.rejectCertificate();
+            }
+            view.certificateError.connect(handleCertificateError);
+
+            view.url = Qt.resolvedUrl('https://revoked.badssl.com');
+            if (!view.waitForLoadFailed(10000))
+                skip("Couldn't load page from network, skipping test.");
+            compare(spyError.count, 1);
+
+            view.certificateError.disconnect(handleCertificateError);
         }
     }
 }
diff --git a/tests/auto/widgets/certificateerror/tst_certificateerror.cpp b/tests/auto/widgets/certificateerror/tst_certificateerror.cpp
index f11d9236c..c5b761818 100644
--- a/tests/auto/widgets/certificateerror/tst_certificateerror.cpp
+++ b/tests/auto/widgets/certificateerror/tst_certificateerror.cpp
@@ -43,6 +43,7 @@ public:
 private Q_SLOTS:
     void handleError_data();
     void handleError();
+    void fatalError();
 };
 
 struct PageWithCertificateErrorHandler : QWebEnginePage
@@ -120,5 +121,21 @@ void tst_CertificateError::handleError()
     QCOMPARE(toPlainTextSync(&page), expectedContent);
 }
 
+void tst_CertificateError::fatalError()
+{
+    PageWithCertificateErrorHandler page(false, false);
+    page.settings()->setAttribute(QWebEngineSettings::ErrorPageEnabled, false);
+    QSignalSpy loadFinishedSpy(&page, &QWebEnginePage::loadFinished);
+
+    page.setUrl(QUrl("https://revoked.badssl.com"));
+    if (!loadFinishedSpy.wait(10000))
+        QSKIP("Couldn't load page from network, skipping test.");
+    QTRY_VERIFY(page.error);
+    QVERIFY(!page.error->isOverridable());
+
+    // Fatal certificate errors are implicitly rejected. This should not cause crash.
+    page.error->rejectCertificate();
+}
+
 QTEST_MAIN(tst_CertificateError)
 #include <tst_certificateerror.moc>
-- 
cgit v1.2.3