Skip to content

Commit

Permalink
BlobURLHandle doesn't work as intended for opaque origin Blob URLs
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=244862

Reviewed by Darin Adler.

BlobURLHandle doesn't work as intended for opaque origin Blob URLs. This is
causing some of the subtests in imported/w3c/web-platform-tests/FileAPI/url/sandboxed-iframe.html
to fail in WebKit.

The issue is that the SecurityOrigin of an opaque-origin blob URL gets stored
in a OriginMap inside ThreadableBlobOrigin. When the blob URL would later get
revoked, we would remove its origin from the OriginMap, without checking if
there are any BlobURLHandles keeping the blob alive.

* LayoutTests/imported/w3c/web-platform-tests/FileAPI/url/sandboxed-iframe-expected.txt:
* Source/WebCore/fileapi/ThreadableBlobRegistry.cpp:
(WebCore::blobURLReferencesMap):
(WebCore::unregisterBlobURLOriginIfNecessary):
(WebCore::ThreadableBlobRegistry::registerBlobURL):
(WebCore::ThreadableBlobRegistry::unregisterBlobURL):
(WebCore::ThreadableBlobRegistry::registerBlobURLHandle):
(WebCore::ThreadableBlobRegistry::unregisterBlobURLHandle):

Canonical link: https://commits.webkit.org/254238@main
  • Loading branch information
cdumez committed Sep 7, 2022
1 parent 4e07bc3 commit 2953e99
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 8 deletions.
Expand Up @@ -21,7 +21,7 @@ PASS XHR with method "OPTIONS" should fail
PASS XHR with method "PUT" should fail
PASS XHR with method "CUSTOM" should fail
PASS XHR should return Content-Type from Blob
FAIL Revoke blob URL after open(), will fetch assert_unreached: Got unexpected error event Reached unreachable code
PASS Revoke blob URL after open(), will fetch
PASS Blob URLs can be used in fetch
PASS fetch with a fragment should succeed
PASS fetch of a revoked URL should fail
Expand All @@ -35,7 +35,7 @@ PASS fetch with method "OPTIONS" should fail
PASS fetch with method "PUT" should fail
PASS fetch with method "CUSTOM" should fail
PASS fetch should return Content-Type from Blob
FAIL Revoke blob URL after creating Request, will fetch promise_test: Unhandled rejection with value: object "TypeError: Load failed"
FAIL Revoke blob URL after creating Request, then clone Request, will fetch promise_test: Unhandled rejection with value: object "TypeError: Load failed"
PASS Revoke blob URL after creating Request, will fetch
PASS Revoke blob URL after creating Request, then clone Request, will fetch
PASS Revoke blob URL after calling fetch, fetch should succeed

38 changes: 33 additions & 5 deletions Source/WebCore/fileapi/ThreadableBlobRegistry.cpp
Expand Up @@ -57,13 +57,24 @@ static ThreadSpecific<BlobUrlOriginMap>& originMap()
{
static std::once_flag onceFlag;
static ThreadSpecific<BlobUrlOriginMap>* map;
std::call_once(onceFlag, []{
std::call_once(onceFlag, [] {
map = new ThreadSpecific<BlobUrlOriginMap>;
});

return *map;
}

static ThreadSpecific<HashCountedSet<String>>& blobURLReferencesMap()
{
static std::once_flag onceFlag;
static ThreadSpecific<HashCountedSet<String>>* map;
std::call_once(onceFlag, [] {
map = new ThreadSpecific<HashCountedSet<String>>;
});

return *map;
}

void ThreadableBlobRegistry::registerFileBlobURL(const URL& url, const String& path, const String& replacementPath, const String& contentType)
{
String effectivePath = !replacementPath.isNull() ? replacementPath : path;
Expand Down Expand Up @@ -99,11 +110,24 @@ static inline bool isBlobURLContainsNullOrigin(const URL& url)
return StringView(url.string()).substring(startIndex, endIndex - startIndex - 1) == "null"_s;
}

static void unregisterBlobURLOriginIfNecessary(const URL& url)
{
if (!isBlobURLContainsNullOrigin(url))
return;

auto urlWithoutFragment = url.stringWithoutFragmentIdentifier();
if (blobURLReferencesMap()->remove(urlWithoutFragment))
originMap()->remove(urlWithoutFragment);
}

void ThreadableBlobRegistry::registerBlobURL(SecurityOrigin* origin, PolicyContainer&& policyContainer, const URL& url, const URL& srcURL)
{
// If the blob URL contains null origin, as in the context with unique security origin or file URL, save the mapping between url and origin so that the origin can be retrived when doing security origin check.
if (origin && isBlobURLContainsNullOrigin(url))
originMap()->add<StringViewHashTranslator>(url.viewWithoutFragmentIdentifier(), origin);
if (origin && isBlobURLContainsNullOrigin(url)) {
auto urlWithoutFragment = url.stringWithoutFragmentIdentifier();
originMap()->add(urlWithoutFragment, origin);
blobURLReferencesMap()->add(urlWithoutFragment);
}

if (isMainThread()) {
blobRegistry().registerBlobURL(url, srcURL, policyContainer);
Expand Down Expand Up @@ -152,8 +176,7 @@ unsigned long long ThreadableBlobRegistry::blobSize(const URL& url)

void ThreadableBlobRegistry::unregisterBlobURL(const URL& url)
{
if (isBlobURLContainsNullOrigin(url))
originMap()->remove<StringViewHashTranslator>(url.viewWithoutFragmentIdentifier());
unregisterBlobURLOriginIfNecessary(url);

ensureOnMainThread([url = url.isolatedCopy()] {
blobRegistry().unregisterBlobURL(url);
Expand All @@ -162,13 +185,18 @@ void ThreadableBlobRegistry::unregisterBlobURL(const URL& url)

void ThreadableBlobRegistry::registerBlobURLHandle(const URL& url)
{
if (isBlobURLContainsNullOrigin(url))
blobURLReferencesMap()->add(url.stringWithoutFragmentIdentifier());

ensureOnMainThread([url = url.isolatedCopy()] {
blobRegistry().registerBlobURLHandle(url);
});
}

void ThreadableBlobRegistry::unregisterBlobURLHandle(const URL& url)
{
unregisterBlobURLOriginIfNecessary(url);

ensureOnMainThread([url = url.isolatedCopy()] {
blobRegistry().unregisterBlobURLHandle(url);
});
Expand Down

0 comments on commit 2953e99

Please sign in to comment.