Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 73b60062710e53db813763debaa06aa0defb1140
https://github.com/WebKit/WebKit/commit/73b60062710e53db813763debaa06aa0defb1140
Author: Alex Christensen <achristen...@apple.com>
Date: 2024-07-26 (Fri, 26 Jul 2024)
Changed paths:
M LayoutTests/platform/ios/TestExpectations
M LayoutTests/platform/mac-wk2/TestExpectations
M LayoutTests/platform/wk2/TestExpectations
M Source/WebCore/loader/cache/CachedResourceLoader.cpp
Log Message:
-----------
Update a request's SameSiteDisposition before using it for memory cache
matching
https://bugs.webkit.org/show_bug.cgi?id=277186
rdar://131857779
Reviewed by Matthew Finkel.
In macOS Sequoia and iOS 18, CFNetwork's default cookie samesite attribute
changed from none to lax
along with other browsers. This exposed an issue in our checking for Vary:
Cookie header matching
when considering whether to use the memory cache or not. This was found by a
failure in two layout
tests, http/tests/cache/disk-cache/disk-cache-vary-cookie-private.html and
http/tests/cache/disk-cache/disk-cache-vary-cookie.html which showed a resource
being loaded from the
disk cache instead of the memory cache.
After reduction and investigation and assistance from the CFNetwork team, I
found we were using the
SameSiteDisposition from a ResourceRequest with this stack trace:
WebCore::cookieRequestHeaderFieldValue(WebCore::CookieJar const*,
WebCore::ResourceRequest const&)
WebCore::verifyVaryingRequestHeaders(WebCore::CookieJar const*,
WTF::Vector<std::__1::pair<WTF::String, WTF::String>, 0ul,
WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::ResourceRequest
const&)::$_0::operator()(WTF::String const&) const::'lambda'()::operator()()
const
WTF::Detail::CallableWrapper<WebCore::verifyVaryingRequestHeaders(WebCore::CookieJar
const*, WTF::Vector<std::__1::pair<WTF::String, WTF::String>, 0ul,
WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::ResourceRequest
const&)::$_0::operator()(WTF::String const&) const::'lambda'(),
WTF::String>::call()
WTF::Function<WTF::String ()>::operator()() const
WebCore::headerValueForVary(WebCore::ResourceRequest const&, WTF::StringView,
WTF::Function<WTF::String ()>&&)
WebCore::verifyVaryingRequestHeaders(WebCore::CookieJar const*,
WTF::Vector<std::__1::pair<WTF::String, WTF::String>, 0ul,
WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::ResourceRequest
const&)::$_0::operator()(WTF::String const&) const
WTF::Detail::CallableWrapper<WebCore::verifyVaryingRequestHeaders(WebCore::CookieJar
const*, WTF::Vector<std::__1::pair<WTF::String, WTF::String>, 0ul,
WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::ResourceRequest
const&)::$_0, WTF::String, WTF::String const&>::call(WTF::String const&)
WTF::Function<WTF::String (WTF::String const&)>::operator()(WTF::String const&)
const
WebCore::verifyVaryingRequestHeadersInternal(WTF::Vector<std::__1::pair<WTF::String,
WTF::String>, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&,
WTF::Function<WTF::String (WTF::String const&)>&&)
WebCore::verifyVaryingRequestHeaders(WebCore::CookieJar const*,
WTF::Vector<std::__1::pair<WTF::String, WTF::String>, 0ul,
WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::ResourceRequest
const&)
WebCore::CachedResource::varyHeaderValuesMatch(WebCore::ResourceRequest const&)
WebCore::CachedResourceLoader::determineRevalidationPolicy(WebCore::CachedResource::Type,
WebCore::CachedResourceRequest&, WebCore::CachedResource*,
WebCore::CachedResourceLoader::ForPreload, WebCore::ImageLoading) const
WebCore::CachedResourceLoader::requestResource(WebCore::CachedResource::Type,
WebCore::CachedResourceRequest&&, WebCore::CachedResourceLoader::ForPreload,
WebCore::ImageLoading)
...
WebCore::XMLHttpRequest::send(WTF::String const&)
Before we were correctly setting the SameSiteDisposition with this stack trace:
WebCore::ResourceRequestBase::setIsSameSite(bool)
WebCore::FrameLoader::addSameSiteInfoToRequestIfNeeded(WebCore::ResourceRequest&,
WebCore::Document const*, WebCore::Page const*)
WebCore::FrameLoader::updateRequestAndAddExtraFields(WebCore::Frame&,
WebCore::ResourceRequest&, WebCore::IsMainResource, WebCore::FrameLoadType,
WebCore::ShouldUpdateAppInitiatedValue,
WebCore::FrameLoader::IsServiceWorkerNavigationLoad,
WebCore::FrameLoader::WillOpenInNewWindow, WebCore::Document*)
WebCore::FrameLoader::updateRequestAndAddExtraFields(WebCore::ResourceRequest&,
WebCore::IsMainResource, WebCore::FrameLoadType,
WebCore::ShouldUpdateAppInitiatedValue,
WebCore::FrameLoader::IsServiceWorkerNavigationLoad,
WebCore::FrameLoader::WillOpenInNewWindow, WebCore::Document*)
WebCore::CachedResource::load(WebCore::CachedResourceLoader&)
WebCore::CachedResourceLoader::requestResource(WebCore::CachedResource::Type,
WebCore::CachedResourceRequest&&, WebCore::CachedResourceLoader::ForPreload,
WebCore::ImageLoading)
...
WebCore::XMLHttpRequest::send(WTF::String const&)
The result was that when we called SameSiteInfo::create,
ResourceRequestBase::isSameSite() was returning
false when it should have been returning true, causing us to give CFNetwork a
dictionary with a key of
_kCFHTTPCookiePolicyPropertySiteForCookies and a value of an empty URL instead
of the non-empty URL, which,
when combined with the samesite attribute change caused them to give us fewer
cookies than they should,
which caused us to incorrectly determine that we can't use the memory cache.
The fix is to call FrameLoader::addSameSiteInfoToRequestIfNeeded before calling
determineRevalidationPolicy.
Calling FrameLoader::addSameSiteInfoToRequestIfNeeded multiple times in the
loading flow is not an issue
because if it has already been called it returns early.
The test had been marked as failing and timing out on several Cocoa platforms.
Since the tests seem
to be passing quite reliably on released OSes and this fixes it on Sequoia, I
updated test expectations
to expect the tests to pass reliably on all Cocoa platforms.
* LayoutTests/platform/ios/TestExpectations:
* LayoutTests/platform/mac-wk2/TestExpectations:
* LayoutTests/platform/wk2/TestExpectations:
* Source/WebCore/loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::requestResource):
Canonical link: https://commits.webkit.org/281441@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
