Merge r228975 - Null-dereference of the second argument resource of…

… DocumentLoader::scheduleSubstituteResourceLoad https://bugs.webkit.org/show_bug.cgi?id=182920 Patch by Fujii Hironori <Hironori.Fujii@sony.com> on 2018-02-24 Reviewed by Darin Adler. A test case imported/w3c/web-platform-tests/html/browsers/offline/appcache/workers/appcache-worker.html always crashes due to a null-dereference if compiled and optimized by GCC 7.2. The second argument `resource` of DocumentLoader::scheduleSubstituteResourceLoad can be null if the resource can't be found in cache. I guess GCC optimizes inline HashMap::add based on assuming the `resource` never becomes null because its type is SubstituteResource&. This changes introduces a new method DocumentLoader::scheduleCannotShowURLError because it looks tricky to pass a nullptr to the second argument of scheduleSubstituteResourceLoad. No new tests (Covered by existing tests). * loader/DocumentLoader.cpp: (WebCore::DocumentLoader::scheduleCannotShowURLError): Added a new method. * loader/DocumentLoader.h: * loader/appcache/ApplicationCacheHost.cpp: (WebCore::ApplicationCacheHost::maybeLoadResource): Call scheduleCannotShowURLError if the resource not found in the appcache.
WebKit · Mar 5, 2018 · 0820d5c · 0820d5c
1 parent c39317f
commit 0820d5c
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 1 deletion.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,33 @@
+2018-02-24  Fujii Hironori  <Hironori.Fujii@sony.com>
+
+        Null-dereference of the second argument `resource` of DocumentLoader::scheduleSubstituteResourceLoad
+        https://bugs.webkit.org/show_bug.cgi?id=182920
+
+        Reviewed by Darin Adler.
+
+        A test case
+        imported/w3c/web-platform-tests/html/browsers/offline/appcache/workers/appcache-worker.html
+        always crashes due to a null-dereference if compiled and optimized
+        by GCC 7.2. The second argument `resource` of
+        DocumentLoader::scheduleSubstituteResourceLoad can be null if the
+        resource can't be found in cache. I guess GCC optimizes inline
+        HashMap::add based on assuming the `resource` never becomes null
+        because its type is SubstituteResource&.
+
+        This changes introduces a new method
+        DocumentLoader::scheduleCannotShowURLError because it looks tricky
+        to pass a nullptr to the second argument of
+        scheduleSubstituteResourceLoad.
+
+        No new tests (Covered by existing tests).
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::scheduleCannotShowURLError): Added a new method.
+        * loader/DocumentLoader.h:
+        * loader/appcache/ApplicationCacheHost.cpp:
+        (WebCore::ApplicationCacheHost::maybeLoadResource):
+        Call scheduleCannotShowURLError if the resource not found in the appcache.
+
 2018-02-23  Chris Dumez  <cdumez@apple.com>
 
         Crash under SchemeRegistry::shouldTreatURLSchemeAsLocal(WTF::String const&)

diff --git a/Source/WebCore/loader/DocumentLoader.cpp b/Source/WebCore/loader/DocumentLoader.cpp
@@ -1432,6 +1432,12 @@ void DocumentLoader::scheduleSubstituteResourceLoad(ResourceLoader& loader, Subs
     deliverSubstituteResourcesAfterDelay();
 }
 
+void DocumentLoader::scheduleCannotShowURLError(ResourceLoader& loader)
+{
+    m_pendingSubstituteResources.set(&loader, nullptr);
+    deliverSubstituteResourcesAfterDelay();
+}
+
 void DocumentLoader::addResponse(const ResourceResponse& response)
 {
     if (!m_stopRecordingResponses)

diff --git a/Source/WebCore/loader/DocumentLoader.h b/Source/WebCore/loader/DocumentLoader.h
@@ -184,6 +184,7 @@ class DocumentLoader : public RefCounted<DocumentLoader>, public FrameDestructio
 #endif
 
     void scheduleSubstituteResourceLoad(ResourceLoader&, SubstituteResource&);
+    void scheduleCannotShowURLError(ResourceLoader&);
 
     // Return the ArchiveResource for the URL only when loading an Archive
     WEBCORE_EXPORT ArchiveResource* archiveResourceForURL(const URL&) const;

diff --git a/Source/WebCore/loader/appcache/ApplicationCacheHost.cpp b/Source/WebCore/loader/appcache/ApplicationCacheHost.cpp
@@ -179,7 +179,10 @@ bool ApplicationCacheHost::maybeLoadResource(ResourceLoader& loader, const Resou
     if (!shouldLoadResourceFromApplicationCache(request, resource))
         return false;
 
-    m_documentLoader.scheduleSubstituteResourceLoad(loader, *resource);
+    if (resource)
+        m_documentLoader.scheduleSubstituteResourceLoad(loader, *resource);
+    else
+        m_documentLoader.scheduleCannotShowURLError(loader);
     return true;
 }