Avoid iterator invalidation bug in WebCore::defaultPortForProtocol

https://bugs.webkit.org/show_bug.cgi?id=172786
<rdar://problem/32499586>

Reviewed by Chris Dumez.

Create the SecurityOrigin objects on the main thread (rather than the worker queues)
since defaultPortForProtocol is not threadsafe.

* loader/ResourceLoadObserver.cpp:
(WebCore::ResourceLoadObserver::logFrameNavigation):
(WebCore::ResourceLoadObserver::logSubresourceLoading):
(WebCore::ResourceLoadObserver::logWebSocketLoading):

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 44f6285..2e4138c 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,19 @@
+2017-05-31  Brent Fulgham  <bfulgham@apple.com>
+
+        Avoid iterator invalidation bug in WebCore::defaultPortForProtocol
+        https://bugs.webkit.org/show_bug.cgi?id=172786
+        <rdar://problem/32499586>
+
+        Reviewed by Chris Dumez.
+
+        Create the SecurityOrigin objects on the main thread (rather than the worker queues)
+        since defaultPortForProtocol is not threadsafe.
+
+        * loader/ResourceLoadObserver.cpp:
+        (WebCore::ResourceLoadObserver::logFrameNavigation):
+        (WebCore::ResourceLoadObserver::logSubresourceLoading):
+        (WebCore::ResourceLoadObserver::logWebSocketLoading):
+
 2017-05-31  Mark Lam  <mark.lam@apple.com>
 
         Remove overrides of visitChildren() that do not add any functionality.

diff --git a/Source/WebCore/loader/ResourceLoadObserver.cpp b/Source/WebCore/loader/ResourceLoadObserver.cpp
index 6ffdbce..a376bb2 100644 (file)
--- a/Source/WebCore/loader/ResourceLoadObserver.cpp
+++ b/Source/WebCore/loader/ResourceLoadObserver.cpp
@@ -147,8 +147,10 @@ void ResourceLoadObserver::logFrameNavigation(const Frame& frame, const Frame& t
     if (targetPrimaryDomain == mainFramePrimaryDomain || targetPrimaryDomain == sourcePrimaryDomain)
         return;
     
+    auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
+    
     ASSERT(m_queue);
-    m_queue->dispatch([this, isMainFrame, isRedirect, sourcePrimaryDomain = sourcePrimaryDomain.isolatedCopy(), mainFramePrimaryDomain = mainFramePrimaryDomain.isolatedCopy(), targetURL = CrossThreadCopier<URL>::copy(targetURL), mainFrameURL = CrossThreadCopier<URL>::copy(mainFrameURL), targetPrimaryDomain = targetPrimaryDomain.isolatedCopy()] () {
+    m_queue->dispatch([this, isMainFrame, isRedirect, sourcePrimaryDomain = sourcePrimaryDomain.isolatedCopy(), mainFramePrimaryDomain = mainFramePrimaryDomain.isolatedCopy(), targetURL = CrossThreadCopier<URL>::copy(targetURL), mainFrameOrigin = mainFrameOrigin->isolatedCopy(), targetPrimaryDomain = targetPrimaryDomain.isolatedCopy()] () {
         
         auto targetOrigin = SecurityOrigin::create(targetURL);
         bool shouldFireDataModificationHandler = false;
@@ -166,7 +168,6 @@ void ResourceLoadObserver::logFrameNavigation(const Frame& frame, const Frame& t
         else {
             targetStatistics.subframeHasBeenLoadedBefore = true;
 
-            auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
             auto subframeUnderTopFrameOriginsResult = targetStatistics.subframeUnderTopFrameOrigins.add(mainFramePrimaryDomain);
             if (subframeUnderTopFrameOriginsResult.isNewEntry)
                 shouldFireDataModificationHandler = true;
@@ -243,8 +244,10 @@ void ResourceLoadObserver::logSubresourceLoading(const Frame* frame, const Resou
     if (targetPrimaryDomain == mainFramePrimaryDomain || (isRedirect && targetPrimaryDomain == sourcePrimaryDomain))
         return;
     
+    auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
+    
     ASSERT(m_queue);
-    m_queue->dispatch([this, isRedirect, sourcePrimaryDomain = sourcePrimaryDomain.isolatedCopy(), mainFramePrimaryDomain = mainFramePrimaryDomain.isolatedCopy(), targetPrimaryDomain = targetPrimaryDomain.isolatedCopy(), mainFrameURL = mainFrameURL.isolatedCopy()] () {
+    m_queue->dispatch([this, isRedirect, sourcePrimaryDomain = sourcePrimaryDomain.isolatedCopy(), mainFramePrimaryDomain = mainFramePrimaryDomain.isolatedCopy(), targetPrimaryDomain = targetPrimaryDomain.isolatedCopy(), mainFrameOrigin = mainFrameOrigin->isolatedCopy()] () {
         
         bool shouldFireDataModificationHandler = false;
         
@@ -255,7 +258,6 @@ void ResourceLoadObserver::logSubresourceLoading(const Frame* frame, const Resou
         // Always fire if we have previously removed data records for this domain
         shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
 
-        auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
         auto subresourceUnderTopFrameOriginsResult = targetStatistics.subresourceUnderTopFrameOrigins.add(mainFramePrimaryDomain);
         if (subresourceUnderTopFrameOriginsResult.isNewEntry)
             shouldFireDataModificationHandler = true;
@@ -321,8 +323,10 @@ void ResourceLoadObserver::logWebSocketLoading(const Frame* frame, const URL& ta
     if (targetPrimaryDomain == mainFramePrimaryDomain)
         return;
 
+    auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
+    
     ASSERT(m_queue);
-    m_queue->dispatch([this, targetPrimaryDomain = targetPrimaryDomain.isolatedCopy(), mainFramePrimaryDomain = mainFramePrimaryDomain.isolatedCopy(), mainFrameURL = mainFrameURL.isolatedCopy()] () {
+    m_queue->dispatch([this, targetPrimaryDomain = targetPrimaryDomain.isolatedCopy(), mainFramePrimaryDomain = mainFramePrimaryDomain.isolatedCopy(), mainFrameOrigin = mainFrameOrigin->isolatedCopy()] () {
         
         bool shouldFireDataModificationHandler = false;
         
@@ -333,7 +337,6 @@ void ResourceLoadObserver::logWebSocketLoading(const Frame* frame, const URL& ta
         // Always fire if we have previously removed data records for this domain
         shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
         
-        auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
         auto subresourceUnderTopFrameOriginsResult = targetStatistics.subresourceUnderTopFrameOrigins.add(mainFramePrimaryDomain);
         if (subresourceUnderTopFrameOriginsResult.isNewEntry)
             shouldFireDataModificationHandler = true;