RESOLVED FIXED 158616
Vary:Cookie validation doesn't work in private browsing
https://bugs.webkit.org/show_bug.cgi?id=158616
Summary Vary:Cookie validation doesn't work in private browsing
Antti Koivisto
Reported 2016-06-10 07:47:18 PDT
We currently reload any resource with Vary:Cookie in private browsing even if we have a memory cache entry.
Attachments
patch (19.58 KB, patch)
2016-06-10 08:30 PDT, Antti Koivisto
no flags
patch (19.63 KB, patch)
2016-06-10 09:46 PDT, Antti Koivisto
darin: review+
patch (24.64 KB, patch)
2016-06-11 02:56 PDT, Antti Koivisto
no flags
patch (22.40 KB, patch)
2016-06-13 07:03 PDT, Antti Koivisto
no flags
Antti Koivisto
Comment 1 2016-06-10 08:30:27 PDT
Antti Koivisto
Comment 2 2016-06-10 09:46:46 PDT
Alex Christensen
Comment 3 2016-06-10 09:49:22 PDT
View in context: https://bugs.webkit.org/attachment.cgi?id=281010&action=review Is this a NetworkStorageSession in the WebProcess? Are its cookies synchronized with the cookies in the NetworkProcess somehow? Could we use this map from SessionTracker so we don't have duplicate code? > Source/WebCore/ChangeLog:13 > + around curretnly WebKit2 level SessionTracker. currently
Darin Adler
Comment 4 2016-06-10 15:28:35 PDT
Comment on attachment 281012 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=281012&action=review > Source/WebCore/platform/network/NetworkStorageSession.h:77 > + NetworkStorageSession(SessionID); Maybe mark this explicit? > Source/WebCore/platform/network/cf/NetworkStorageSessionCFNet.cpp:44 > NetworkStorageSession::NetworkStorageSession(SessionID sessionID, RetainPtr<CFURLStorageSessionRef> platformSession) Argument type should be RetainPtr&&.
Antti Koivisto
Comment 5 2016-06-11 02:56:20 PDT
Radar WebKit Bug Importer
Comment 6 2016-06-11 03:53:54 PDT
Antti Koivisto
Comment 7 2016-06-11 04:08:58 PDT
Csaba Osztrogonác
Comment 8 2016-06-11 06:23:10 PDT
(In reply to comment #7) > https://trac.webkit.org/r201967 It broke the WinCairo build, see build.webkit.org for details. cc-ing port maintainers too
Antti Koivisto
Comment 9 2016-06-11 10:57:51 PDT
I tried fixing WinCairo in https://trac.webkit.org/r201972
Alexey Proskuryakov
Comment 10 2016-06-11 12:27:26 PDT
This change caused assertion failures on IndexedDB tests (not always the same test). For whatever reason, I don't see this on build.webkit.org bots, only internally. I'm going to roll out, and will I'll e-mail links to the crashes.
WebKit Commit Bot
Comment 11 2016-06-11 12:30:31 PDT
Re-opened since this is blocked by bug 158665
Brady Eidson
Comment 12 2016-06-11 17:41:53 PDT
The assert was no specific to IndexedDB tests but rather was underneath InjectedBundle::setPrivateBrowsingEnabled IDB tests are the vast majority of setPrivateBrowsingEnabled users, but not the exclusive ones - Other tests do it too.
Antti Koivisto
Comment 13 2016-06-13 02:39:51 PDT
(In reply to comment #12) > The assert was no specific to IndexedDB tests but rather was underneath > InjectedBundle::setPrivateBrowsingEnabled > > IDB tests are the vast majority of setPrivateBrowsingEnabled users, but not > the exclusive ones - Other tests do it too. setPrivateBrowsingEnabled uses non-unique “legacyPrivateSessionID" which causes the assert. Not sure why public bots don't hit it though. Maybe two setPrivateBrowsingEnabled tests never run in the same process there for some reason.
Antti Koivisto
Comment 14 2016-06-13 07:03:08 PDT
Created attachment 281172 [details] patch Different approach, add a SessionId version of the cookie getter function and do the mapping to NetworkStorageSession on WebKit side.
WebKit Commit Bot
Comment 15 2016-06-15 07:10:38 PDT
Comment on attachment 281172 [details] patch Clearing flags on attachment: 281172 Committed r202089: <http://trac.webkit.org/changeset/202089>
WebKit Commit Bot
Comment 16 2016-06-15 07:10:43 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.