Bug 217010

Summary: [SOUP] Do not set site for cookies twice in case of redirection
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, bugs-noreply, ews-watchlist, gustavo, mcatanzaro, pgriffis
Priority: P2 Keywords: Gtk
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch mcatanzaro: review+

Description Carlos Garcia Campos 2020-09-26 00:59:58 PDT 
ResourceRequest::updateSoupMessage() already fills the same site information, but we are duplicating the code in NetworkDataTaskSoup::continueHTTPRedirection()
Comment 1 Carlos Garcia Campos 2020-09-26 01:03:02 PDT 
Created attachment 409771 [details]
Patch
Comment 2 Michael Catanzaro 2020-09-26 04:47:18 PDT 
Comment on attachment 409771 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=409771&action=review

> Source/WebCore/platform/network/soup/ResourceRequestSoup.cpp:115
> +    if (!isSameSiteUnspecified()) {

Is it OK for SameSite to ever be unspecified when we send a message? Maybe we should have an assert to ensure that doesn't happen?
Comment 3 Carlos Garcia Campos 2020-09-26 06:49:53 PDT 
(In reply to Michael Catanzaro from comment #2)
> Comment on attachment 409771 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=409771&action=review
> 
> > Source/WebCore/platform/network/soup/ResourceRequestSoup.cpp:115
> > +    if (!isSameSiteUnspecified()) {
> 
> Is it OK for SameSite to ever be unspecified when we send a message? Maybe
> we should have an assert to ensure that doesn't happen?

I don't know.
Comment 4 Carlos Garcia Campos 2020-09-26 06:52:42 PDT 
Committed r267621: <https://trac.webkit.org/changeset/267621>
Comment 5 Michael Catanzaro 2020-09-26 09:00:00 PDT 
(In reply to Michael Catanzaro from comment #2)
> Is it OK for SameSite to ever be unspecified when we send a message? Maybe
> we should have an assert to ensure that doesn't happen?

Hm, I was thinking: "can we assert that same site data has been provided when needed?" But I think this actually is just used to indicate SameSite is not specified, rather than being set with SameSite=Lax, SameSite=Strict, or SameSite=None.