Cookies are not updated on redirect request. #149

Closed
glennpratt opened this Issue Oct 22, 2012 · 9 comments

Comments

Projects
None yet
5 participants
Contributor

glennpratt commented Oct 22, 2012

I'm dealing with a single sign-on system that adds a cookie and redirects back to the current URL when you arrive on a satellite site you haven't visited recently.

Guzzle receives the new set cookie and adds it to the cookie jar correctly, but the request to the site doesn't have the cookie, so ends up in a redirect loop.

I've tried adding the cookies to the Request object a few different ways, but it doesn't appear any changes make it to cURL during the redirect.

If I catch the Maximum (5) redirects followed exception and re-issue the request, the cookie is now sent correctly and all is well.

<?php
    try {
      $response = $request->send();
    } catch (\Guzzle\Http\Exception\CurlException $e) {
      if (!$e->getErrorNo() == 47) {
        throw $e;
      }
      $response = $request->send();
    }
Contributor

glennpratt commented Oct 22, 2012

I'd be happy to make a patch here, but I don't know how or if it's possible to change the request during the redirect... perhaps Guzzle would need to handle redirects on it's own.

I assume if this issue could be fixed, #120 could handled seamlessly as well.

Owner

mtdowling commented Oct 22, 2012

Thanks for the report. I'd prefer not to implement redirect logic in Guzzle, so let's first see if cURL handles can be updated in the middle of a redirect. I bet that they can.

I think the best thing to do right now would be to create a branch and write a failing test using the node.js test server. We can then work on figuring out how we can update the cURL handle while it's transferring.

Contributor

glennpratt commented Oct 22, 2012

Hey, that was pretty painless: #150.

glennpratt added a commit to glennpratt/guzzle that referenced this issue Oct 23, 2012

Owner

mtdowling commented Oct 30, 2012

Well crap. Looks like cURL's CURLOPT_HEADERFUNCTION callback is invoked only after initiating a redirect request. There doesn't seem to be a way to intercept curl and update the curl handle before sending the redirect request.

Here's what I was working on, but it shows that the event is only emitted after the followup request is sent:

diff --git a/src/Guzzle/Http/Curl/CurlMulti.php b/src/Guzzle/Http/Curl/CurlMulti.php
index d324ed6..94fd537 100644
--- a/src/Guzzle/Http/Curl/CurlMulti.php
+++ b/src/Guzzle/Http/Curl/CurlMulti.php
@@ -352,7 +352,6 @@ class CurlMulti extends AbstractHasDispatcher implements CurlMultiInterface
         $wrapper = CurlHandle::factory($request);
         $this->handles->attach($request, $wrapper);
         $this->resourceHash[(int) $wrapper->getHandle()] = $request;
-        $request->getParams()->set('curl_handle', $wrapper);

         return $wrapper;
     }
@@ -387,12 +386,15 @@ class CurlMulti extends AbstractHasDispatcher implements CurlMultiInterface

             // Get messages from curl handles
             while ($done = curl_multi_info_read($this->multiHandle)) {
-                $request = $this->resourceHash[(int) $done['handle']];
-                $handle = $this->handles[$request];
-                try {
-                    $this->processResponse($request, $handle, $done);
-                } catch (\Exception $e) {
-                    $this->removeErroredRequest($request, $e);
+                $id = (int) $done['handle'];
+                if (isset($this->resourceHash[$id])) {
+                    $request = $this->resourceHash[$id];
+                    $handle = $this->handles[$request];
+                    try {
+                        $this->processResponse($request, $handle, $done);
+                    } catch (\Exception $e) {
+                        $this->removeErroredRequest($request, $e);
+                    }
                 }
             }

diff --git a/src/Guzzle/Http/Curl/RequestMediator.php b/src/Guzzle/Http/Curl/RequestMediator.php
index f7ca06b..b85bc08 100644
--- a/src/Guzzle/Http/Curl/RequestMediator.php
+++ b/src/Guzzle/Http/Curl/RequestMediator.php
@@ -43,6 +43,7 @@ class RequestMediator
     public function setCurlHandle(CurlHandle $handle)
     {
         $this->curlHandle = $handle;
+        $this->request->getParams()->set('curl_handle', $handle);

         return $this;
     }
diff --git a/src/Guzzle/Plugin/Cookie/CookiePlugin.php b/src/Guzzle/Plugin/Cookie/CookiePlugin.php
index 4686ce0..428d9f6 100644
--- a/src/Guzzle/Plugin/Cookie/CookiePlugin.php
+++ b/src/Guzzle/Plugin/Cookie/CookiePlugin.php
@@ -80,8 +80,40 @@ class CookiePlugin implements EventSubscriberInterface
      */
     public function onRequestReceiveStatusLine(Event $event)
     {
-        if ($event['previous_response']) {
-            $this->cookieJar->addCookiesFromResponse($event['previous_response']);
+        if (!($response = $event['previous_response'])) {
+            return;
+        }
+
+        $this->cookieJar->addCookiesFromResponse($response);
+        if ($response->isRedirect()) {
+
+            $request = $response->getRequest();
+            $event['request'] = $request;
+
+            // Re-Trigger the request.before_send to ensure cookies are updated
+            $this->onRequestBeforeSend($event);
+
+            if ($request->hasHeader('Cookie')) {
+                $cookie = (string) $request->getHeader('Cookie');
+
+                if ($handle = $request->getParams()->get('curl_handle')) {
+
+                    // Remove any existing cookie headers
+                    $headers = array_filter(
+                        $handle->getOptions()->get(CURLOPT_HTTPHEADER),
+                        function ($value) use ($cookie) {
+                            return strpos($value, 'Cookie:') === false;
+                        }
+                    );
+
+                    // Add the new cookie header
+                    $headers[] = "Cookie: {$cookie}";
+
+                    // Update the curl handle with the new cookie value
+                    curl_setopt($handle->getHandle(), CURLOPT_HTTPHEADER, $headers);
+                    fwrite(STDERR, 'Updating handle' . "\n\n");
+                }
+            }
         }
     }
 }
diff --git a/tests/Guzzle/Tests/Plugin/Cookie/CookiePluginTest.php b/tests/Guzzle/Tests/Plugin/Cookie/CookiePluginTest.php
index e979b39..1ecebb6 100644
--- a/tests/Guzzle/Tests/Plugin/Cookie/CookiePluginTest.php
+++ b/tests/Guzzle/Tests/Plugin/Cookie/CookiePluginTest.php
@@ -76,6 +76,9 @@ class CookiePluginTest extends \Guzzle\Tests\GuzzleTestCase
         ));

         $client = new Client($this->getServer()->getUrl());
+        $client->getConfig()->set('curl.options', array(
+            CURLOPT_VERBOSE => true
+        ));
         $client->getEventDispatcher()->addSubscriber($plugin);

         $request = $client->get();
@@ -90,7 +93,6 @@ class CookiePluginTest extends \Guzzle\Tests\GuzzleTestCase

         // Confirm subsequent requests have the cookie.
         $this->assertEquals('test=583551', $requests[2]->getHeader('Cookie'));
-
         // Confirm the redirected request has the cookie.
         $this->assertEquals('test=583551', $requests[1]->getHeader('Cookie'));
     }

Looks like Guzzle will need to handle redirects in PHP rather than cURL... Actually, this will be a good thing for Guzzle. It will make it easier to recover from redirect requests that cannot be rewound, will make it possible to support cookies in redirects, and will generally be easier to understand. It won't be easy though. We'll need to be very careful when implementing POST redirects and probably allow the same level of customization found in curl when dealing with POST redirects.

Just wondering what the status of this is? I'm currently using Guzzle 3.7.4 and the Cookie plugin and I'm not seeing my cookies being set on redirects. Was this supposed to be fixed?

Same than @greatwitenorth using guzzle 5.2.0: if allow_redirects is set to false, there is no cookie in reponse. See https://gist.github.com/mageekguy/d2c337d040b33e453563 for more information.

Debug mode show that cookies seems to be ignored in case of redirect:

* Hostname was found in DNS cache
* Hostname in DNS cache was stale, zapped
*   Trying 10.6.0.13...
* Connected to foo.bar (10.6.0.13) port 80 (#27)
> POST /index.php HTTP/1.1
Host: foo.bar
User-Agent: Guzzle/5.2.0 curl/7.35.0 PHP/5.5.9-1ubuntu4.5
Content-Type: application/x-www-form-urlencoded
Content-Length: 54

* upload completely sent off: 54 out of 54 bytes
< HTTP/1.1 302 Found
< Date: Mon, 16 Feb 2015 11:25:02 GMT
* Server Apache is not blacklisted
< Server: Apache
< Location: admin/index.php
< Content-Length: 0
< Content-Type: 'text/html'
< 
* Connection #27 to host foo.bar left intact
* Found bundle for host foo.bar: 0x7f2744d389a0
* Re-using existing connection! (#27) with host foo.bar
* Connected to foo.bar (10.6.0.13) port 80 (#27)
> GET /admin/index.php HTTP/1.1
Host: foo.bar
User-Agent: Guzzle/5.2.0 curl/7.35.0 PHP/5.5.9-1ubuntu4.5
Content-Type: application/x-www-form-urlencoded

< HTTP/1.1 200 OK
< Date: Mon, 16 Feb 2015 11:25:02 GMT
* Server Apache is not blacklisted
< Server: Apache
< P3P: CP="CUR ADM OUR NOR STA NID"
< Set-Cookie: sessionID=phpads54e1d38ed5b760-97980772; path=/; domain=foo.bar
< Transfer-Encoding: chunked
< Content-Type: text/html; charset=ISO-8859-1
< 
* Connection #27 to host foo.bar left intact

Ok, guys, sorry for the noise, seems that i'm using the bad URL...
All my apologize.

egeogretmen commented Mar 15, 2017

When I use default options for curl while initializing Guzzle, I see that Guzzle fails to update the cookies when a Set-Cookie header is sent in a 302 redirect. As you can see below 302's Set-Cookie header is different than the Cookie header in the next request (which is the cookie that was set before by making a simple GET request to the server).

> POST /reservation/ibe/loginProcess?locale=en_US&TKN_EXCHG=0585E6DE50F5B9A3A3B185D213A7DC2E HTTP/1.1
Host: www.some-host.com
User-Agent: GuzzleHttp/6.0.1 curl/7.47.0 PHP/5.6.30-1+deb.sury.org~xenial+1
Content-Type: application/x-www-form-urlencoded
Cookie: JSESSIONID=681AEC0665200BC5640D48A74B896FE2.ibe1; BIGipServeriflyres_SE_STG_IBE_Pool=!+gz6Q3H45Xe+ptySQzWgdfD35L+b1VYxR8kXs/WmjXnqtsTp9ffKpEqqWtGow8Oip6Y2AeuNrvkdJQ==
Content-Length: 151

* upload completely sent off: 151 out of 151 bytes
< HTTP/1.1 302 Found
< Date: Wed, 15 Mar 2017 06:52:28 GMT
< Server: Apache
< Location: https://www.some-host.com/reservation/ibe/agent
< Content-Length: 0
< Set-Cookie: JSESSIONID=17DFF3EEF4D5403424248A7733CF8D97.ibe1; Path=/reservation/; Secure; HttpOnly
< 
* Connection #0 to host www.some-host.com left intact
* Found bundle for host www.some-host.com: 0x5627fa06cee0 [can pipeline]
* Re-using existing connection! (#0) with host www.some-host.com
* Connected to www.some-host.com (X.X.X.X) port 443 (#0)
> GET /reservation/ibe/agent HTTP/1.1
Host: www.some-host.com
User-Agent: GuzzleHttp/6.0.1 curl/7.47.0 PHP/5.6.30-1+deb.sury.org~xenial+1
Content-Type: application/x-www-form-urlencoded
Cookie: JSESSIONID=681AEC0665200BC5640D48A74B896FE2.ibe1; BIGipServeriflyres_SE_STG_IBE_Pool=!+gz6Q3H45Xe+ptySQzWgdfD35L+b1VYxR8kXs/WmjXnqtsTp9ffKpEqqWtGow8Oip6Y2AeuNrvkdJQ==

It is suggested in some places to set CURLOPT_COOKIEFILE option so that I guess curl handles the cookies. This time I saw that there were multiple Cookie headers which may somewhat be related to #124 . Remote host this time gives 404 when it encounters with multiple Cookie headers.

> POST /reservation/ibe/loginProcess?locale=en_US&TKN_EXCHG=E4AC7267457F52A18C29C99F3A21862C HTTP/1.1
Host: www.some-host.com
Cookie: JSESSIONID=9EE5FDB6EDA35F4260492A1A660A674E.ibe1; BIGipServeriflyres_SE_STG_IBE_Pool=!gCl4EaZRZuEY5luSQzWgdfD35L+b1RlT6Zj21R2AXXKUrsJRw4OHoFj00NlkygJ4Tdx0D/MIDS6NdQ==
User-Agent: GuzzleHttp/6.0.1 curl/7.47.0 PHP/5.6.30-1+deb.sury.org~xenial+1
Content-Type: application/x-www-form-urlencoded
Cookie: JSESSIONID=9EE5FDB6EDA35F4260492A1A660A674E.ibe1; BIGipServeriflyres_SE_STG_IBE_Pool=!gCl4EaZRZuEY5luSQzWgdfD35L+b1RlT6Zj21R2AXXKUrsJRw4OHoFj00NlkygJ4Tdx0D/MIDS6NdQ==
Content-Length: 151

* upload completely sent off: 151 out of 151 bytes
< HTTP/1.1 404 Not Found
< Date: Wed, 15 Mar 2017 06:47:22 GMT
< Server: Apache
< Content-Length: 0
* Replaced cookie JSESSIONID="E9E0B682A7ED6729EFE710C8F5657434.ibe2" for domain www.some-host.com, path /reservation/, expire 0
< Set-Cookie: JSESSIONID=E9E0B682A7ED6729EFE710C8F5657434.ibe2; Path=/reservation/; Secure; HttpOnly
* Replaced cookie BIGipServeriflyres_SE_STG_IBE_Pool="!8F4/7nbjZpG92zySQzWgdfD35L+b1cziC9ySzNEgVng80dGUrg4IgyeGoWB7QC0JbPXn1e7T7wXfDmE=" for domain www.some-host.com, path /, expire 0
< Set-Cookie: BIGipServeriflyres_SE_STG_IBE_Pool=!8F4/7nbjZpG92zySQzWgdfD35L+b1cziC9ySzNEgVng80dGUrg4IgyeGoWB7QC0JbPXn1e7T7wXfDmE=; path=/; Httponly
< 
* Connection #0 to host www.some-host.com left intact

I was only be able to make this redirect properly by enabling CURLOPT_COOKIEFILE and at the same time setting 'cookies' option to false while initializing the Guzzle Client. I don't know if this is intended, but want to report it anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment