Changeset 86397 in webkit

Timestamp:

May 12, 2011 4:45:35 PM (13 years ago)

Author:

andersca@apple.com

Message:

2011-05-12 Anders Carlsson <​andersca@apple.com>

Reviewed by Dan Bernstein.

Crash when a plug-in tries to load a resource from the application cache
​https://bugs.webkit.org/show_bug.cgi?id=60735
<rdar://problem/8216142>

When loading application cache resources from disk, we would never restore the
HTTP status code, which means that resources would always have a 0 status code.
This led to the plug-in stream loader canceling the load, something which the application
cache loading machinery couldn't deal and thus would crash.

Fix the reading of the HTTP status code, and make the loader more robust against
application cache loads being canceled.

Unfortunately, I couldn't come up with a test for this because I couldn't find a reliable way
to force the application cache to be read from disk, so all resources would have valid values.

• loader/DocumentLoader.cpp: (WebCore::DocumentLoader::substituteResourceDeliveryTimerFired):
• loader/appcache/ApplicationCacheStorage.cpp: (WebCore::ApplicationCacheStorage::loadCache):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/DocumentLoader.cpp (modified) (1 diff)
• loader/appcache/ApplicationCacheStorage.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-05-12  Anders Carlsson  <andersca@apple.com>
+
+        Reviewed by Dan Bernstein.
+
+        Crash when a plug-in tries to load a resource from the application cache
+        https://bugs.webkit.org/show_bug.cgi?id=60735
+        <rdar://problem/8216142>
+
+        When loading application cache resources from disk, we would never restore the
+        HTTP status code, which means that resources would always have a 0 status code.
+        This led to the plug-in stream loader canceling the load, something which the application
+        cache loading machinery couldn't deal and thus would crash.
+
+        Fix the reading of the HTTP status code, and make the loader more robust against
+        application cache loads being canceled.
+
+        Unfortunately, I couldn't come up with a test for this because I couldn't find a reliable way
+        to force the application cache to be read from disk, so all resources would have valid values.
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::substituteResourceDeliveryTimerFired):
+        * loader/appcache/ApplicationCacheStorage.cpp:
+        (WebCore::ApplicationCacheStorage::loadCache):
+
 2011-05-12  Emil A Eklund  <eae@chromium.org>
 

trunk/Source/WebCore/loader/DocumentLoader.cpp

@@ -590 +590 @@
         
             loader->didReceiveResponse(resource->response());
+
+            // Calling ResourceLoader::didReceiveResponse can end up cancelling the load,
+            // so we need to check if the loader has reached its terminal state.
+            if (loader->reachedTerminalState())
+                return;
+
+            // Calling ResourceLoader::didReceiveData can end up cancelling the load,
+            // so we need to check if the loader has reached its terminal state.
             loader->didReceiveData(data->data(), data->size(), data->size(), true);
+            if (loader->reachedTerminalState())
+                return;
+
             loader->didFinishLoading(0);
         } else {

trunk/Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp

@@ -1056 +1056 @@
 {
     SQLiteStatement cacheStatement(m_database, 
-                                   "SELECT url, type, mimeType, textEncodingName, headers, CacheResourceData.data, CacheResourceData.path FROM CacheEntries INNER JOIN CacheResources ON CacheEntries.resource=CacheResources.id "
+                                   "SELECT url, statusCode, type, mimeType, textEncodingName, headers, CacheResourceData.data, CacheResourceData.path FROM CacheEntries INNER JOIN CacheResources ON CacheEntries.resource=CacheResources.id "
                                    "INNER JOIN CacheResourceData ON CacheResourceData.id=CacheResources.data WHERE CacheEntries.cache=?");
     if (cacheStatement.prepare() != SQLResultOk) {
@@ -1073 +1073 @@
         KURL url(ParsedURLString, cacheStatement.getColumnText(0));
         
-        unsigned type = static_cast<unsigned>(cacheStatement.getColumnInt64(1));
+        int httpStatusCode = cacheStatement.getColumnInt(1);
+
+        unsigned type = static_cast<unsigned>(cacheStatement.getColumnInt64(2));
 
         Vector<char> blob;
-        cacheStatement.getColumnBlobAsVector(5, blob);
+        cacheStatement.getColumnBlobAsVector(6, blob);
         
         RefPtr<SharedBuffer> data = SharedBuffer::adoptVector(blob);
         
-        String path = cacheStatement.getColumnText(6);
+        String path = cacheStatement.getColumnText(7);
         long long size = 0;
         if (path.isEmpty())
@@ -1089 +1091 @@
         }
         
-        String mimeType = cacheStatement.getColumnText(2);
-        String textEncodingName = cacheStatement.getColumnText(3);
+        String mimeType = cacheStatement.getColumnText(3);
+        String textEncodingName = cacheStatement.getColumnText(4);
         
         ResourceResponse response(url, mimeType, size, textEncodingName, "");
-
-        String headers = cacheStatement.getColumnText(4);
+        response.setHTTPStatusCode(httpStatusCode);
+
+        String headers = cacheStatement.getColumnText(5);
         parseHeaders(headers, response);