RESOLVED FIXED 226386
Same-site lax cookies not sent by fetch event handler after page reload
https://bugs.webkit.org/show_bug.cgi?id=226386
Summary Same-site lax cookies not sent by fetch event handler after page reload
Steffen Weber
Reported 2021-05-28 11:44:39 PDT
Requests sent by Safari lack SameSite=Lax cookies when a service worker with a fetch event handler is involved. How to reproduce: 1. Fetch a SameSite=Lax demo cookie by visiting https://www.computerbase.de/api/samesite-demo-cookie (*) 2. Search Google for "computerbase" 3. Open the "Network" tab of the developer tools 4. Click on the search result that leads to our homepage (may not the first one depending on your location) 5. In the "Network" tab, find the "document" request and have a look at the cookies sent by Safari. For now, the "samesite-demo-cookie" is there. 6. Now reload our homepage. (Maybe this causes our Service Worker to fully initialize / activate?) 7. Repeat steps 2-4. Now the cookie doesn't get sent anymore. (*) I've created this script because other cookies set by ComputerBase don't use SameSite=Lax in Safari anymore because of this bug. The fetch event handler of our service worker doesn't handle all requests, for some requests it just returns "null" to let the browser handle the request (as if the service worker didn't exist). For example, the fetch event handler of our service worker returns "null" for all forum URLs (all URLs starting with "https://www.computerbase.de/forum/"). It turns out that forum URLs have no problem with same-site cookies: When I click on the search results entry for our forum (should be displayed when searching for "computerbase", if not then search for "computerbase forum") then the same-site demo cookie is always there as expected. If I then go back to the SERP and click on our homepage link again, the cookie is missing. Go back to the SERP and click on our forum link again, the cookie is there again. And so on... So we have the following: 1) Same-site cookie is sent to our homepage on first try, bot not anymore after one or two reloads (probably after the service-worker has been initialized and our fetch event handler is used). 2) Same-site cookie is always sent to our forum pages who are ignored by the fetch event handler. I think the combination of these two observations indicates that there is an issue with same-site cookies for requests sent by the fetch event handler. Split from Bug #219650 as suggested by John Wilander.
Attachments
Patch (5.65 KB, patch)
2021-06-07 03:04 PDT, youenn fablet
no flags
Patch (12.50 KB, patch)
2021-12-07 06:34 PST, youenn fablet
no flags
Patch (96.42 KB, patch)
2021-12-08 00:46 PST, youenn fablet
ews-feeder: commit-queue-
Patch (16.08 KB, patch)
2021-12-08 01:28 PST, youenn fablet
no flags
Patch for landing (16.09 KB, patch)
2021-12-08 07:40 PST, youenn fablet
no flags
John Wilander
Comment 1 2021-05-28 11:55:20 PDT
Thanks, Steffen! Cc’ing Youenn who knows most about Fetch and ServiceWorkers. Youenn, I can assist on the SameSite cookie part.
youenn fablet
Comment 2 2021-06-02 12:15:14 PDT
Hi Steffen, did you notice the lack of cookies from service worker by using web inspector or by noticing issues with the document being received or through server logs? If it is web inspector only, I am wondering whether we do not have an issue retrieving request/response cookie information when requests/responses go through the service worker.
youenn fablet
Comment 3 2021-06-02 12:15:50 PDT
Sadly, debugging service worker in STP seems broken, I do not see service workers be listed in the debug menu even though they are serving content.
youenn fablet
Comment 4 2021-06-02 12:17:04 PDT
Per Arne, I know there was a regression that we fixed some time ago where sandbox hardening broke service worker debugging. Maybe the issue came back. Bisecting might be feasible here.
Steffen Weber
Comment 5 2021-06-03 12:26:28 PDT
Initially, I only looked at the Safari Developer Tools to check the sent cookies but I've now verified that they are indeed not sent (by temporarily modifying our homepage to dump all cookies for requests originating at a specific BrowserStack IP address). Btw, I'm able to reproduce the issue by simply repeating step 6: The same-site cookie is gone after the second reload. Step 7 is not necessary.
Radar WebKit Bug Importer
Comment 6 2021-06-04 11:45:19 PDT
youenn fablet
Comment 7 2021-06-07 03:04:57 PDT
youenn fablet
Comment 8 2021-06-07 03:07:09 PDT
Steffen, I wrote the following test that is using same site cookie and everything seems to work as expected from this test. I used 'secure; HttpOnly; SameSite=Lax' as seems to be the case in your website. Let me know if you can spot a difference with your setup. As can be seen, the cookies are not provided in web inspector when doing a fetch through service worker. This should probably be fixed in a different bug.
youenn fablet
Comment 9 2021-06-08 02:30:47 PDT
Comment on attachment 430733 [details] Patch Might be nice to have a test landed so adding r? Steffen, please let me know if there is any difference with your setup.
Steffen Weber
Comment 10 2021-06-11 02:51:27 PDT
I'm on vacation. I'll have another look at the end of June. Sorry!
Steffen Weber
Comment 11 2021-06-24 04:58:54 PDT
I'm not familiar with Web Platform Tests but does the test actually simulate the process of navigating away from the site and back to the site via an external link? This is the crucial step that triggers the issue. It definitely isn't just a cosmetic Developer Tools issue because the problem was reported to us by actual Safari users (who were logged-out after clicking on Google result links).
youenn fablet
Comment 12 2021-12-07 06:19:15 PST
*** Bug 232440 has been marked as a duplicate of this bug. ***
erik.witt
Comment 13 2021-12-07 06:31:20 PST
Hey Youenn, thanks for linking to this issue. Is there any chances this gets fixed? I was concerced about the progress on my ticket: https://bugs.webkit.org/show_bug.cgi?id=232440 but seeing this one is open since May, I am getting a bit anxious. This is a huge issue on effected sites as I outlined in the ticket above. Happy to help if you need any more details
youenn fablet
Comment 14 2021-12-07 06:34:00 PST
youenn fablet
Comment 15 2021-12-07 06:34:27 PST
(In reply to erik.witt from comment #13) > Hey Youenn, thanks for linking to this issue. Is there any chances this gets > fixed? I was concerced about the progress on my ticket: > https://bugs.webkit.org/show_bug.cgi?id=232440 but seeing this one is open > since May, I am getting a bit anxious. > > This is a huge issue on effected sites as I outlined in the ticket above. > Happy to help if you need any more details I think I have a fix. You repro steps helped a lot.
youenn fablet
Comment 16 2021-12-08 00:46:40 PST
youenn fablet
Comment 17 2021-12-08 01:28:58 PST
Chris Dumez
Comment 18 2021-12-08 07:19:54 PST
Comment on attachment 446332 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446332&action=review > Source/WebCore/loader/FrameLoader.h:320 > + enum class IsServiceWorkerNavigationLoad { No, Yes }; : bool
youenn fablet
Comment 19 2021-12-08 07:40:33 PST
Created attachment 446364 [details] Patch for landing
EWS
Comment 20 2021-12-08 08:28:21 PST
Committed r286656 (244969@main): <https://commits.webkit.org/244969@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 446364 [details].
Brent Fulgham
Comment 21 2022-02-04 12:20:44 PST
This change should be present in STP 139, iOS 15.4 Beta, and macOS 12.3 Beta.
Note You need to log in before you can comment on or make changes to this bug.