From 3acfd3b1cd39e89fb6bc2f0da3412f07d68de7b6 Mon Sep 17 00:00:00 2001 From: Kirill Burtsev Date: Thu, 20 May 2021 17:16:00 +0200 Subject: Ensure certificate error callback call for all types Amends a2a9ea11f9. Actually reject certificate when it's considered to be fatal. Adapt tests for possible regression but checking if load really failed (due to missing internet access) or was just halted by missing callback call internaly. Pick-to: 6.2 Change-Id: I656525c353ce410f7bda8c55227a29fcd617bfdd Reviewed-by: Michal Klocek Reviewed-by: Qt CI Bot --- .../core/certificateerror/tst_certificateerror.cpp | 16 ++++++++++----- .../auto/quick/qmltests/data/TestWebEngineView.qml | 16 ++++++++++++--- .../quick/qmltests/data/tst_certificateError.qml | 23 +++++++++++++-------- .../qmltests/data/tst_navigationRequested.qml | 24 +++++++++++----------- .../quick/qmltests/data/tst_newViewRequest.qml | 6 +++--- 5 files changed, 53 insertions(+), 32 deletions(-) (limited to 'tests/auto') diff --git a/tests/auto/core/certificateerror/tst_certificateerror.cpp b/tests/auto/core/certificateerror/tst_certificateerror.cpp index da679fa8a..3c43d997f 100644 --- a/tests/auto/core/certificateerror/tst_certificateerror.cpp +++ b/tests/auto/core/certificateerror/tst_certificateerror.cpp @@ -139,13 +139,19 @@ void tst_CertificateError::fatalError() QSignalSpy loadFinishedSpy(&page, &QWebEnginePage::loadFinished); page.setUrl(QUrl("https://revoked.badssl.com")); - if (!loadFinishedSpy.wait(10000)) + if (!loadFinishedSpy.wait(10000)) { + QVERIFY2(!page.error, "There shouldn't be any certificate error if not loaded due to missing internet access!"); 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(); + // revoked certificate might not be reported as invalid by chromium and the load will silently succeed + bool failed = !loadFinishedSpy.first().first().toBool(), hasError = bool(page.error); + QCOMPARE(failed, hasError); + if (hasError) { + QVERIFY(!page.error->isOverridable()); + // Fatal certificate errors are implicitly rejected. But second call should not cause crash. + page.error->rejectCertificate(); + } } QTEST_MAIN(tst_CertificateError) diff --git a/tests/auto/quick/qmltests/data/TestWebEngineView.qml b/tests/auto/quick/qmltests/data/TestWebEngineView.qml index aa9d67d99..68417c6c5 100644 --- a/tests/auto/quick/qmltests/data/TestWebEngineView.qml +++ b/tests/auto/quick/qmltests/data/TestWebEngineView.qml @@ -35,19 +35,29 @@ WebEngineView { property bool windowCloseRequestedSignalEmitted: false settings.focusOnNavigationEnabled: true + function loadSucceeded() { return loadStatus == WebEngineView.LoadSucceededStatus } + function loadFailed() { return loadStatus == WebEngineView.LoadFailedStatus } + function loadStopped() { return loadStatus == WebEngineView.LoadStoppedStatus } + + function waitForLoadResult(timeout) { + loadStatus = null + var r = _waitFor(function() { return loadStatus != null && loadStatus != WebEngineView.LoadStartedStatus }, timeout) + return r + } + function waitForLoadSucceeded(timeout) { - var success = _waitFor(function() { return loadStatus == WebEngineView.LoadSucceededStatus }, timeout) loadStatus = null + var success = _waitFor(function() { return loadStatus == WebEngineView.LoadSucceededStatus }, timeout) return success } function waitForLoadFailed(timeout) { - var failure = _waitFor(function() { return loadStatus == WebEngineView.LoadFailedStatus }, timeout) loadStatus = null + var failure = _waitFor(function() { return loadStatus == WebEngineView.LoadFailedStatus }, timeout) return failure } function waitForLoadStopped(timeout) { - var stop = _waitFor(function() { return loadStatus == WebEngineView.LoadStoppedStatus }, timeout) loadStatus = null + var stop = _waitFor(function() { return loadStatus == WebEngineView.LoadStoppedStatus }, timeout) return stop } function waitForWindowCloseRequested() { diff --git a/tests/auto/quick/qmltests/data/tst_certificateError.qml b/tests/auto/quick/qmltests/data/tst_certificateError.qml index bad24bf0f..b254bb621 100644 --- a/tests/auto/quick/qmltests/data/tst_certificateError.qml +++ b/tests/auto/quick/qmltests/data/tst_certificateError.qml @@ -123,20 +123,25 @@ TestWebEngineView { } 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(); - } + let error = undefined + var handleCertificateError = function(e) { error = e; } view.certificateError.connect(handleCertificateError); view.url = Qt.resolvedUrl('https://revoked.badssl.com'); - if (!view.waitForLoadFailed(10000)) + if (!view.waitForLoadResult()) { + verify(!error, "There shouldn't be any certificate error if not loaded due to missing internet access!"); skip("Couldn't load page from network, skipping test."); - compare(spyError.count, 1); - + } view.certificateError.disconnect(handleCertificateError); + + // revoked certificate might not be reported as invalid by chromium and the load will silently succeed + const failed = view.loadStatus == WebEngineView.LoadFailedStatus, hasError = Boolean(error) + compare(hasError, failed) + if (failed) { + verify(!error.overrideable); + // Fatal certificate errors are implicitly rejected. But second call should not cause crash. + error.rejectCertificate(); + } } } } diff --git a/tests/auto/quick/qmltests/data/tst_navigationRequested.qml b/tests/auto/quick/qmltests/data/tst_navigationRequested.qml index 462dc8297..11ea69e11 100644 --- a/tests/auto/quick/qmltests/data/tst_navigationRequested.qml +++ b/tests/auto/quick/qmltests/data/tst_navigationRequested.qml @@ -94,19 +94,17 @@ TestWebEngineView { // Test if we get notified about main frame and iframe loads compare(navigationSpy.count, 0) webEngineView.url = Qt.resolvedUrl("test-iframe.html") - navigationSpy.wait() + verify(webEngineView.waitForLoadSucceeded()) compare(attributes.mainUrl, Qt.resolvedUrl("test-iframe.html")) - navigationSpy.wait() compare(attributes.iframeUrl, Qt.resolvedUrl("test1.html")) compare(navigationSpy.count, 2) - verify(webEngineView.waitForLoadSucceeded()) // Test if we get notified about clicked links mouseClick(webEngineView, 100, 100) - tryCompare(navigationSpy, "count", 3) + verify(webEngineView.waitForLoadSucceeded()) compare(attributes.mainUrl, Qt.resolvedUrl("test1.html")) verify(attributes.linkClickedNavigationRequested) - verify(webEngineView.waitForLoadSucceeded()) + compare(navigationSpy.count, 3) } function test_ignoreLinkClickedRequest() { @@ -117,26 +115,28 @@ TestWebEngineView { shouldIgnoreLinkClicks = true mouseClick(webEngineView, 100, 100) - tryCompare(navigationSpy, "count", 3) - compare(attributes.mainUrl, Qt.resolvedUrl("test1.html")) - verify(attributes.linkClickedNavigationRequested) - verify(attributes.linkClickedNavigationIgnored) // We ignored the main frame request, so we should // get notified that the load has been stopped. verify(webEngineView.waitForLoadStopped()) verify(!webEngineView.loading) + + compare(navigationSpy.count, 3) + compare(attributes.mainUrl, Qt.resolvedUrl("test1.html")) + verify(attributes.linkClickedNavigationRequested) + verify(attributes.linkClickedNavigationIgnored) } function test_ignoreSubFrameRequest() { // Test if we can ignore sub frame requests shouldIgnoreSubFrameRequests = true webEngineView.url = Qt.resolvedUrl("test-iframe.html") - tryCompare(navigationSpy, "count", 2) - compare(attributes.mainUrl, Qt.resolvedUrl("test-iframe.html")) - compare(attributes.iframeUrl, Qt.resolvedUrl("test1.html")) // We ignored the sub frame request, so // the main frame load should still succeed. verify(webEngineView.waitForLoadSucceeded()) + + compare(navigationSpy.count, 2) + compare(attributes.mainUrl, Qt.resolvedUrl("test-iframe.html")) + compare(attributes.iframeUrl, Qt.resolvedUrl("test1.html")) } } } diff --git a/tests/auto/quick/qmltests/data/tst_newViewRequest.qml b/tests/auto/quick/qmltests/data/tst_newViewRequest.qml index ac402674d..fb99e9cee 100644 --- a/tests/auto/quick/qmltests/data/tst_newViewRequest.qml +++ b/tests/auto/quick/qmltests/data/tst_newViewRequest.qml @@ -122,7 +122,7 @@ TestWebEngineView { verify(!newViewRequest.userInitiated); if (viewType === "dialog") { - verify(dialog.webEngineView.waitForLoadSucceeded()); + tryVerify(dialog.webEngineView.loadSucceeded) compare(dialog.webEngineView.url, ""); dialog.destroy(); } @@ -143,7 +143,7 @@ TestWebEngineView { compare(newViewRequest.requestedUrl, url); verify(!newViewRequest.userInitiated); if (viewType === "dialog") { - verify(dialog.webEngineView.waitForLoadSucceeded()); + tryVerify(dialog.webEngineView.loadSucceeded) dialog.destroy(); } newViewRequestedSpy.clear(); @@ -166,7 +166,7 @@ TestWebEngineView { compare(newViewRequest.destination, WebEngineNewViewRequest.InNewDialog); verify(newViewRequest.userInitiated); if (viewType === "dialog") { - verify(dialog.webEngineView.waitForLoadSucceeded()); + tryVerify(dialog.webEngineView.loadSucceeded) dialog.destroy(); } newViewRequestedSpy.clear(); -- cgit v1.2.3