Skip to content
Permalink
Browse files
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.

Canonical link: https://commits.webkit.org/198830@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@228975 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
fujii authored and webkit-commit-queue committed Feb 24, 2018
1 parent 51fb984 commit 33cbce0dcf8cd8297e08316149fc8c7116917e4c
@@ -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-17 Darin Adler <darin@apple.com>

Prepare for ExtendedColor changes (first step)
@@ -1437,6 +1437,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)
@@ -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;
@@ -182,7 +182,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;
}

0 comments on commit 33cbce0

Please sign in to comment.