From 7537526093c92e89672d1e952a9baceecaa91730 Mon Sep 17 00:00:00 2001 From: Kirill Burtsev Date: Mon, 11 Feb 2019 19:21:03 +0100 Subject: Remove download properly on profile destruction to avoid use after free MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the Widgets API, download items are children of the profile and are destroyed when the parent profile destroys its children. The download item's destructor can therefore not access the profile, as it would cause a heap-use-after-free crashes. On quick side turn ongoing downloads cleanup to match widgets one. Fixes: QTBUG-73839 Change-Id: Iabb379e91187e3e68ebcd4693fec35883b72b1f2 Reviewed-by: Michael Brüning Reviewed-by: Jüri Valdmann --- src/webengine/api/qquickwebengineprofile.cpp | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) (limited to 'src/webengine/api/qquickwebengineprofile.cpp') diff --git a/src/webengine/api/qquickwebengineprofile.cpp b/src/webengine/api/qquickwebengineprofile.cpp index ddc71602b..26fcf28f7 100644 --- a/src/webengine/api/qquickwebengineprofile.cpp +++ b/src/webengine/api/qquickwebengineprofile.cpp @@ -175,13 +175,6 @@ QQuickWebEngineProfilePrivate::~QQuickWebEngineProfilePrivate() m_profileAdapter->removeClient(this); } - for (QQuickWebEngineDownloadItem *download : qAsConst(m_ongoingDownloads)) { - if (download) - download->cancel(); - } - - m_ongoingDownloads.clear(); - if (m_profileAdapter != QtWebEngineCore::ProfileAdapter::defaultProfileAdapter()) delete m_profileAdapter; } @@ -215,6 +208,23 @@ void QQuickWebEngineProfilePrivate::cancelDownload(quint32 downloadId) void QQuickWebEngineProfilePrivate::downloadDestroyed(quint32 downloadId) { m_ongoingDownloads.remove(downloadId); + if (m_profileAdapter) + m_profileAdapter->removeDownload(downloadId); +} + +void QQuickWebEngineProfilePrivate::cleanDownloads() +{ + for (auto download : m_ongoingDownloads.values()) { + if (!download) + continue; + + if (!download->isFinished()) + download->cancel(); + + if (m_profileAdapter) + m_profileAdapter->removeDownload(download->id()); + } + m_ongoingDownloads.clear(); } void QQuickWebEngineProfilePrivate::downloadRequested(DownloadItemInfo &info) @@ -239,6 +249,7 @@ void QQuickWebEngineProfilePrivate::downloadRequested(DownloadItemInfo &info) QQuickWebEngineDownloadItem *download = new QQuickWebEngineDownloadItem(itemPrivate, q); m_ongoingDownloads.insert(info.id, download); + QObject::connect(download, &QQuickWebEngineDownloadItem::destroyed, q, [id = info.id, this] () { downloadDestroyed(id); }); QQmlEngine::setObjectOwnership(download, QQmlEngine::JavaScriptOwnership); Q_EMIT q->downloadRequested(download); @@ -252,7 +263,6 @@ void QQuickWebEngineProfilePrivate::downloadRequested(DownloadItemInfo &info) if (state == QQuickWebEngineDownloadItem::DownloadRequested) { // Delete unaccepted downloads. info.accepted = false; - m_ongoingDownloads.remove(info.id); delete download; } } @@ -275,7 +285,6 @@ void QQuickWebEngineProfilePrivate::downloadUpdated(const DownloadItemInfo &info if (info.state != ProfileAdapterClient::DownloadInProgress) { Q_EMIT q->downloadFinished(download); - m_ongoingDownloads.remove(info.id); } } @@ -380,6 +389,7 @@ QQuickWebEngineProfile::QQuickWebEngineProfile(QQuickWebEngineProfilePrivate *pr */ QQuickWebEngineProfile::~QQuickWebEngineProfile() { + d_ptr->cleanDownloads(); } /*! -- cgit v1.2.1 From 9d362e9d5fabf243c7a326073dd376ffa7aab3f5 Mon Sep 17 00:00:00 2001 From: Kai Koehne Date: Tue, 19 Feb 2019 15:33:40 +0100 Subject: Revision WebEngineProfile::userNotification Change-Id: I6a5ff72c91cb1b173ca140efe3d4c95036f945eb Reviewed-by: Allan Sandfeld Jensen --- src/webengine/api/qquickwebengineprofile.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'src/webengine/api/qquickwebengineprofile.cpp') diff --git a/src/webengine/api/qquickwebengineprofile.cpp b/src/webengine/api/qquickwebengineprofile.cpp index cf9dae90a..edd460f3a 100644 --- a/src/webengine/api/qquickwebengineprofile.cpp +++ b/src/webengine/api/qquickwebengineprofile.cpp @@ -382,6 +382,7 @@ void QQuickWebEngineProfilePrivate::userScripts_clear(QQmlListProperty Date: Thu, 21 Feb 2019 17:01:03 +0100 Subject: Doc: Always treat \brief as full sentence Make sure all \brief descriptions start with an upper-case letter and end with a . Also start descriptions of \class with the name of the class or struct. Change-Id: Ifd2656201f9c1dff092085508a5423ce516e2d3f Reviewed-by: Leena Miettinen --- src/webengine/api/qquickwebengineprofile.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/webengine/api/qquickwebengineprofile.cpp') diff --git a/src/webengine/api/qquickwebengineprofile.cpp b/src/webengine/api/qquickwebengineprofile.cpp index 26fcf28f7..b7abb13fc 100644 --- a/src/webengine/api/qquickwebengineprofile.cpp +++ b/src/webengine/api/qquickwebengineprofile.cpp @@ -958,7 +958,7 @@ QQuickWebEngineSettings *QQuickWebEngineProfile::settings() const \property QQuickWebEngineProfile::userScripts \since 5.9 - \brief the collection of scripts that are injected into all pages that share + \brief The collection of scripts that are injected into all pages that share this profile. \sa QQuickWebEngineScript, QQmlListReference -- cgit v1.2.1 From 678cb710bb07188454b19a70b5e5595b8ea41c2a Mon Sep 17 00:00:00 2001 From: Kai Koehne Date: Fri, 22 Feb 2019 12:24:05 +0100 Subject: Doc: Talk about 'empty string' in WebEngineProfile::downloadPath This is clearer than 'null string', and actually also what the code checks for. Change-Id: I856de48016b609cb7a8be1286f8be51ad765abd6 Reviewed-by: Leena Miettinen --- src/webengine/api/qquickwebengineprofile.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/webengine/api/qquickwebengineprofile.cpp') diff --git a/src/webengine/api/qquickwebengineprofile.cpp b/src/webengine/api/qquickwebengineprofile.cpp index edd460f3a..061e210fb 100644 --- a/src/webengine/api/qquickwebengineprofile.cpp +++ b/src/webengine/api/qquickwebengineprofile.cpp @@ -881,7 +881,7 @@ bool QQuickWebEngineProfile::isUsedForGlobalCertificateVerification() const Overrides the default path used for download location. - If set to the null string, the default path is restored. + If set to an empty string, the default path is restored. \note By default, the download path is QStandardPaths::DownloadLocation. */ @@ -894,7 +894,7 @@ bool QQuickWebEngineProfile::isUsedForGlobalCertificateVerification() const Overrides the default path used for download location, setting it to \a path. - If set to the null string, the default path is restored. + If set to an empty string, the default path is restored. \note By default, the download path is QStandardPaths::DownloadLocation. */ -- cgit v1.2.1 From 56fadb571f32b721d8b99554e6e38692009ec37f Mon Sep 17 00:00:00 2001 From: Michal Klocek Date: Thu, 28 Feb 2019 10:49:22 +0100 Subject: Force destruction of webcontent client before profile adapter Currently users might forget to delete webcontent client before profile adapter. This might be nasty if users are not aware of default profile. Instead of asserting badly in chromium, clean up and release chromium resources. This avoids the crash, but might leak memory if users never deletes page. Task-number: QTBUG-74021 Change-Id: I66f466f169d12f7ee08866d505260dca47800bb0 Reviewed-by: Allan Sandfeld Jensen --- src/webengine/api/qquickwebengineprofile.cpp | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) (limited to 'src/webengine/api/qquickwebengineprofile.cpp') diff --git a/src/webengine/api/qquickwebengineprofile.cpp b/src/webengine/api/qquickwebengineprofile.cpp index b7abb13fc..73577a04c 100644 --- a/src/webengine/api/qquickwebengineprofile.cpp +++ b/src/webengine/api/qquickwebengineprofile.cpp @@ -163,11 +163,6 @@ QQuickWebEngineProfilePrivate::QQuickWebEngineProfilePrivate(ProfileAdapter *pro QQuickWebEngineProfilePrivate::~QQuickWebEngineProfilePrivate() { - - while (!m_webContentsAdapterClients.isEmpty()) { - m_webContentsAdapterClients.first()->destroy(); - } - if (m_profileAdapter) { // In the case the user sets this profile as the parent of the interceptor // it can be deleted before the browser-context still referencing it is. @@ -179,14 +174,16 @@ QQuickWebEngineProfilePrivate::~QQuickWebEngineProfilePrivate() delete m_profileAdapter; } -void QQuickWebEngineProfilePrivate::addWebContentsAdapterClient(QQuickWebEngineViewPrivate *adapter) +void QQuickWebEngineProfilePrivate::addWebContentsAdapterClient(QtWebEngineCore::WebContentsAdapterClient *adapter) { - m_webContentsAdapterClients.append(adapter); + Q_ASSERT(m_profileAdapter); + m_profileAdapter->addWebContentsAdapterClient(adapter); } -void QQuickWebEngineProfilePrivate::removeWebContentsAdapterClient(QQuickWebEngineViewPrivate*adapter) +void QQuickWebEngineProfilePrivate::removeWebContentsAdapterClient(QtWebEngineCore::WebContentsAdapterClient*adapter) { - m_webContentsAdapterClients.removeAll(adapter); + Q_ASSERT(m_profileAdapter); + m_profileAdapter->removeWebContentsAdapterClient(adapter); } QtWebEngineCore::ProfileAdapter *QQuickWebEngineProfilePrivate::profileAdapter() const -- cgit v1.2.1 From 4dc312011bcaa2ee2cf812b5b84dc9238130e608 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Wed, 27 Feb 2019 13:49:30 +0100 Subject: Tie client certificate stores to profiles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the client certificate store from being global to being tied to individual profiles. Change-Id: Ib21ae14c501b7d0612b84ae7535120291aeeada2 Reviewed-by: Jüri Valdmann --- src/webengine/api/qquickwebengineprofile.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'src/webengine/api/qquickwebengineprofile.cpp') diff --git a/src/webengine/api/qquickwebengineprofile.cpp b/src/webengine/api/qquickwebengineprofile.cpp index 061e210fb..0ec4b19ce 100644 --- a/src/webengine/api/qquickwebengineprofile.cpp +++ b/src/webengine/api/qquickwebengineprofile.cpp @@ -1072,4 +1072,15 @@ QQmlListProperty QQuickWebEngineProfile::userScripts() d->userScripts_clear); } +/*! + \since 5.13 + + Returns the profile's client certificate store. +*/ +QWebEngineClientCertificateStore *QQuickWebEngineProfile::clientCertificateStore() +{ + Q_D(QQuickWebEngineProfile); + return d->profileAdapter()->clientCertificateStore(); +} + QT_END_NAMESPACE -- cgit v1.2.1 From dc6c8d5c9fe61ba58d065564f4d6dc0571475004 Mon Sep 17 00:00:00 2001 From: Peter Varga Date: Wed, 6 Mar 2019 13:56:01 +0100 Subject: Fix -no-ssl build Change-Id: I978f70545484060218f5243c74978c85bc603c16 Reviewed-by: Allan Sandfeld Jensen --- src/webengine/api/qquickwebengineprofile.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/webengine/api/qquickwebengineprofile.cpp') diff --git a/src/webengine/api/qquickwebengineprofile.cpp b/src/webengine/api/qquickwebengineprofile.cpp index 3c4ec0595..ac75b5356 100644 --- a/src/webengine/api/qquickwebengineprofile.cpp +++ b/src/webengine/api/qquickwebengineprofile.cpp @@ -1086,8 +1086,12 @@ QQmlListProperty QQuickWebEngineProfile::userScripts() */ QWebEngineClientCertificateStore *QQuickWebEngineProfile::clientCertificateStore() { +#if QT_CONFIG(ssl) Q_D(QQuickWebEngineProfile); return d->profileAdapter()->clientCertificateStore(); +#else + return nullptr; +#endif } QT_END_NAMESPACE -- cgit v1.2.1