Bug 64575

Summary: [soup] "Too many redirects" error loading chat in plus.google.com
Product: WebKit Reporter: Xan Lopez <xan.lopez>
Component: WebKitGTKAssignee: Sergio Villar Senin <svillar>
Status: RESOLVED FIXED    
Severity: Normal CC: a.radke, cdumez, cgarcia, danw, dino, d-r, gustavo, mrobinson, rakuco, sa, svillar, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 104475    
Attachments:
Description Flags
Forward cookies on redirects
none
Patch
none
Patch mrobinson: review+

Xan Lopez
Reported 2011-07-14 17:13:37 PDT
Google+ supposedly has a built-in chat feature (like GMail). In Ephy all I get is a "Too many redirect errors". Checking the Inspector the page apparently tries to do a GET of some insanely complex talkgadget.google.com URI (can't copy&paste though? another bug? :)).
Attachments
Forward cookies on redirects (735 bytes, patch)
2012-10-04 11:17 PDT, Sergio Villar Senin
no flags
Patch (1.62 KB, patch)
2013-02-19 11:16 PST, Sergio Villar Senin
no flags
Patch (4.88 KB, patch)
2013-02-25 07:36 PST, Sergio Villar Senin
mrobinson: review+
Martin Robinson
Comment 1 2011-07-14 18:05:57 PDT
I saw this on Google Calendar some time ago as well. Gustavo recently fixed an issue where Google Calendar was detecting WebKitGTK+ as mobile. I wonder if that's happening here as well.
Sven Arvidsson
Comment 2 2011-08-14 14:08:47 PDT
I get the same error when I try to reach http://www.sjscycles.co.uk/ in Epiphany. Is this the same problem or should I file a new bug?
a.radke
Comment 3 2012-04-01 08:00:43 PDT
This nasty issue is still present in libwebkit 1.8. As Chromium is working this could be part of the gtk framwork?
Martin Robinson
Comment 4 2012-04-01 19:38:41 PDT
(In reply to comment #2) > I get the same error when I try to reach http://www.sjscycles.co.uk/ in Epiphany. Is this the same problem or should I file a new bug? I confirmed that this fails in Epiphany and works great in GtkLauncher, so I suspect that this is a user-agent problem.
Xan Lopez
Comment 5 2012-04-02 00:44:05 PDT
(In reply to comment #4) > I confirmed that this fails in Epiphany and works great in GtkLauncher, so I suspect that this is a user-agent problem. We already pretend (in theory) to be Google Chrome, what else should be do here?
Sergio Villar Senin
Comment 6 2012-04-02 01:33:42 PDT
(In reply to comment #5) > (In reply to comment #4) > > I confirmed that this fails in Epiphany and works great in GtkLauncher, so I suspect that this is a user-agent problem. > > We already pretend (in theory) to be Google Chrome, what else should be do here? Actually it's not an UA issue. It's easy to test with curl or wget that the UA reported by ephy is perfectly fine. The problem (at least for me in the www.sjscycles.co.uk site) is that we're sending a Cookie with invalid characters for the IIS server. I'll take a look.
Sergio Villar Senin
Comment 7 2012-04-02 01:58:50 PDT
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > I confirmed that this fails in Epiphany and works great in GtkLauncher, so I suspect that this is a user-agent problem. > > > > We already pretend (in theory) to be Google Chrome, what else should be do here? > > Actually it's not an UA issue. It's easy to test with curl or wget that the UA reported by ephy is perfectly fine. The problem (at least for me in the www.sjscycles.co.uk site) is that we're sending a Cookie with invalid characters for the IIS server. I'll take a look. So yeah, it turned out that I have some .co.uk cookies in the ephy cookie jar that were confusing the server. After I removed them everything worked fine. So here the real bug is IMO that we are storing "supercookies" http://en.wikipedia.org/wiki/HTTP_cookie#Supercookie.
Sergio Villar Senin
Comment 8 2012-04-09 15:42:28 PDT
This bug is now handled in libsoup: https://bugzilla.gnome.org/show_bug.cgi?id=673802
Sergio Villar Senin
Comment 9 2012-09-21 02:02:33 PDT
The bug is still there. I'll take a look at it again.
Sergio Villar Senin
Comment 10 2012-09-26 10:06:56 PDT
OK, so after examining carefully the HTTP communications I can conclude that the problem here are cookies. Basically Google+ tries to set some cookies with domain talkgadget.google.com from a plus.google.com domain, which we basically reject because they don't pass the domain match test. Thing is that Chrome or FF accept them without any problem...
Sergio Villar Senin
Comment 11 2012-09-27 06:08:14 PDT
(In reply to comment #10) > OK, so after examining carefully the HTTP communications I can conclude that the problem here are cookies. Basically Google+ tries to set some cookies with domain talkgadget.google.com from a plus.google.com domain, which we basically reject because they don't pass the domain match test. > > Thing is that Chrome or FF accept them without any problem... I think I know the reason, and the problem is that we don't properly handle the case of multiple redirects, as the firstPartyForCookies is not updated properly.
Sergio Villar Senin
Comment 12 2012-10-04 09:33:38 PDT
OK, I have a one-liner fix for this issue. Apparently tests are still working fine, but before posting it here I need to double-check if there is any test covering this case.
Sergio Villar Senin
Comment 13 2012-10-04 11:17:31 PDT
Created attachment 167139 [details] Forward cookies on redirects So that's what I'm talking about. Basically that page requires some cookies to be forwarded for a proper authentication. Honestly I am not sure about if doing something like this is either standard complaint and/or secure. Dan any comment? PS: not setting the r? because I this needs a tests, a proper ChangeLog, blah blah
Dan Winship
Comment 14 2012-10-04 11:20:59 PDT
There's no standard for "first party cookies", but this change definitely seems right to me
Sergio Villar Senin
Comment 15 2013-02-19 11:16:45 PST
Martin Robinson
Comment 16 2013-02-19 11:18:19 PST
Comment on attachment 189119 [details] Patch Is it possible to get a test for this?
Sergio Villar Senin
Comment 17 2013-02-25 07:36:58 PST
Martin Robinson
Comment 18 2013-02-25 07:50:03 PST
Comment on attachment 190057 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190057&action=review Nice. Just a couple small things before landing. > LayoutTests/http/tests/cookies/resources/redirect.php:6 > + if ($_GET['step'] == 1) > + { > + header("HTTP/1.0 302 Found"); > + header('Location: http://localhost:8000/cookies/resources/redirect.php?step=2'); > + } Do you mind converting this file to use WebKit style when landing? I also recommend giving it a slightly more descriptive name. Since it isn't shared, maybe you can just name it after the test? > LayoutTests/http/tests/cookies/resources/redirect.php:18 > + echo "<p>PASSED: Cookie successfully set</p>\n"; I'm not sure the paragraph tag is necessary here. > LayoutTests/http/tests/cookies/resources/redirect.php:22 > + echo "<p>FAILED: Cookie not set</p>\n"; Ditto.
Sergio Villar Senin
Comment 19 2013-02-25 08:58:14 PST
Alexey Proskuryakov
Comment 20 2013-02-25 21:35:01 PST
This regression test breaks other cookie tests, filed bug 110844.
Dean Jackson
Comment 21 2013-03-06 17:13:39 PST
I think this is still causing some flakiness on Mac. http/tests/cookies/third-party-cookie-relaxing.html http/tests/plugins/third-party-cookie-accept-policy.html I filed https://bugs.webkit.org/show_bug.cgi?id=111650
Note You need to log in before you can comment on or make changes to this bug.