Title: [218539] releases/WebKitGTK/webkit-2.16
- Revision
- 218539
- Author
- carlo...@webkit.org
- Date
- 2017-06-19 23:31:06 -0700 (Mon, 19 Jun 2017)
Log Message
Merge r217445 - 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.
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog (218538 => 218539)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog 2017-06-20 06:24:41 UTC (rev 218538)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog 2017-06-20 06:31:06 UTC (rev 218539)
@@ -1,3 +1,16 @@
+2017-05-25 Chris Dumez <cdu...@apple.com>
+
+ 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.
+
+ 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.
+
2017-05-24 Jiewen Tan <jiewen_...@apple.com>
Crash on WebCore::FrameSelection::setSelectionWithoutUpdatingAppearance + 1195
Added: releases/WebKitGTK/webkit-2.16/LayoutTests/http/tests/xmlhttprequest/cacheable-cross-origin-redirect-crash-expected.txt (0 => 218539)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/http/tests/xmlhttprequest/cacheable-cross-origin-redirect-crash-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/http/tests/xmlhttprequest/cacheable-cross-origin-redirect-crash-expected.txt 2017-06-20 06:31:06 UTC (rev 218539)
@@ -0,0 +1,10 @@
+Tests that we do not crash when XHR reuses a cached resource that contains a simple cross origin redirect.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS internals.xhrResponseSource(xhr) is "Memory cache"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: releases/WebKitGTK/webkit-2.16/LayoutTests/http/tests/xmlhttprequest/cacheable-cross-origin-redirect-crash.html (0 => 218539)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/http/tests/xmlhttprequest/cacheable-cross-origin-redirect-crash.html (rev 0)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/http/tests/xmlhttprequest/cacheable-cross-origin-redirect-crash.html 2017-06-20 06:31:06 UTC (rev 218539)
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description("Tests that we do not crash when XHR reuses a cached resource that contains a simple cross origin redirect.");
+jsTestIsAsync = true;
+
+const url = ""
+
+xhr = new XMLHttpRequest();
+xhr.withCredentials = true;
+xhr._onload_ = function() {
+ xhr = new XMLHttpRequest();
+ xhr.withCredentials = true;
+ xhr._onload_ = function() {
+ shouldBeEqualToString("internals.xhrResponseSource(xhr)", "Memory cache");
+ finishJSTest();
+ };
+ xhr.open("GET", url);
+ xhr.send();
+};
+xhr.open("GET", url);
+xhr.send();
+</script>
+</body>
+</html>
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog (218538 => 218539)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog 2017-06-20 06:24:41 UTC (rev 218538)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog 2017-06-20 06:31:06 UTC (rev 218539)
@@ -1,3 +1,25 @@
+2017-05-25 Chris Dumez <cdu...@apple.com>
+
+ 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.
+
+ 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:
+
2017-05-25 Zalan Bujtas <za...@apple.com>
ASSERTION FAILED: !needsStyleRecalc() || !document().childNeedsStyleRecalc()
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/DocumentThreadableLoader.cpp (218538 => 218539)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/DocumentThreadableLoader.cpp 2017-06-20 06:24:41 UTC (rev 218538)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/DocumentThreadableLoader.cpp 2017-06-20 06:31:06 UTC (rev 218539)
@@ -226,6 +226,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.
@@ -250,12 +251,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.
@@ -263,7 +265,8 @@
return;
m_options.allowCredentials = DoNotAllowStoredCredentials;
- m_options.maxRedirectCount -= m_resource->loader()->redirectCount();
+ m_options.maxRedirectCount -= m_redirectCount;
+ m_redirectCount = 0;
clearResource();
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/DocumentThreadableLoader.h (218538 => 218539)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/DocumentThreadableLoader.h 2017-06-20 06:24:41 UTC (rev 218538)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/DocumentThreadableLoader.h 2017-06-20 06:31:06 UTC (rev 218539)
@@ -132,6 +132,7 @@
std::unique_ptr<ContentSecurityPolicy> m_contentSecurityPolicy;
std::optional<CrossOriginPreflightChecker> m_preflightChecker;
std::optional<HTTPHeaderMap> m_originalHeaders;
+ unsigned m_redirectCount { 0 };
ShouldLogError m_shouldLogError;
};
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
