Title: [230425] releases/WebKitGTK/webkit-2.20/Source
Revision
230425
Author
carlo...@webkit.org
Date
2018-04-09 06:56:10 -0700 (Mon, 09 Apr 2018)

Log Message

Merge r230196 - [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: releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog (230424 => 230425)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog	2018-04-09 13:33:18 UTC (rev 230424)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog	2018-04-09 13:56:10 UTC (rev 230425)
@@ -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-03-27  Fujii Hironori  <hironori.fu...@sony.com>
 
         [GTK] Layout test editing/deleting/delete-surrogatepair.html crashing with CRITICAL **: enchant_dict_check: assertion 'g_utf8_validate(word, len, NULL)' failed

Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/network/NetworkStorageSession.h (230424 => 230425)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/network/NetworkStorageSession.h	2018-04-09 13:33:18 UTC (rev 230424)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/network/NetworkStorageSession.h	2018-04-09 13:56:10 UTC (rev 230425)
@@ -121,7 +121,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*);
@@ -158,10 +158,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)
     UniqueRef<CookieJarCurl> m_cookieStorage;
     mutable CookieJarDB m_cookieDatabase;

Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/network/soup/NetworkStorageSessionSoup.cpp (230424 => 230425)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/network/soup/NetworkStorageSessionSoup.cpp	2018-04-09 13:33:18 UTC (rev 230424)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/network/soup/NetworkStorageSessionSoup.cpp	2018-04-09 13:56:10 UTC (rev 230425)
@@ -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: releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp (230424 => 230425)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp	2018-04-09 13:33:18 UTC (rev 230424)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp	2018-04-09 13:56:10 UTC (rev 230425)
@@ -835,7 +835,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: releases/WebKitGTK/webkit-2.20/Source/WebKit/ChangeLog (230424 => 230425)


--- releases/WebKitGTK/webkit-2.20/Source/WebKit/ChangeLog	2018-04-09 13:33:18 UTC (rev 230424)
+++ releases/WebKitGTK/webkit-2.20/Source/WebKit/ChangeLog	2018-04-09 13:56:10 UTC (rev 230425)
@@ -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  Michael Catanzaro  <mcatanz...@igalia.com>
 
         [GTK] DragAndDropHandler.cpp should include GUniquePtrGtk.h

Modified: releases/WebKitGTK/webkit-2.20/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp (230424 => 230425)


--- releases/WebKitGTK/webkit-2.20/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp	2018-04-09 13:33:18 UTC (rev 230424)
+++ releases/WebKitGTK/webkit-2.20/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp	2018-04-09 13:56:10 UTC (rev 230425)
@@ -499,7 +499,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();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to