Bug 113208 - [Chromium] Add WebURLRequest::setIgnoreResponseCookies().
Summary: [Chromium] Add WebURLRequest::setIgnoreResponseCookies().
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-25 07:58 PDT by Philippe Liard
Modified: 2013-04-08 17:03 PDT (History)
10 users (show)

See Also:


Attachments
Patch (6.10 KB, patch)
2013-03-25 08:04 PDT, Philippe Liard
levin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Liard 2013-03-25 07:58:10 PDT
This method will be used in Chromium to send cookies in the favicon request but without processing them in the response to avoid side effects.

See this bug on the Chromium side: https://code.google.com/p/chromium/issues/detail?id=223412.
Comment 1 Philippe Liard 2013-03-25 08:04:53 PDT
Created attachment 194860 [details]
Patch
Comment 2 WebKit Review Bot 2013-03-25 08:08:49 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 3 Darin Fisher (:fishd, Google) 2013-03-25 08:35:59 PDT
Are you sure this won't lead to a bunch of "infinite" redirects?  When a cookie is not set, or needs to be updates, sites will commonly issues a redirect with a set-cookie header and a location back to the original URL.  If the cookie is not sent in the subsequent request, the server will issue the redirect again.  This repeats until the browser decides that it has followed too many redirects.  This sounds costly, especially if it were to happen a lot.
Comment 4 David Levin 2013-03-25 08:59:07 PDT
I would recommend against modifying ResourceRequestBase unless you plan to modify all ports to support it.

Consider doing it like WebURLRequest::setAllowStoredCredentials does instead where it doesn't need to touch ResourceRequestBase.

Also, Darin makes a fair point which I didn't consider...Maybe this is harder than I first thought. :(
Comment 5 Philippe Liard 2013-03-25 09:26:45 PDT
Good point Darin. I wasn't aware of this mechanism. If I understand correctly, this means that we are already broken in that sense since we completely disable cookies for the favicon request (since r181483), right?
Comment 6 Philippe Liard 2013-03-25 09:54:00 PDT
Would it make sense to detect a redirect response to the original URL (redirect loop) and fallback to enabling cookies in that case to break the loop (only for favicon requests)? Or does this sound too complicated in terms of expected behavior?
Comment 7 Darin Fisher (:fishd, Google) 2013-03-25 09:59:55 PDT
(In reply to comment #5)
> Good point Darin. I wasn't aware of this mechanism. If I understand correctly, this means that we are already broken in that sense since we completely disable cookies for the favicon request (since r181483), right?

Yeah, I don't think that was the correct solution either, but I could be wrong.  There seems to be a variety of complications.  Some of these might just stem from how our (Chromium's) favicon loading is already a bit different than normal resource loading?  Maybe making it less different would be an alternative / better way to fix the set of issues?

Given the behavior of other browsers as detailed in https://code.google.com/p/chromium/issues/detail?id=114082, perhaps there are other solutions?
Comment 8 Philippe Liard 2013-03-25 10:05:53 PDT
Yes, I will revisit the list of solutions and will continue the discussion on the crbug.

Thanks.
Comment 9 David Levin 2013-03-25 10:22:12 PDT
Comment on attachment 194860 [details]
Patch

r- for now per discussion in bug.