Changeset 130348 in webkit

Timestamp:

Oct 3, 2012 4:58:33 PM (12 years ago)

Author:

Martin Robinson

Message:

[soup] WebKit crashes when doing a http request
​https://bugs.webkit.org/show_bug.cgi?id=98055

Patch by Arnaud Renevier <​a.renevier@sisa.samsung.com> on 2012-10-03
Reviewed by Martin Robinson.

On i386, (d->m_firstRequest.timeoutInterval() * 1000) results in 0 if
timeoutInterval() is INT_MAX. So, set default timeout to 0 to avoid
calling soup_add_timeout with a 0 value.

Also, if resource handle is deleted before "request-started" signal is
emitted, soupMessage handle points to a deleted object, and a crash
occurs. So, reset soupMessage handle data in
cleanupSoupRequestOperation so it won't happen anymore.

Lastly, if timeout occurs before request is completed, handle is
deleted, and crash occurs in sendRequestCallback due to an early
destroyed handle. To avoid that, call handle->cancel in
requestTimeoutCallback. There is no need to call
cleanupSoupRequestOperation anymore since handle->cancel will trigger
sendRequestCallback, and as handle is deleted,
cleanupSoupRequestOperation will be called automatically.

No new tests yet, tests will be added with the patch in bug 74802.

• platform/network/ResourceRequestBase.cpp:

(WebCore):

• platform/network/soup/ResourceHandleSoup.cpp:

(WebCore::cleanupSoupRequestOperation):
(WebCore::ResourceHandle::platformSetDefersLoading):
(WebCore::requestTimeoutCallback):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/network/ResourceRequestBase.cpp (modified) (1 diff)
• platform/network/soup/ResourceHandleSoup.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-10-03  Arnaud Renevier  <a.renevier@sisa.samsung.com>
+
+        [soup] WebKit crashes when doing a http request
+        https://bugs.webkit.org/show_bug.cgi?id=98055
+
+        Reviewed by Martin Robinson.
+
+        On i386, (d->m_firstRequest.timeoutInterval() * 1000) results in 0 if
+        timeoutInterval() is INT_MAX. So, set default timeout to 0 to avoid
+        calling soup_add_timeout with a 0 value.
+
+        Also, if resource handle is deleted before "request-started" signal is
+        emitted, soupMessage handle points to a deleted object, and a crash
+        occurs. So, reset soupMessage handle data in
+        cleanupSoupRequestOperation so it won't happen anymore.
+
+        Lastly, if timeout occurs before request is completed, handle is
+        deleted, and crash occurs in sendRequestCallback due to an early
+        destroyed handle. To avoid that, call handle->cancel in
+        requestTimeoutCallback. There is no need to call
+        cleanupSoupRequestOperation anymore since handle->cancel will trigger
+        sendRequestCallback, and as handle is deleted,
+        cleanupSoupRequestOperation will be called automatically.
+
+        No new tests yet, tests will be added with the patch in bug 74802.
+
+        * platform/network/ResourceRequestBase.cpp:
+        (WebCore):
+        * platform/network/soup/ResourceHandleSoup.cpp:
+        (WebCore::cleanupSoupRequestOperation):
+        (WebCore::ResourceHandle::platformSetDefersLoading):
+        (WebCore::requestTimeoutCallback):
+
 2012-10-03  Adam Barth  <abarth@webkit.org>
 

trunk/Source/WebCore/platform/network/ResourceRequestBase.cpp

@@ -36 +36 @@
 namespace WebCore {
 
-#if !PLATFORM(MAC) || USE(CFNETWORK)
+#if !USE(SOUP) && (!PLATFORM(MAC) || USE(CFNETWORK))
 double ResourceRequestBase::s_defaultTimeoutInterval = INT_MAX;
 #else
 // Will use NSURLRequest default timeout unless set to a non-zero value with setDefaultTimeoutInterval().
+// For libsoup the timeout enabled with integer milliseconds. We set 0 as the default value to avoid integer overflow.
 double ResourceRequestBase::s_defaultTimeoutInterval = 0;
 #endif

trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp

@@ -382 +382 @@
         g_signal_handlers_disconnect_matched(d->m_soupMessage.get(), G_SIGNAL_MATCH_DATA,
                                              0, 0, 0, 0, handle);
+        g_object_set_data(G_OBJECT(d->m_soupMessage.get()), "handle", 0);
         d->m_soupMessage.clear();
     }
@@ -875 +876 @@
     // Except when canceling a possible timeout timer, we only need to take action here to UN-defer loading.
     if (defersLoading) {
-        g_source_destroy(d->m_timeoutSource.get());
-        d->m_timeoutSource.clear();
+        if (d->m_timeoutSource) {
+            g_source_destroy(d->m_timeoutSource.get());
+            d->m_timeoutSource.clear();
+        }
         return;
     }
@@ -1023 +1026 @@
     timeoutError.setIsTimeout(true);
     client->didFail(handle.get(), timeoutError);
-    cleanupSoupRequestOperation(handle.get());
+    handle->cancel();
 
     return FALSE;