WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(12.50 KB, patch)
2021-12-07 06:34 PST
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch
(96.42 KB, patch)
2021-12-08 00:46 PST
,
youenn fablet
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(16.08 KB, patch)
2021-12-08 01:28 PST
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch for landing
(16.09 KB, patch)
2021-12-08 07:40 PST
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
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
<
rdar://problem/78878853
>
youenn fablet
Comment 7
2021-06-07 03:04:57 PDT
Created
attachment 430733
[details]
Patch
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
Created
attachment 446168
[details]
Patch
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
Created
attachment 446330
[details]
Patch
youenn fablet
Comment 17
2021-12-08 01:28:58 PST
Created
attachment 446332
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug