Crash under ApplicationCacheGroup::didFailLoadingEntry()
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 13 Jul 2018 22:54:16 +0000 (22:54 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 13 Jul 2018 22:54:16 +0000 (22:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187661
<rdar://problem/42179755>

Reviewed by Youenn Fablet.

If ApplicationCacheResourceLoader::create() fails synchronously with
ApplicationCacheResourceLoader::Error::CannotCreateResource error, then
m_entryLoader will be null when didFailLoadingEntry() is called. However,
didFailLoadingEntry() fails to null check m_entryLoader before using it.

* loader/appcache/ApplicationCacheGroup.cpp:
(WebCore::ApplicationCacheGroup::didFailLoadingEntry):
(WebCore::ApplicationCacheGroup::startLoadingEntry):
* loader/appcache/ApplicationCacheGroup.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233823 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp
Source/WebCore/loader/appcache/ApplicationCacheGroup.h

index b35181d..fc5a992 100644 (file)
@@ -1,3 +1,21 @@
+2018-07-13  Chris Dumez  <cdumez@apple.com>
+
+        Crash under ApplicationCacheGroup::didFailLoadingEntry()
+        https://bugs.webkit.org/show_bug.cgi?id=187661
+        <rdar://problem/42179755>
+
+        Reviewed by Youenn Fablet.
+
+        If ApplicationCacheResourceLoader::create() fails synchronously with
+        ApplicationCacheResourceLoader::Error::CannotCreateResource error, then
+        m_entryLoader will be null when didFailLoadingEntry() is called. However,
+        didFailLoadingEntry() fails to null check m_entryLoader before using it.
+
+        * loader/appcache/ApplicationCacheGroup.cpp:
+        (WebCore::ApplicationCacheGroup::didFailLoadingEntry):
+        (WebCore::ApplicationCacheGroup::startLoadingEntry):
+        * loader/appcache/ApplicationCacheGroup.h:
+
 2018-07-13  Alex Christensen  <achristensen@webkit.org>
 
         Add release assertion to check thread in TimerBase::setNextFireTime
index 06ed8ef..f1380c4 100644 (file)
@@ -538,14 +538,13 @@ void ApplicationCacheGroup::didFinishLoadingEntry(const URL& entryURL)
     startLoadingEntry();
 }
 
-void ApplicationCacheGroup::didFailLoadingEntry(ApplicationCacheResourceLoader::Error error, const URL& entryURL)
+void ApplicationCacheGroup::didFailLoadingEntry(ApplicationCacheResourceLoader::Error error, const URL& entryURL, unsigned type)
 {
     // FIXME: We should get back the error from ApplicationCacheResourceLoader level.
     ResourceError resourceError { error == ApplicationCacheResourceLoader::Error::CannotCreateResource ? ResourceError::Type::AccessControl : ResourceError::Type::General };
 
     InspectorInstrumentation::didFailLoading(m_frame, m_frame->loader().documentLoader(), m_currentResourceIdentifier, resourceError);
 
-    unsigned type = m_entryLoader->type();
     URL url(entryURL);
     url.removeFragmentIdentifier();
 
@@ -554,7 +553,7 @@ void ApplicationCacheGroup::didFailLoadingEntry(ApplicationCacheResourceLoader::
     m_pendingEntries.remove(url);
 
     if ((type & ApplicationCacheResource::Explicit) || (type & ApplicationCacheResource::Fallback)) {
-        m_frame->document()->addConsoleMessage(MessageSource::AppCache, MessageLevel::Error, "Application Cache update failed, because " + url.stringCenterEllipsizedToLength() + (m_entryLoader->hasRedirection() ? " was redirected." : " could not be fetched."));
+        m_frame->document()->addConsoleMessage(MessageSource::AppCache, MessageLevel::Error, makeString("Application Cache update failed, because ", url.stringCenterEllipsizedToLength(), (m_entryLoader && m_entryLoader->hasRedirection() ? " was redirected." : " could not be fetched.")));
         // Note that cacheUpdateFailed() can cause the cache group to be deleted.
         cacheUpdateFailed();
         return;
@@ -904,12 +903,13 @@ void ApplicationCacheGroup::startLoadingEntry()
 
     auto& documentLoader = *m_frame->loader().documentLoader();
     auto requestURL = request.url();
-    m_entryLoader = ApplicationCacheResourceLoader::create(m_pendingEntries.begin()->value, documentLoader.cachedResourceLoader(), WTFMove(request), [this, requestURL = WTFMove(requestURL)] (auto&& resourceOrError) {
+    unsigned type = m_pendingEntries.begin()->value;
+    m_entryLoader = ApplicationCacheResourceLoader::create(type, documentLoader.cachedResourceLoader(), WTFMove(request), [this, requestURL = WTFMove(requestURL), type] (auto&& resourceOrError) {
         if (!resourceOrError.has_value()) {
             auto error = resourceOrError.error();
             if (error == ApplicationCacheResourceLoader::Error::Abort)
                 return;
-            this->didFailLoadingEntry(error, requestURL);
+            this->didFailLoadingEntry(error, requestURL, type);
             return;
         }
 
index 9f628fd..abaa73e 100644 (file)
@@ -104,7 +104,7 @@ private:
     void didFinishLoadingManifest();
     void didFailLoadingManifest(ApplicationCacheResourceLoader::Error);
 
-    void didFailLoadingEntry(ApplicationCacheResourceLoader::Error, const URL&);
+    void didFailLoadingEntry(ApplicationCacheResourceLoader::Error, const URL&, unsigned type);
     void didFinishLoadingEntry(const URL&);
 
     void didReachMaxAppCacheSize();