- Revision
- 291689
- Author
- cdu...@apple.com
- Date
- 2022-03-22 12:51:52 -0700 (Tue, 22 Mar 2022)
Log Message
REGRESSION (iOS 15.1 / r280824) QuickLook - model not loading when passing extra parameters
https://bugs.webkit.org/show_bug.cgi?id=236069
<rdar://problem/88461772>
Reviewed by Darin Adler.
Source/WebCore:
To extend the lifetime of Blob objects while loading/downloading is pending, we rely to
BlobURLHandle objects, which tell the NetworkProcess that we still need this blob data,
even if the corresponding Blob object gets GC'd by JS. The issue here is that the load
was not using the vanilla blob URL but was intead appending a fragment to the URL. As
a result, BlobRegistryImpl::registerBlobURLHandle() would fail to look up the
corresponding blob data and would fail to extend its lifetime.
To address the issue, BlobRegistryImpl::registerBlobURLHandle() / unregisterBlobURLHandle()
now strip fragments from the blob URL, similarly to what was already done inside
BlobRegistryImpl::getBlobDataFromURL().
Test: fast/files/blob-with-fragment-as-frame-url.html
* platform/network/BlobRegistryImpl.cpp:
(WebCore::blobURLWithoutFragment):
(WebCore::BlobRegistryImpl::registerBlobURLHandle):
(WebCore::BlobRegistryImpl::unregisterBlobURLHandle):
Source/WebKit:
Fix theoretical blob data leak. A WebContent process could create several BlobURLHandles for
the same blob URL. This would result in several calls to NetworkConnectionToWebProcess::registerBlobURLHandle()
for the same URL and several calls to to BlobRegistryImpl::registerBlobURLHandle() for the
same URL as well. BlobRegistryImpl is using a HashCountedSet for m_blobReferences in order
to deal with this fact. However, NetworkConnectionToWebProcess was using a simple HashSet
for m_blobURLHandles. As a result, if the WebContent process would exit and didClose()
would get called, the NetworkConnectionToWebProcess may call BlobRegistryImpl::unregisterBlobURLHandle()
only once even though the WebContent process had several handles for this URL, which would
not fully remove the URL from BlobRegistryImpl's HashCountedSet. To address the issue,
NetworkConnectionToWebProcess::m_blobURLHandles is now a HashCountedSet too and we call
BlobRegistryImpl::unregisterBlobURLHandle() as many times as needed in didClose().
* NetworkProcess/NetworkConnectionToWebProcess.cpp:
(WebKit::NetworkConnectionToWebProcess::didClose):
* NetworkProcess/NetworkConnectionToWebProcess.h:
LayoutTests:
Add layout test coverage.
* fast/files/blob-with-fragment-as-frame-url-expected.txt: Added.
* fast/files/blob-with-fragment-as-frame-url.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (291688 => 291689)
--- trunk/LayoutTests/ChangeLog 2022-03-22 19:45:56 UTC (rev 291688)
+++ trunk/LayoutTests/ChangeLog 2022-03-22 19:51:52 UTC (rev 291689)
@@ -1,3 +1,16 @@
+2022-03-22 Chris Dumez <cdu...@apple.com>
+
+ REGRESSION (iOS 15.1 / r280824) QuickLook - model not loading when passing extra parameters
+ https://bugs.webkit.org/show_bug.cgi?id=236069
+ <rdar://problem/88461772>
+
+ Reviewed by Darin Adler.
+
+ Add layout test coverage.
+
+ * fast/files/blob-with-fragment-as-frame-url-expected.txt: Added.
+ * fast/files/blob-with-fragment-as-frame-url.html: Added.
+
2022-03-22 Matteo Flores <matteo_flo...@apple.com>
[ iOS EWS ] fast/text/text-shadow-ink-overflow-missing.html is a flaky image failure
Added: trunk/LayoutTests/fast/files/blob-with-fragment-as-frame-url-expected.txt (0 => 291689)
--- trunk/LayoutTests/fast/files/blob-with-fragment-as-frame-url-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/files/blob-with-fragment-as-frame-url-expected.txt 2022-03-22 19:51:52 UTC (rev 291689)
@@ -0,0 +1,12 @@
+Tests the lifetime of a Blob URL with a fragment.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS iframe.contentDocument.body.innerText is "FOO"
+PASS iframe.contentWindow.location.href == blobURLWithFragment is true
+PASS iframe.contentDocument.URL == blobURLWithFragment is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/files/blob-with-fragment-as-frame-url.html (0 => 291689)
--- trunk/LayoutTests/fast/files/blob-with-fragment-as-frame-url.html (rev 0)
+++ trunk/LayoutTests/fast/files/blob-with-fragment-as-frame-url.html 2022-03-22 19:51:52 UTC (rev 291689)
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description("Tests the lifetime of a Blob URL with a fragment.");
+jsTestIsAsync = true;
+
+_onload_ = () => {
+ let blobContents = `<body>FOO</body>`;
+ let blob = new Blob([blobContents], { type: "text/html" });
+ blobURL = URL.createObjectURL(blob);
+ blobURLWithFragment = blobURL + "#foo=1";
+ iframe = document.getElementById("testFrame");
+ iframe._onload_ = () => {
+ shouldBeEqualToString("iframe.contentDocument.body.innerText", "FOO");
+ shouldBeTrue("iframe.contentWindow.location.href == blobURLWithFragment");
+ shouldBeTrue("iframe.contentDocument.URL == blobURLWithFragment");
+ finishJSTest();
+ };
+ let anchor = document.createElement("a");
+ anchor.target = "testFrame";
+ anchor.href = ""
+ anchor.click();
+ URL.revokeObjectURL(blobURL);
+}
+</script>
+<iframe id="testFrame" name="testFrame" src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (291688 => 291689)
--- trunk/Source/WebCore/ChangeLog 2022-03-22 19:45:56 UTC (rev 291688)
+++ trunk/Source/WebCore/ChangeLog 2022-03-22 19:51:52 UTC (rev 291689)
@@ -1,3 +1,29 @@
+2022-03-22 Chris Dumez <cdu...@apple.com>
+
+ REGRESSION (iOS 15.1 / r280824) QuickLook - model not loading when passing extra parameters
+ https://bugs.webkit.org/show_bug.cgi?id=236069
+ <rdar://problem/88461772>
+
+ Reviewed by Darin Adler.
+
+ To extend the lifetime of Blob objects while loading/downloading is pending, we rely to
+ BlobURLHandle objects, which tell the NetworkProcess that we still need this blob data,
+ even if the corresponding Blob object gets GC'd by JS. The issue here is that the load
+ was not using the vanilla blob URL but was intead appending a fragment to the URL. As
+ a result, BlobRegistryImpl::registerBlobURLHandle() would fail to look up the
+ corresponding blob data and would fail to extend its lifetime.
+
+ To address the issue, BlobRegistryImpl::registerBlobURLHandle() / unregisterBlobURLHandle()
+ now strip fragments from the blob URL, similarly to what was already done inside
+ BlobRegistryImpl::getBlobDataFromURL().
+
+ Test: fast/files/blob-with-fragment-as-frame-url.html
+
+ * platform/network/BlobRegistryImpl.cpp:
+ (WebCore::blobURLWithoutFragment):
+ (WebCore::BlobRegistryImpl::registerBlobURLHandle):
+ (WebCore::BlobRegistryImpl::unregisterBlobURLHandle):
+
2022-03-22 Per Arne Vollan <pvol...@apple.com>
Enable content filtering in the Network process
Modified: trunk/Source/WebCore/platform/network/BlobRegistryImpl.cpp (291688 => 291689)
--- trunk/Source/WebCore/platform/network/BlobRegistryImpl.cpp 2022-03-22 19:45:56 UTC (rev 291688)
+++ trunk/Source/WebCore/platform/network/BlobRegistryImpl.cpp 2022-03-22 19:51:52 UTC (rev 291689)
@@ -50,6 +50,11 @@
namespace WebCore {
+static String blobURLWithoutFragment(const URL& url)
+{
+ return url.hasFragmentIdentifier() ? url.stringWithoutFragmentIdentifier().toString() : url.string();
+}
+
BlobRegistryImpl::~BlobRegistryImpl() = default;
static Ref<ResourceHandle> createBlobResourceHandle(const ResourceRequest& request, ResourceHandleClient* client)
@@ -375,14 +380,16 @@
void BlobRegistryImpl::registerBlobURLHandle(const URL& url)
{
- if (m_blobs.contains(url.string()))
- m_blobReferences.add(url.string());
+ auto urlKey = blobURLWithoutFragment(url);
+ if (m_blobs.contains(urlKey))
+ m_blobReferences.add(urlKey);
}
void BlobRegistryImpl::unregisterBlobURLHandle(const URL& url)
{
- if (m_blobReferences.remove(url.string()))
- m_blobs.remove(url.string());
+ auto urlKey = blobURLWithoutFragment(url);
+ if (m_blobReferences.remove(urlKey))
+ m_blobs.remove(urlKey);
}
} // namespace WebCore
Modified: trunk/Source/WebKit/ChangeLog (291688 => 291689)
--- trunk/Source/WebKit/ChangeLog 2022-03-22 19:45:56 UTC (rev 291688)
+++ trunk/Source/WebKit/ChangeLog 2022-03-22 19:51:52 UTC (rev 291689)
@@ -1,3 +1,27 @@
+2022-03-22 Chris Dumez <cdu...@apple.com>
+
+ REGRESSION (iOS 15.1 / r280824) QuickLook - model not loading when passing extra parameters
+ https://bugs.webkit.org/show_bug.cgi?id=236069
+ <rdar://problem/88461772>
+
+ Reviewed by Darin Adler.
+
+ Fix theoretical blob data leak. A WebContent process could create several BlobURLHandles for
+ the same blob URL. This would result in several calls to NetworkConnectionToWebProcess::registerBlobURLHandle()
+ for the same URL and several calls to to BlobRegistryImpl::registerBlobURLHandle() for the
+ same URL as well. BlobRegistryImpl is using a HashCountedSet for m_blobReferences in order
+ to deal with this fact. However, NetworkConnectionToWebProcess was using a simple HashSet
+ for m_blobURLHandles. As a result, if the WebContent process would exit and didClose()
+ would get called, the NetworkConnectionToWebProcess may call BlobRegistryImpl::unregisterBlobURLHandle()
+ only once even though the WebContent process had several handles for this URL, which would
+ not fully remove the URL from BlobRegistryImpl's HashCountedSet. To address the issue,
+ NetworkConnectionToWebProcess::m_blobURLHandles is now a HashCountedSet too and we call
+ BlobRegistryImpl::unregisterBlobURLHandle() as many times as needed in didClose().
+
+ * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+ (WebKit::NetworkConnectionToWebProcess::didClose):
+ * NetworkProcess/NetworkConnectionToWebProcess.h:
+
2022-03-22 Youenn Fablet <you...@apple.com>
LibWebRTCCodecsProxy.mm and SharedVideoFrame.cpp do not need to be built as part of WebContent executable
Modified: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp (291688 => 291689)
--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp 2022-03-22 19:45:56 UTC (rev 291688)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp 2022-03-22 19:51:52 UTC (rev 291689)
@@ -436,8 +436,10 @@
networkSession->broadcastChannelRegistry().removeConnection(connection);
for (auto& url : m_blobURLs)
networkSession->blobRegistry().unregisterBlobURL(url);
- for (auto& url : m_blobURLHandles)
- networkSession->blobRegistry().unregisterBlobURLHandle(url);
+ for (auto& [url, count] : m_blobURLHandles) {
+ for (unsigned i = 0; i < count; ++i)
+ networkSession->blobRegistry().unregisterBlobURLHandle(url);
+ }
}
// All trackers of resources that were in the middle of being loaded were
Modified: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h (291688 => 291689)
--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h 2022-03-22 19:45:56 UTC (rev 291688)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h 2022-03-22 19:51:52 UTC (rev 291689)
@@ -439,7 +439,7 @@
Ref<NetworkSchemeRegistry> m_schemeRegistry;
HashSet<URL> m_blobURLs;
- HashSet<URL> m_blobURLHandles;
+ HashCountedSet<URL> m_blobURLHandles;
#if ENABLE(IPC_TESTING_API)
IPCTester m_ipcTester;
#endif
