Slight CTTE in PingLoader
authorandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 8 Feb 2014 20:02:40 +0000 (20:02 +0000)
committerandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 8 Feb 2014 20:02:40 +0000 (20:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=128462

Reviewed by Dan Bernstein.

PingLoader always wants a non-null frame.

* html/HTMLAnchorElement.cpp:
(WebCore::HTMLAnchorElement::sendPings):
* html/parser/XSSAuditorDelegate.cpp:
(WebCore::XSSAuditorDelegate::didBlockScript):
* inspector/InspectorInstrumentation.h:
(WebCore::InspectorInstrumentation::continueAfterPingLoader):
* loader/PingLoader.cpp:
(WebCore::PingLoader::loadImage):
(WebCore::PingLoader::sendPing):
(WebCore::PingLoader::sendViolationReport):
(WebCore::PingLoader::createPingLoader):
(WebCore::PingLoader::PingLoader):
* loader/PingLoader.h:
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::requestImage):
* page/ContentSecurityPolicy.cpp:
(WebCore::ContentSecurityPolicy::reportViolation):

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

Source/WebCore/ChangeLog
Source/WebCore/html/HTMLAnchorElement.cpp
Source/WebCore/html/parser/XSSAuditorDelegate.cpp
Source/WebCore/inspector/InspectorInstrumentation.h
Source/WebCore/loader/PingLoader.cpp
Source/WebCore/loader/PingLoader.h
Source/WebCore/loader/cache/CachedResourceLoader.cpp
Source/WebCore/page/ContentSecurityPolicy.cpp

index fbb77d4..54c87e1 100644 (file)
@@ -1,3 +1,30 @@
+2014-02-08  Anders Carlsson  <andersca@apple.com>
+
+        Slight CTTE in PingLoader
+        https://bugs.webkit.org/show_bug.cgi?id=128462
+
+        Reviewed by Dan Bernstein.
+
+        PingLoader always wants a non-null frame.
+
+        * html/HTMLAnchorElement.cpp:
+        (WebCore::HTMLAnchorElement::sendPings):
+        * html/parser/XSSAuditorDelegate.cpp:
+        (WebCore::XSSAuditorDelegate::didBlockScript):
+        * inspector/InspectorInstrumentation.h:
+        (WebCore::InspectorInstrumentation::continueAfterPingLoader):
+        * loader/PingLoader.cpp:
+        (WebCore::PingLoader::loadImage):
+        (WebCore::PingLoader::sendPing):
+        (WebCore::PingLoader::sendViolationReport):
+        (WebCore::PingLoader::createPingLoader):
+        (WebCore::PingLoader::PingLoader):
+        * loader/PingLoader.h:
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::requestImage):
+        * page/ContentSecurityPolicy.cpp:
+        (WebCore::ContentSecurityPolicy::reportViolation):
+
 2014-02-08  Dan Bernstein  <mitz@apple.com>
 
         Remove client-drawn highlights (-webkit-highlight, WebHTMLHighlighter)
index 60259c1..c046443 100644 (file)
@@ -516,7 +516,7 @@ void HTMLAnchorElement::sendPings(const URL& destinationURL)
 
     SpaceSplitString pingURLs(getAttribute(pingAttr), false);
     for (unsigned i = 0; i < pingURLs.size(); i++)
-        PingLoader::sendPing(document().frame(), document().completeURL(pingURLs[i]), destinationURL);
+        PingLoader::sendPing(*document().frame(), document().completeURL(pingURLs[i]), destinationURL);
 }
 
 void HTMLAnchorElement::handleClick(Event* event)
index 9cba069..322d44d 100644 (file)
@@ -110,7 +110,7 @@ void XSSAuditorDelegate::didBlockScript(const XSSInfo& xssInfo)
         frameLoader.client().didDetectXSS(m_document.url(), xssInfo.m_didBlockEntirePage);
 
         if (!m_reportURL.isEmpty())
-            PingLoader::sendViolationReport(m_document.frame(), m_reportURL, generateViolationReport());
+            PingLoader::sendViolationReport(*m_document.frame(), m_reportURL, generateViolationReport());
     }
 
     if (xssInfo.m_didBlockEntirePage)
index cd5e91c..ddff892 100644 (file)
@@ -164,7 +164,7 @@ public:
 
     static void applyEmulatedMedia(Frame*, String*);
     static void willSendRequest(Frame*, unsigned long identifier, DocumentLoader*, ResourceRequest&, const ResourceResponse& redirectResponse);
-    static void continueAfterPingLoader(Frame*, unsigned long identifier, DocumentLoader*, ResourceRequest&, const ResourceResponse&);
+    static void continueAfterPingLoader(Frame&, unsigned long identifier, DocumentLoader*, ResourceRequest&, const ResourceResponse&);
     static void markResourceAsCached(Page*, unsigned long identifier);
     static void didLoadResourceFromMemoryCache(Page*, DocumentLoader*, CachedResource*);
     static InspectorInstrumentationCookie willReceiveResourceData(Frame*, unsigned long identifier, int length);
@@ -1186,10 +1186,10 @@ inline void InspectorInstrumentation::willSendRequest(Frame* frame, unsigned lon
 #endif
 }
 
-inline void InspectorInstrumentation::continueAfterPingLoader(Frame* frame, unsigned long identifier, DocumentLoader* loader, ResourceRequest& request, const ResourceResponse& response)
+inline void InspectorInstrumentation::continueAfterPingLoader(Frame& frame, unsigned long identifier, DocumentLoader* loader, ResourceRequest& request, const ResourceResponse& response)
 {
 #if ENABLE(INSPECTOR)
-    if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForFrame(frame))
+    if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForFrame(&frame))
         InspectorInstrumentation::continueAfterPingLoaderImpl(instrumentingAgents, identifier, loader, request, response);
 #else
     UNUSED_PARAM(frame);
index 70f2b25..d51cd52 100644 (file)
 
 namespace WebCore {
 
-void PingLoader::loadImage(Frame* frame, const URL& url)
+void PingLoader::loadImage(Frame& frame, const URL& url)
 {
-    if (!frame->document()->securityOrigin()->canDisplay(url)) {
-        FrameLoader::reportLocalLoadFailed(frame, url);
+    if (!frame.document()->securityOrigin()->canDisplay(url)) {
+        FrameLoader::reportLocalLoadFailed(&frame, url);
         return;
     }
 
     ResourceRequest request(url);
     request.setHTTPHeaderField("Cache-Control", "max-age=0");
-    String referrer = SecurityPolicy::generateReferrerHeader(frame->document()->referrerPolicy(), request.url(), frame->loader().outgoingReferrer());
+    String referrer = SecurityPolicy::generateReferrerHeader(frame.document()->referrerPolicy(), request.url(), frame.loader().outgoingReferrer());
     if (!referrer.isEmpty())
         request.setHTTPReferrer(referrer);
-    frame->loader().addExtraFieldsToSubresourceRequest(request);
+    frame.loader().addExtraFieldsToSubresourceRequest(request);
 
     createPingLoader(frame, request);
 }
 
 // http://www.whatwg.org/specs/web-apps/current-work/multipage/links.html#hyperlink-auditing
-void PingLoader::sendPing(Frame* frame, const URL& pingURL, const URL& destinationURL)
+void PingLoader::sendPing(Frame& frame, const URL& pingURL, const URL& destinationURL)
 {
     ResourceRequest request(pingURL);
     request.setHTTPMethod("POST");
     request.setHTTPContentType("text/ping");
     request.setHTTPBody(FormData::create("PING"));
     request.setHTTPHeaderField("Cache-Control", "max-age=0");
-    frame->loader().addExtraFieldsToSubresourceRequest(request);
+    frame.loader().addExtraFieldsToSubresourceRequest(request);
 
-    SecurityOrigin* sourceOrigin = frame->document()->securityOrigin();
+    SecurityOrigin* sourceOrigin = frame.document()->securityOrigin();
     RefPtr<SecurityOrigin> pingOrigin = SecurityOrigin::create(pingURL);
     FrameLoader::addHTTPOriginIfNeeded(request, sourceOrigin->toString());
     request.setHTTPHeaderField("Ping-To", destinationURL);
-    if (!SecurityPolicy::shouldHideReferrer(pingURL, frame->loader().outgoingReferrer())) {
-        request.setHTTPHeaderField("Ping-From", frame->document()->url());
+    if (!SecurityPolicy::shouldHideReferrer(pingURL, frame.loader().outgoingReferrer())) {
+        request.setHTTPHeaderField("Ping-From", frame.document()->url());
         if (!sourceOrigin->isSameSchemeHostPort(pingOrigin.get())) {
-            String referrer = SecurityPolicy::generateReferrerHeader(frame->document()->referrerPolicy(), pingURL, frame->loader().outgoingReferrer());
+            String referrer = SecurityPolicy::generateReferrerHeader(frame.document()->referrerPolicy(), pingURL, frame.loader().outgoingReferrer());
             if (!referrer.isEmpty())
                 request.setHTTPReferrer(referrer);
         }
@@ -92,40 +92,40 @@ void PingLoader::sendPing(Frame* frame, const URL& pingURL, const URL& destinati
     createPingLoader(frame, request);
 }
 
-void PingLoader::sendViolationReport(Frame* frame, const URL& reportURL, PassRefPtr<FormData> report)
+void PingLoader::sendViolationReport(Frame& frame, const URL& reportURL, PassRefPtr<FormData> report)
 {
     ResourceRequest request(reportURL);
     request.setHTTPMethod("POST");
     request.setHTTPContentType("application/json");
     request.setHTTPBody(report);
-    frame->loader().addExtraFieldsToSubresourceRequest(request);
+    frame.loader().addExtraFieldsToSubresourceRequest(request);
 
-    String referrer = SecurityPolicy::generateReferrerHeader(frame->document()->referrerPolicy(), reportURL, frame->loader().outgoingReferrer());
+    String referrer = SecurityPolicy::generateReferrerHeader(frame.document()->referrerPolicy(), reportURL, frame.loader().outgoingReferrer());
     if (!referrer.isEmpty())
         request.setHTTPReferrer(referrer);
 
     createPingLoader(frame, request);
 }
 
-void PingLoader::createPingLoader(Frame* frame, ResourceRequest& request)
+void PingLoader::createPingLoader(Frame& frame, ResourceRequest& request)
 {
     // No need to free the PingLoader object or manage it via a smart pointer - it will kill itself as soon as it receives a response.
     new PingLoader(frame, request);
 }
 
-PingLoader::PingLoader(Frame* frame, ResourceRequest& request)
+PingLoader::PingLoader(Frame& frame, ResourceRequest& request)
     : m_timeout(this, &PingLoader::timeoutTimerFired)
 {
-    unsigned long identifier = frame->page()->progress().createUniqueIdentifier();
+    unsigned long identifier = frame.page()->progress().createUniqueIdentifier();
     // FIXME: Why activeDocumentLoader? I would have expected documentLoader().
     // Itseems like the PingLoader should be associated with the current
     // Document in the Frame, but the activeDocumentLoader will be associated
     // with the provisional DocumentLoader if there is a provisional
     // DocumentLoader.
-    m_shouldUseCredentialStorage = frame->loader().client().shouldUseCredentialStorage(frame->loader().activeDocumentLoader(), identifier);
-    m_handle = ResourceHandle::create(frame->loader().networkingContext(), request, this, false, false);
+    m_shouldUseCredentialStorage = frame.loader().client().shouldUseCredentialStorage(frame.loader().activeDocumentLoader(), identifier);
+    m_handle = ResourceHandle::create(frame.loader().networkingContext(), request, this, false, false);
 
-    InspectorInstrumentation::continueAfterPingLoader(frame, identifier, frame->loader().activeDocumentLoader(), request, ResourceResponse());
+    InspectorInstrumentation::continueAfterPingLoader(frame, identifier, frame.loader().activeDocumentLoader(), request, ResourceResponse());
 
     // If the server never responds, FrameLoader won't be able to cancel this load and
     // we'll sit here waiting forever. Set a very generous timeout, just in case.
index 295e2f4..b3bec06 100644 (file)
@@ -54,15 +54,15 @@ class ResourceResponse;
 class PingLoader : private ResourceHandleClient {
     WTF_MAKE_NONCOPYABLE(PingLoader); WTF_MAKE_FAST_ALLOCATED;
 public:
-    static void loadImage(Frame*, const URL& url);
-    static void sendPing(Frame*, const URL& pingURL, const URL& destinationURL);
-    static void sendViolationReport(Frame*, const URL& reportURL, PassRefPtr<FormData> report);
+    static void loadImage(Frame&, const URL&);
+    static void sendPing(Frame&, const URL& pingURL, const URL& destinationURL);
+    static void sendViolationReport(Frame&, const URL& reportURL, PassRefPtr<FormData> report);
 
     virtual ~PingLoader();
 
 private:
-    static void createPingLoader(Frame*, ResourceRequest&);
-    PingLoader(Frame*, ResourceRequest&);
+    static void createPingLoader(Frame&, ResourceRequest&);
+    PingLoader(Frame&, ResourceRequest&);
 
     virtual void didReceiveResponse(ResourceHandle*, const ResourceResponse&) override { delete this; }
     virtual void didReceiveData(ResourceHandle*, const char*, unsigned, int) override { delete this; }
index 61e6018..11f18a8 100644 (file)
@@ -149,14 +149,15 @@ Frame* CachedResourceLoader::frame() const
 
 CachedResourceHandle<CachedImage> CachedResourceLoader::requestImage(CachedResourceRequest& request)
 {
-    if (Frame* f = frame()) {
-        if (f->loader().pageDismissalEventBeingDispatched() != FrameLoader::NoDismissal) {
+    if (Frame* frame = this->frame()) {
+        if (frame->loader().pageDismissalEventBeingDispatched() != FrameLoader::NoDismissal) {
             URL requestURL = request.resourceRequest().url();
             if (requestURL.isValid() && canRequest(CachedResource::ImageResource, requestURL, request.options(), request.forPreload()))
-                PingLoader::loadImage(f, requestURL);
-            return 0;
+                PingLoader::loadImage(*frame, requestURL);
+            return nullptr;
         }
     }
+    
     request.setDefer(clientDefersImage(request.resourceRequest().url()) ? CachedResourceRequest::DeferredByClient : CachedResourceRequest::NoDefer);
     return toCachedImage(requestResource(CachedResource::ImageResource, request).get());
 }
index 927c085..1fa112a 100644 (file)
@@ -1805,8 +1805,8 @@ void ContentSecurityPolicy::reportViolation(const String& directiveText, const S
 
     RefPtr<FormData> report = FormData::create(reportObject->toJSONString().utf8());
 
-    for (size_t i = 0; i < reportURIs.size(); ++i)
-        PingLoader::sendViolationReport(frame, reportURIs[i], report);
+    for (const auto& url : reportURIs)
+        PingLoader::sendViolationReport(*frame, url, report);
 }
 
 void ContentSecurityPolicy::reportUnsupportedDirective(const String& name) const