- Revision
- 230196
- Author
- carlo...@webkit.org
- Date
- 2018-04-03 00:09:34 -0700 (Tue, 03 Apr 2018)
Log Message
[GTK] NetworkProcess from WebKitGtk+ 2.19.9x SIGSEVs in NetworkStorageSession (secret search callback)
https://bugs.webkit.org/show_bug.cgi?id=183346
Reviewed by Michael Catanzaro.
Source/WebCore:
This might happen if a request is cancelled right after the password request starts and before it finishes. We
should cancel the password search when the network request is cancelled, not only when the NetworkStorageSession
is destroyed.
* platform/network/NetworkStorageSession.h:
* platform/network/soup/NetworkStorageSessionSoup.cpp:
(WebCore::NetworkStorageSession::~NetworkStorageSession):
(WebCore::SecretServiceSearchData::SecretServiceSearchData): Helper struct to keep the request cancellable and
completion handler.
(WebCore::NetworkStorageSession::getCredentialFromPersistentStorage): Create a SecretServiceSearchData for the
request.
* platform/network/soup/ResourceHandleSoup.cpp:
(WebCore::ResourceHandle::didReceiveAuthenticationChallenge): Pass the request cancellable to
NetworkStorageSession::getCredentialFromPersistentStorage().
Source/WebKit:
Pass the request cancellable to NetworkStorageSession::getCredentialFromPersistentStorage().
* NetworkProcess/soup/NetworkDataTaskSoup.cpp:
(WebKit::NetworkDataTaskSoup::authenticate):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (230195 => 230196)
--- trunk/Source/WebCore/ChangeLog 2018-04-03 01:03:46 UTC (rev 230195)
+++ trunk/Source/WebCore/ChangeLog 2018-04-03 07:09:34 UTC (rev 230196)
@@ -1,3 +1,25 @@
+2018-04-03 Carlos Garcia Campos <cgar...@igalia.com>
+
+ [GTK] NetworkProcess from WebKitGtk+ 2.19.9x SIGSEVs in NetworkStorageSession (secret search callback)
+ https://bugs.webkit.org/show_bug.cgi?id=183346
+
+ Reviewed by Michael Catanzaro.
+
+ This might happen if a request is cancelled right after the password request starts and before it finishes. We
+ should cancel the password search when the network request is cancelled, not only when the NetworkStorageSession
+ is destroyed.
+
+ * platform/network/NetworkStorageSession.h:
+ * platform/network/soup/NetworkStorageSessionSoup.cpp:
+ (WebCore::NetworkStorageSession::~NetworkStorageSession):
+ (WebCore::SecretServiceSearchData::SecretServiceSearchData): Helper struct to keep the request cancellable and
+ completion handler.
+ (WebCore::NetworkStorageSession::getCredentialFromPersistentStorage): Create a SecretServiceSearchData for the
+ request.
+ * platform/network/soup/ResourceHandleSoup.cpp:
+ (WebCore::ResourceHandle::didReceiveAuthenticationChallenge): Pass the request cancellable to
+ NetworkStorageSession::getCredentialFromPersistentStorage().
+
2018-04-02 Eric Carlson <eric.carl...@apple.com>
[Extra zoom mode] Replace video with a placeholder image during fullscreen transition
Modified: trunk/Source/WebCore/platform/network/NetworkStorageSession.h (230195 => 230196)
--- trunk/Source/WebCore/platform/network/NetworkStorageSession.h 2018-04-03 01:03:46 UTC (rev 230195)
+++ trunk/Source/WebCore/platform/network/NetworkStorageSession.h 2018-04-03 07:09:34 UTC (rev 230196)
@@ -124,7 +124,7 @@
SoupCookieJar* cookieStorage() const;
void setCookieStorage(SoupCookieJar*);
void setCookieObserverHandler(Function<void ()>&&);
- void getCredentialFromPersistentStorage(const ProtectionSpace&, Function<void (Credential&&)> completionHandler);
+ void getCredentialFromPersistentStorage(const ProtectionSpace&, GCancellable*, Function<void (Credential&&)>&& completionHandler);
void saveCredentialToPersistentStorage(const ProtectionSpace&, const Credential&);
#elif USE(CURL)
NetworkStorageSession(PAL::SessionID, NetworkingContext*);
@@ -162,10 +162,6 @@
mutable std::unique_ptr<SoupNetworkSession> m_session;
GRefPtr<SoupCookieJar> m_cookieStorage;
Function<void ()> m_cookieObserverHandler;
-#if USE(LIBSECRET)
- Function<void (Credential&&)> m_persisentStorageCompletionHandler;
- GRefPtr<GCancellable> m_persisentStorageCancellable;
-#endif
#elif USE(CURL)
RefPtr<NetworkingContext> m_context;
Modified: trunk/Source/WebCore/platform/network/soup/NetworkStorageSessionSoup.cpp (230195 => 230196)
--- trunk/Source/WebCore/platform/network/soup/NetworkStorageSessionSoup.cpp 2018-04-03 01:03:46 UTC (rev 230195)
+++ trunk/Source/WebCore/platform/network/soup/NetworkStorageSessionSoup.cpp 2018-04-03 07:09:34 UTC (rev 230196)
@@ -61,10 +61,6 @@
NetworkStorageSession::~NetworkStorageSession()
{
g_signal_handlers_disconnect_matched(m_cookieStorage.get(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
-
-#if USE(LIBSECRET)
- g_cancellable_cancel(m_persisentStorageCancellable.get());
-#endif
}
static std::unique_ptr<NetworkStorageSession>& defaultSession()
@@ -187,9 +183,22 @@
ASSERT_NOT_REACHED();
return "unknown";
}
+
+struct SecretServiceSearchData {
+ SecretServiceSearchData(GCancellable* cancellable, Function<void (Credential&&)>&& completionHandler)
+ : cancellable(cancellable)
+ , completionHandler(WTFMove(completionHandler))
+ {
+ }
+
+ ~SecretServiceSearchData() = default;
+
+ GRefPtr<GCancellable> cancellable;
+ Function<void (Credential&&)> completionHandler;
+};
#endif // USE(LIBSECRET)
-void NetworkStorageSession::getCredentialFromPersistentStorage(const ProtectionSpace& protectionSpace, Function<void (Credential&&)> completionHandler)
+void NetworkStorageSession::getCredentialFromPersistentStorage(const ProtectionSpace& protectionSpace, GCancellable* cancellable, Function<void (Credential&&)>&& completionHandler)
{
#if USE(LIBSECRET)
if (m_sessionID.isEphemeral()) {
@@ -215,28 +224,24 @@
return;
}
- m_persisentStorageCancellable = adoptGRef(g_cancellable_new());
- m_persisentStorageCompletionHandler = WTFMove(completionHandler);
+ auto data = "" WTFMove(completionHandler));
secret_service_search(nullptr, SECRET_SCHEMA_COMPAT_NETWORK, attributes.get(),
- static_cast<SecretSearchFlags>(SECRET_SEARCH_UNLOCK | SECRET_SEARCH_LOAD_SECRETS), m_persisentStorageCancellable.get(),
+ static_cast<SecretSearchFlags>(SECRET_SEARCH_UNLOCK | SECRET_SEARCH_LOAD_SECRETS), cancellable,
[](GObject* source, GAsyncResult* result, gpointer userData) {
+ auto data = ""
GUniqueOutPtr<GError> error;
GUniquePtr<GList> elements(secret_service_search_finish(SECRET_SERVICE(source), result, &error.outPtr()));
- if (g_error_matches (error.get(), G_IO_ERROR, G_IO_ERROR_CANCELLED))
+ if (g_cancellable_is_cancelled(data->cancellable.get()) || error || !elements || !elements->data) {
+ data->completionHandler({ });
return;
-
- NetworkStorageSession* session = static_cast<NetworkStorageSession*>(userData);
- auto completionHandler = std::exchange(session->m_persisentStorageCompletionHandler, nullptr);
- if (error || !elements || !elements->data) {
- completionHandler({ });
- return;
}
- GRefPtr<SecretItem> secretItem = adoptGRef(static_cast<SecretItem*>(elements->data));
+ GRefPtr<SecretItem> secretItem = static_cast<SecretItem*>(elements->data);
+ g_list_foreach(elements.get(), reinterpret_cast<GFunc>(g_object_unref), nullptr);
GRefPtr<GHashTable> attributes = adoptGRef(secret_item_get_attributes(secretItem.get()));
String user = String::fromUTF8(static_cast<const char*>(g_hash_table_lookup(attributes.get(), "user")));
if (user.isEmpty()) {
- completionHandler({ });
+ data->completionHandler({ });
return;
}
@@ -243,8 +248,8 @@
size_t length;
GRefPtr<SecretValue> secretValue = adoptGRef(secret_item_get_secret(secretItem.get()));
const char* passwordData = secret_value_get(secretValue.get(), &length);
- completionHandler(Credential(user, String::fromUTF8(passwordData, length), CredentialPersistencePermanent));
- }, this);
+ data->completionHandler(Credential(user, String::fromUTF8(passwordData, length), CredentialPersistencePermanent));
+ }, data.release());
#else
UNUSED_PARAM(protectionSpace);
completionHandler({ });
Modified: trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp (230195 => 230196)
--- trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp 2018-04-03 01:03:46 UTC (rev 230195)
+++ trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp 2018-04-03 07:09:34 UTC (rev 230196)
@@ -834,7 +834,7 @@
// use HTTP authentication. In the end, this doesn't matter much, because persistent credentials
// will become session credentials after the first use.
if (useCredentialStorage && d->m_context && d->m_context->isValid()) {
- d->m_context->storageSession().getCredentialFromPersistentStorage(challenge.protectionSpace(), [this, protectedThis = makeRef(*this)] (Credential&& credential) {
+ d->m_context->storageSession().getCredentialFromPersistentStorage(challenge.protectionSpace(), d->m_cancellable.get(), [this, protectedThis = makeRef(*this)] (Credential&& credential) {
continueDidReceiveAuthenticationChallenge(WTFMove(credential));
});
return;
Modified: trunk/Source/WebKit/ChangeLog (230195 => 230196)
--- trunk/Source/WebKit/ChangeLog 2018-04-03 01:03:46 UTC (rev 230195)
+++ trunk/Source/WebKit/ChangeLog 2018-04-03 07:09:34 UTC (rev 230196)
@@ -1,3 +1,15 @@
+2018-04-03 Carlos Garcia Campos <cgar...@igalia.com>
+
+ [GTK] NetworkProcess from WebKitGtk+ 2.19.9x SIGSEVs in NetworkStorageSession (secret search callback)
+ https://bugs.webkit.org/show_bug.cgi?id=183346
+
+ Reviewed by Michael Catanzaro.
+
+ Pass the request cancellable to NetworkStorageSession::getCredentialFromPersistentStorage().
+
+ * NetworkProcess/soup/NetworkDataTaskSoup.cpp:
+ (WebKit::NetworkDataTaskSoup::authenticate):
+
2018-04-02 Brady Eidson <beid...@apple.com>
Process swapping on navigation needs to handle server redirects.
Modified: trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp (230195 => 230196)
--- trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp 2018-04-03 01:03:46 UTC (rev 230195)
+++ trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp 2018-04-03 07:09:34 UTC (rev 230196)
@@ -502,7 +502,7 @@
// will become session credentials after the first use.
if (m_storedCredentialsPolicy == StoredCredentialsPolicy::Use) {
auto protectionSpace = challenge.protectionSpace();
- m_session->networkStorageSession().getCredentialFromPersistentStorage(protectionSpace,
+ m_session->networkStorageSession().getCredentialFromPersistentStorage(protectionSpace, m_cancellable.get(),
[this, protectedThis = makeRef(*this), authChallenge = WTFMove(challenge)] (Credential&& credential) mutable {
if (m_state == State::Canceling || m_state == State::Completed || !m_client) {
clearRequest();