DocumentThreadableLoader::redirectReceived() should not rely on the resource's loader
https://bugs.webkit.org/show_bug.cgi?id=172578
<rdar://problem/30754582>
Reviewed by Youenn Fablet.
Source/WebCore:
DocumentThreadableLoader::redirectReceived() should not rely on the resource's loader. The rest of the methods do not.
It is unsafe for it to rely on the resource's loader because it gets cleared when the load completes. A CachedRawresource
may be reused from the memory cache once its load has completed.
This would cause crashes in CachedRawResource::didAddClient() when replaying the redirects because it would call
DocumentThreadableLoader::redirectReceived() and potentially not have a loader anymore. To hit this exact code path,
you would need to make repeated XHR to a cacheable simple cross-origin resource that has cacheable redirect.
Test: http/tests/xmlhttprequest/cacheable-cross-origin-redirect-crash.html
* loader/DocumentThreadableLoader.cpp:
(WebCore::DocumentThreadableLoader::redirectReceived):
* loader/DocumentThreadableLoader.h:
LayoutTests:
Add layout test coverage.
* http/tests/xmlhttprequest/cacheable-cross-origin-redirect-crash-expected.txt: Added.
* http/tests/xmlhttprequest/cacheable-cross-origin-redirect-crash.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@217445 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/loader/DocumentThreadableLoader.cpp b/Source/WebCore/loader/DocumentThreadableLoader.cpp
index 71c7118..2ca54ab 100644
--- a/Source/WebCore/loader/DocumentThreadableLoader.cpp
+++ b/Source/WebCore/loader/DocumentThreadableLoader.cpp
@@ -228,6 +228,7 @@
ASSERT_UNUSED(resource, &resource == m_resource);
Ref<DocumentThreadableLoader> protectedThis(*this);
+ ++m_redirectCount;
// FIXME: We restrict this check to Fetch API for the moment, as this might disrupt WorkerScriptLoader.
// Reassess this check based on https://github.com/whatwg/fetch/issues/393 discussions.
@@ -252,12 +253,13 @@
m_sameOriginRequest = false;
ASSERT(m_resource);
- ASSERT(m_resource->loader());
- ASSERT(m_options.mode == FetchOptions::Mode::Cors);
ASSERT(m_originalHeaders);
- // Loader might have modified the origin to a unique one, let's reuse it for subsequent loads.
- m_origin = m_resource->loader()->origin();
+ // Use a unique for subsequent loads if needed.
+ // https://fetch.spec.whatwg.org/#concept-http-redirect-fetch (Step 10).
+ ASSERT(m_options.mode == FetchOptions::Mode::Cors);
+ if (!securityOrigin().canRequest(redirectResponse.url()) && !SecurityOrigin::create(redirectResponse.url())->canRequest(request.url()))
+ m_origin = SecurityOrigin::createUnique();
// Except in case where preflight is needed, loading should be able to continue on its own.
// But we also handle credentials here if it is restricted to SameOrigin.
@@ -265,7 +267,8 @@
return;
m_options.allowCredentials = DoNotAllowStoredCredentials;
- m_options.maxRedirectCount -= m_resource->loader()->redirectCount();
+ m_options.maxRedirectCount -= m_redirectCount;
+ m_redirectCount = 0;
clearResource();
