Bug 238202

Summary: Computation of Document siteForCookies is buggy in case document is created by window.open
Product: WebKit Reporter: youenn fablet <youennf>
Component: Page LoadingAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, cdumez, clopez, esprehn+autocc, ews-watchlist, ggaren, japhet, kangil.han, webkit-bug-importer, wilander
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Tests
none
Patch
none
Patch for landing none

youenn fablet
Reported 2022-03-22 07:22:21 PDT
Attachments
Tests (9.27 KB, patch)
2022-03-22 07:23 PDT, youenn fablet
no flags
Patch (12.35 KB, patch)
2022-03-22 09:30 PDT, youenn fablet
no flags
Patch for landing (12.93 KB, patch)
2022-03-23 00:58 PDT, youenn fablet
no flags
Radar WebKit Bug Importer
Comment 1 2022-03-22 07:22:35 PDT
youenn fablet
Comment 2 2022-03-22 07:23:20 PDT
youenn fablet
Comment 3 2022-03-22 07:23:48 PDT
Uploaded tests are passing in Chrome and Firefox but not in Safari.
youenn fablet
Comment 4 2022-03-22 09:30:41 PDT
John Wilander
Comment 5 2022-03-22 11:02:41 PDT
Comment on attachment 455381 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=455381&action=review r=me based on enhancing the tests (and getting green EWS bots). > LayoutTests/http/tests/cookies/resources/testharness-helpers.js:38 > + document.cookie = LAX_DOM + "=1; SameSite=Lax; Max-Age=100; path=/"; Please add a SameSite=Strict cookie too and make sure it works as expected. > LayoutTests/http/tests/cookies/resources/testharness-helpers.js:68 > + document.cookie = LAX_DOM + "=1; SameSite=Lax; Max-Age=100; path=/"; Ditto on a SameSite=Strict cookie. > LayoutTests/http/tests/cookies/same-site/popup-from-iframe-same-site-with-post-form-expected.txt:2 > +PASS popup opened as 'about:blank', then post navigation to 127.0.0.1, so samesite cookies are sent. This output should be more specific and say whether SameSite Lax and SameSite Strict cookies were sent. > LayoutTests/http/tests/cookies/same-site/popup-from-iframe-same-site-with-post-form-expected.txt:3 > +PASS popup opened as '127.0.0.1', then post navigation to 127.0.0.1, so samesite cookies are sent. Ditto. > LayoutTests/http/tests/cookies/same-site/popup-from-iframe-same-site-with-post-form-expected.txt:4 > +PASS popup loaded as '127.0.0.1', then post navigation to 127.0.0.1, so samesite cookies are sent. Ditto, plus I would like if this test output was distinct from the one above. Could we add more details so that it's clear what's being tested? > LayoutTests/http/tests/cookies/same-site/popup-same-site-with-post-form-expected.txt:2 > +PASS popup opened as 'about:blank', then post navigation to 127.0.0.1, so samesite cookies are sent. Ditto on cookie details. > LayoutTests/http/tests/cookies/same-site/popup-same-site-with-post-form-expected.txt:3 > +PASS popup opened as '127.0.0.1', then post navigation to 127.0.0.1, so samesite cookies are sent. Ditto. > LayoutTests/http/tests/cookies/same-site/popup-same-site-with-post-form-expected.txt:4 > +PASS popup loaded as '127.0.0.1', then post navigation to 127.0.0.1, so samesite cookies are sent. Ditto, plus the comment on making it distinct.
youenn fablet
Comment 6 2022-03-23 00:57:53 PDT
(In reply to John Wilander from comment #5) > Comment on attachment 455381 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=455381&action=review > > r=me based on enhancing the tests (and getting green EWS bots). Updated the tests accordingly. > > LayoutTests/http/tests/cookies/same-site/popup-same-site-with-post-form-expected.txt:4 > > +PASS popup loaded as '127.0.0.1', then post navigation to 127.0.0.1, so samesite cookies are sent. > > Ditto, plus the comment on making it distinct. I updated the comment. In one case we are opening the popup on 127.0.0.1 and in the other case we are fully loading a page in 127.0.0.1, before doing post navigation.
youenn fablet
Comment 7 2022-03-23 00:58:06 PDT
Created attachment 455472 [details] Patch for landing
youenn fablet
Comment 8 2022-03-23 01:12:11 PDT
EWS
Comment 9 2022-03-23 02:05:56 PDT
Committed r291741 (248773@main): <https://commits.webkit.org/248773@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 455472 [details].
John Wilander
Comment 10 2022-03-23 16:12:00 PDT
Hmm. Are you sure SameSite=strict cookies should be sent in these cases?
youenn fablet
Comment 12 2022-03-23 23:43:08 PDT
(In reply to John Wilander from comment #10) > Hmm. Are you sure SameSite=strict cookies should be sent in these cases? Navigation is same origin (127.0.0.1 to 127.0.0.1) so SameSite=strict should do the same as SameSite=lax, no?
youenn fablet
Comment 13 2022-03-23 23:46:11 PDT
FWIW, Chrome and Firefox are also sending strict cookies for those tests. https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-cookie-same-site-00#section-4.1.1 seems to be about allowing same-site cookies in some cases of cross-origin top level navigations (for GET method).
Arcady Goldmints-Orlov
Comment 14 2022-04-27 13:47:18 PDT
*** Bug 227819 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.