Bug 214222

Summary: ASSERT([filteredCookies.get() count] <= 1) on imported/w3c/web-platform-tests/websockets/cookies/third-party-cookie-accepted.https.html
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Page LoadingAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, achristensen, ap, beidson, ggaren, webkit-bot-watchers-bugzilla, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=214880
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Chris Dumez
Reported 2020-07-11 10:19:03 PDT
imported/w3c/web-platform-tests/websockets/cookies/third-party-cookie-accepted.https.html is crashing consistently on iOS WK2 Debug configuration: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x000000011d578d0e WTFCrash + 14 (Assertions.cpp:295) 1 com.apple.WebCore 0x0000000122d552eb WTFCrashWithInfo(int, char const*, char const*, int) + 27 2 com.apple.WebCore 0x0000000124aa42a0 WebCore::NetworkStorageSession::setCookiesFromDOM(WTF::URL const&, WebCore::SameSiteInfo const&, WTF::URL const&, WTF::Optional<WTF::ObjectIdentifier<WebCore::FrameIdentifierType> >, WTF::Optional<WTF::ObjectIdentifier<WebCore::PageIdentifierType> >, WebCore::ShouldAskITP, WTF::String const&, WebCore::ShouldRelaxThirdPartyCookieBlocking) const + 1136 (NetworkStorageSessionCocoa.mm:467) 3 com.apple.WebKit 0x000000010cb40b1b WebKit::NetworkConnectionToWebProcess::setCookiesFromDOM(WTF::URL const&, WebCore::SameSiteInfo const&, WTF::URL const&, WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebCore::ShouldAskITP, WTF::String const&, WebCore::ShouldRelaxThirdPartyCookieBlocking) + 299 (NetworkConnectionToWebProcess.cpp:655) 4 com.apple.WebKit 0x000000010c697615 void IPC::callMemberFunctionImpl<WebKit::NetworkConnectionToWebProcess, void (WebKit::NetworkConnectionToWebProcess::*)(WTF::URL const&, WebCore::SameSiteInfo const&, WTF::URL const&, WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebCore::ShouldAskITP, WTF::String const&, WebCore::ShouldRelaxThirdPartyCookieBlocking), std::__1::tuple<WTF::URL, WebCore::SameSiteInfo, WTF::URL, WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebCore::ShouldAskITP, WTF::String, WebCore::ShouldRelaxThirdPartyCookieBlocking>, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul, 7ul>(WebKit::NetworkConnectionToWebProcess*, void (WebKit::NetworkConnectionToWebProcess::*)(WTF::URL const&, WebCore::SameSiteInfo const&, WTF::URL const&, WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebCore::ShouldAskITP, WTF::String const&, WebCore::ShouldRelaxThirdPartyCookieBlocking), std::__1::tuple<WTF::URL, WebCore::SameSiteInfo, WTF::URL, WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebCore::ShouldAskITP, WTF::String, WebCore::ShouldRelaxThirdPartyCookieBlocking>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul, 7ul>) + 437 (HandleMessage.h:42) 5 com.apple.WebKit 0x000000010c690580 void IPC::callMemberFunction<WebKit::NetworkConnectionToWebProcess, void (WebKit::NetworkConnectionToWebProcess::*)(WTF::URL const&, WebCore::SameSiteInfo const&, WTF::URL const&, WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebCore::ShouldAskITP, WTF::String const&, WebCore::ShouldRelaxThirdPartyCookieBlocking), std::__1::tuple<WTF::URL, WebCore::SameSiteInfo, WTF::URL, WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebCore::ShouldAskITP, WTF::String, WebCore::ShouldRelaxThirdPartyCookieBlocking>, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul, 7ul> >(std::__1::tuple<WTF::URL, WebCore::SameSiteInfo, WTF::URL, WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebCore::ShouldAskITP, WTF::String, WebCore::ShouldRelaxThirdPartyCookieBlocking>&&, WebKit::NetworkConnectionToWebProcess*, void (WebKit::NetworkConnectionToWebProcess::*)(WTF::URL const&, WebCore::SameSiteInfo const&, WTF::URL const&, WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebCore::ShouldAskITP, WTF::String const&, WebCore::ShouldRelaxThirdPartyCookieBlocking)) + 112 (HandleMessage.h:48) 6 com.apple.WebKit 0x000000010c66496e void IPC::handleMessage<Messages::NetworkConnectionToWebProcess::SetCookiesFromDOM, WebKit::NetworkConnectionToWebProcess, void (WebKit::NetworkConnectionToWebProcess::*)(WTF::URL const&, WebCore::SameSiteInfo const&, WTF::URL const&, WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebCore::ShouldAskITP, WTF::String const&, WebCore::ShouldRelaxThirdPartyCookieBlocking)>(IPC::Decoder&, WebKit::NetworkConnectionToWebProcess*, void (WebKit::NetworkConnectionToWebProcess::*)(WTF::URL const&, WebCore::SameSiteInfo const&, WTF::URL const&, WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebCore::ShouldAskITP, WTF::String const&, WebCore::ShouldRelaxThirdPartyCookieBlocking)) + 190 (HandleMessage.h:115) 7 com.apple.WebKit 0x000000010c6630dc WebKit::NetworkConnectionToWebProcess::didReceiveNetworkConnectionToWebProcessMessage(IPC::Connection&, IPC::Decoder&) + 1116 (NetworkConnectionToWebProcessMessageReceiver.cpp:365) 8 com.apple.WebKit 0x000000010cb3bd63 WebKit::NetworkConnectionToWebProcess::didReceiveMessage(IPC::Connection&, IPC::Decoder&) + 179 (NetworkConnectionToWebProcess.cpp:191) 9 com.apple.WebKit 0x000000010c5a54ff IPC::Connection::dispatchMessage(IPC::Decoder&) + 431 (Connection.cpp:1002) 10 com.apple.WebKit 0x000000010c5a5e30 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 528 11 com.apple.WebKit 0x000000010c5a64c0 IPC::Connection::dispatchOneIncomingMessage() + 208 (Connection.cpp:1139) 12 com.apple.WebKit 0x000000010c5c5278 IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_7::operator()() + 88 (Connection.cpp:979) 13 com.apple.WebKit 0x000000010c5c518e WTF::Detail::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_7, void>::call() + 30 (Function.h:52) 14 com.apple.JavaScriptCore 0x000000011d5a4b02 WTF::Function<void ()>::operator()() const + 130 (Function.h:84) 15 com.apple.JavaScriptCore 0x000000011d61d015 WTF::RunLoop::performWork() + 341 (RunLoop.cpp:120) 16 com.apple.JavaScriptCore 0x000000011d6205b1 WTF::RunLoop::performWork(void*) + 33 (RunLoopCF.cpp:39) 17 com.apple.CoreFoundation 0x0000000108189c71 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17 18 com.apple.CoreFoundation 0x0000000108189b9c __CFRunLoopDoSource0 + 76 19 com.apple.CoreFoundation 0x0000000108189374 __CFRunLoopDoSources0 + 180 20 com.apple.CoreFoundation 0x0000000108183f6e __CFRunLoopRun + 974 21 com.apple.CoreFoundation 0x0000000108183884 CFRunLoopRunSpecific + 404 22 com.apple.Foundation 0x0000000107a49831 -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 211 23 com.apple.Foundation 0x0000000107a49a45 -[NSRunLoop(NSRunLoop) run] + 76 24 libxpc.dylib 0x0000000109dd65ae _xpc_objc_main + 297 25 libxpc.dylib 0x0000000109dd8976 xpc_main + 132 26 com.apple.WebKit 0x000000010cce509b WebKit::XPCServiceMain(int, char const**) + 1067 (XPCServiceMain.mm:171) 27 com.apple.WebKit 0x000000010e20758b WKXPCServiceMain + 27 (WKMain.mm:33) 28 com.apple.WebKit.Networking 0x00000001078e9da2 main + 34 (AuxiliaryProcessMain.cpp:30) 29 libdyld.dylib 0x0000000109aa210d start + 1
Attachments
Patch (14.24 KB, patch)
2020-07-22 16:47 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (14.24 KB, patch)
2020-07-22 16:51 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (14.15 KB, patch)
2020-07-23 08:47 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2020-07-11 10:21:16 PDT
Updated expectations in <https://trac.webkit.org/changeset/264268>.
Alexey Proskuryakov
Comment 2 2020-07-14 23:01:15 PDT
Line 467 is this, which is a bit surprising. ASSERT([filteredCookies.get() count] <= 1);
Radar WebKit Bug Importer
Comment 3 2020-07-14 23:01:23 PDT
<rdar://problem/65587120>
Chris Dumez
Comment 4 2020-07-22 15:04:21 PDT
Looks like we use a different API for parsing cookies on macOS and iOS: #if PLATFORM(MAC) NSArray *unfilteredCookies = [NSHTTPCookie _parsedCookiesWithResponseHeaderFields:headerFields forURL:cookieURL]; #else NSArray *unfilteredCookies = [NSHTTPCookie cookiesWithResponseHeaderFields:headerFields forURL:cookieURL]; #endif
Chris Dumez
Comment 5 2020-07-22 15:18:32 PDT
The cookie string is: samesite-unspecified=0.2755897489430851; Path=/, samesite-lax=0.2755897489430851; Path=/; SameSite=Lax, samesite-strict=0.2755897489430851; Path=/; SameSite=Strict, samesite-none=0.2755897489430851; Path=/; SameSite=None; Secure
Chris Dumez
Comment 6 2020-07-22 15:24:23 PDT
(In reply to Chris Dumez from comment #5) > The cookie string is: > > samesite-unspecified=0.2755897489430851; Path=/, > samesite-lax=0.2755897489430851; Path=/; SameSite=Lax, > samesite-strict=0.2755897489430851; Path=/; SameSite=Strict, > samesite-none=0.2755897489430851; Path=/; SameSite=None; Secure the HTML spec says to treat the string set to document.cookie as a Set-Cookie header. Set-Cookie header is defined here: https://tools.ietf.org/html/rfc6265#section-4.1 It should only support one cookie indeed.
Chris Dumez
Comment 7 2020-07-22 16:47:46 PDT
Created attachment 404995 [details] Patch
Chris Dumez
Comment 8 2020-07-22 16:51:11 PDT
Created attachment 404996 [details] Patch
Alex Christensen
Comment 9 2020-07-22 20:27:41 PDT
Comment on attachment 404996 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404996&action=review I think this is good, but this is a bad time to make a change like this. Could this wait until after the next branch? > Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm:424 > + RetainPtr<NSDate> dateInAWeek = adoptNS([[NSDate alloc] initWithTimeIntervalSinceNow:cappedLifetime->seconds()]); auto > Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm:459 > + setHTTPCookiesForURL(cookieStorage().get(), [NSArray arrayWithObject:cookie], cookieURL, firstParty, sameSiteInfo); @[cookie]
Chris Dumez
Comment 10 2020-07-23 08:35:22 PDT
(In reply to Alex Christensen from comment #9) > Comment on attachment 404996 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=404996&action=review > > I think this is good, but this is a bad time to make a change like this. > Could this wait until after the next branch? I guess it can but it is merely aligning iOS with shipping macOS. > > > Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm:424 > > + RetainPtr<NSDate> dateInAWeek = adoptNS([[NSDate alloc] initWithTimeIntervalSinceNow:cappedLifetime->seconds()]); > > auto > > > Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm:459 > > + setHTTPCookiesForURL(cookieStorage().get(), [NSArray arrayWithObject:cookie], cookieURL, firstParty, sameSiteInfo); > > @[cookie]
Chris Dumez
Comment 11 2020-07-23 08:37:58 PDT
(In reply to Chris Dumez from comment #10) > (In reply to Alex Christensen from comment #9) > > Comment on attachment 404996 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=404996&action=review > > > > I think this is good, but this is a bad time to make a change like this. > > Could this wait until after the next branch? > > I guess it can but it is merely aligning iOS with shipping macOS. It also only impact document.cookie setting. > > > > > > Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm:424 > > > + RetainPtr<NSDate> dateInAWeek = adoptNS([[NSDate alloc] initWithTimeIntervalSinceNow:cappedLifetime->seconds()]); > > > > auto > > > > > Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm:459 > > > + setHTTPCookiesForURL(cookieStorage().get(), [NSArray arrayWithObject:cookie], cookieURL, firstParty, sameSiteInfo); > > > > @[cookie]
Chris Dumez
Comment 12 2020-07-23 08:47:46 PDT
Created attachment 405042 [details] Patch
EWS
Comment 13 2020-07-27 13:58:19 PDT
Committed r264943: <https://trac.webkit.org/changeset/264943> All reviewed patches have been landed. Closing bug and clearing flags on attachment 405042 [details].
Aakash Jain
Comment 14 2020-07-28 04:12:35 PDT
(In reply to EWS from comment #13) > Committed r264943: <https://trac.webkit.org/changeset/264943> http/tests/cookies/document-cookie-multiple-cookies.html is failing consistently since this commit, as also warned by EWS. History: https://results.webkit.org/?suite=layout-tests&test=http%2Ftests%2Fcookies%2Fdocument-cookie-multiple-cookies.html
Aakash Jain
Comment 15 2020-07-28 04:13:01 PDT
e.g.: https://ews-build.s3-us-west-2.amazonaws.com/Windows-EWS/r405345-31731-clean-tree/results.html -PASS document.cookie is "samesite-unspecified=0" +FAIL document.cookie should be samesite-unspecified=0. Was samesite-lax=1; samesite-strict=2; samesite-unspecified=0.
Chris Dumez
Comment 16 2020-07-28 09:41:03 PDT
(In reply to Aakash Jain from comment #15) > e.g.: > https://ews-build.s3-us-west-2.amazonaws.com/Windows-EWS/r405345-31731-clean- > tree/results.html > > -PASS document.cookie is "samesite-unspecified=0" > +FAIL document.cookie should be samesite-unspecified=0. Was samesite-lax=1; > samesite-strict=2; samesite-unspecified=0. Ok. Windows still has the bug. I only fixed it for iOS.
Chris Dumez
Comment 17 2020-07-28 10:00:34 PDT
(In reply to Chris Dumez from comment #16) > (In reply to Aakash Jain from comment #15) > > e.g.: > > https://ews-build.s3-us-west-2.amazonaws.com/Windows-EWS/r405345-31731-clean- > > tree/results.html > > > > -PASS document.cookie is "samesite-unspecified=0" > > +FAIL document.cookie should be samesite-unspecified=0. Was samesite-lax=1; > > samesite-strict=2; samesite-unspecified=0. > > Ok. Windows still has the bug. I only fixed it for iOS. Fixing the same bug on Windows via Bug 214880.
Note You need to log in before you can comment on or make changes to this bug.