From 48227d4caa97a78091c933c5dbc8406293be3905 Mon Sep 17 00:00:00 2001
From: Beanow <beanow@oscp.info>
Date: Fri, 27 Feb 2015 04:00:22 +0100
Subject: [PATCH 1/6] Implemented first parts of followRedirects support.

---
 CHANGELOG.md                 |  5 ++
 README.md                    |  8 +++-
 src/Client.php               | 21 +++------
 src/ConnectorPair.php        | 22 +++++++++
 src/Factory.php              |  3 +-
 src/Request.php              | 73 ++++++++++++++++++++++++-----
 src/RequestData.php          | 26 +++++++++++
 src/RequestOptions.php       | 89 ++++++++++++++++++++++++++++++++++++
 tests/RequestOptionsTest.php | 84 ++++++++++++++++++++++++++++++++++
 tests/RequestTest.php        | 50 +++++++++++++++-----
 10 files changed, 342 insertions(+), 39 deletions(-)
 create mode 100644 src/ConnectorPair.php
 create mode 100644 src/RequestOptions.php
 create mode 100644 tests/RequestOptionsTest.php

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8feb3be..2ab07c3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Changelog
 
+## 0.5.0 (2015-02-27)
+
+* New feature: follow redirect option
+* BC break: `Client` and `Request` take different constructor parameters.
+
 ## 0.4.0 (2014-02-02)
 
 * BC break: Drop unused `Response::getBody()`
diff --git a/README.md b/README.md
index e4f0827..fcf7a4f 100644
--- a/README.md
+++ b/README.md
@@ -49,11 +49,17 @@ $request->on('response', function ($response) {
 });
 $request->end();
 $loop->run();
+
+?>
 ```
 
+## Work in progress
+
+* followRedirects option
+* maxRedirects option
+
 ## TODO
 
 * gzip content encoding
 * chunked transfer encoding
 * keep-alive connections
-* following redirections
diff --git a/src/Client.php b/src/Client.php
index 456564c..a500826 100644
--- a/src/Client.php
+++ b/src/Client.php
@@ -2,29 +2,20 @@
 
 namespace React\HttpClient;
 
-use React\SocketClient\ConnectorInterface;
-
 class Client
 {
-    private $connector;
-    private $secureConnector;
+    private $connectorPair;
 
-    public function __construct(ConnectorInterface $connector, ConnectorInterface $secureConnector)
+    public function __construct(ConnectorPair $connectorPair)
     {
-        $this->connector = $connector;
-        $this->secureConnector = $secureConnector;
+        $this->connectorPair = $connectorPair;
     }
 
-    public function request($method, $url, array $headers = [])
+    public function request($method, $url, array $headers = [], array $options = null)
     {
         $requestData = new RequestData($method, $url, $headers);
-        $connector = $this->getConnectorForScheme($requestData->getScheme());
-
-        return new Request($connector, $requestData);
-    }
+        $requestOptions = new RequestOptions($options);
 
-    private function getConnectorForScheme($scheme)
-    {
-        return ('https' === $scheme) ? $this->secureConnector : $this->connector;
+        return new Request($this->connectorPair, $requestData, $requestOptions);
     }
 }
diff --git a/src/ConnectorPair.php b/src/ConnectorPair.php
new file mode 100644
index 0000000..6e7c717
--- /dev/null
+++ b/src/ConnectorPair.php
@@ -0,0 +1,22 @@
+<?php
+
+namespace React\HttpClient;
+
+use React\SocketClient\ConnectorInterface;
+
+class ConnectorPair
+{
+    private $connector;
+    private $secureConnector;
+
+    public function __construct(ConnectorInterface $connector, ConnectorInterface $secureConnector)
+    {
+        $this->connector = $connector;
+        $this->secureConnector = $secureConnector;
+    }
+
+    public function getConnectorForScheme($scheme)
+    {
+        return ('https' === $scheme) ? $this->secureConnector : $this->connector;
+    }
+}
diff --git a/src/Factory.php b/src/Factory.php
index b9785f0..40b798d 100644
--- a/src/Factory.php
+++ b/src/Factory.php
@@ -13,7 +13,8 @@ public function create(LoopInterface $loop, Resolver $resolver)
     {
         $connector = new Connector($loop, $resolver);
         $secureConnector = new SecureConnector($connector, $loop);
+        $connectorPair = new ConnectorPair($connector, $secureConnector);
 
-        return new Client($connector, $secureConnector);
+        return new Client($connectorPair);
     }
 }
diff --git a/src/Request.php b/src/Request.php
index 8dbd150..94b86d2 100644
--- a/src/Request.php
+++ b/src/Request.php
@@ -4,7 +4,6 @@
 
 use Evenement\EventEmitterTrait;
 use Guzzle\Parser\Message\MessageParser;
-use React\SocketClient\ConnectorInterface;
 use React\Stream\WritableStreamInterface;
 
 /**
@@ -24,7 +23,9 @@ class Request implements WritableStreamInterface
     const STATE_END = 3;
 
     private $connector;
+    private $connectorPair;
     private $requestData;
+    private $requestOptions;
 
     private $stream;
     private $buffer;
@@ -32,10 +33,17 @@ class Request implements WritableStreamInterface
     private $response;
     private $state = self::STATE_INIT;
 
-    public function __construct(ConnectorInterface $connector, RequestData $requestData)
+    private $redirectCount;
+    private $redirectLocations;
+
+    public function __construct(ConnectorPair $connectorPair, RequestData $requestData, RequestOptions $requestOptions)
     {
-        $this->connector = $connector;
+        $this->connectorPair = $connectorPair;
         $this->requestData = $requestData;
+        $this->requestOptions = $requestOptions;
+        $this->connector = $connectorPair->getConnectorForScheme($requestData->getScheme());
+        $this->redirectCount = 0;
+        $this->redirectLocations = [$requestData->getUrl()];
     }
 
     public function isWritable()
@@ -50,12 +58,11 @@ public function writeHead()
         }
 
         $this->state = self::STATE_WRITING_HEAD;
-
         $requestData = $this->requestData;
         $streamRef = &$this->stream;
         $stateRef = &$this->state;
 
-        $this
+        return $this
             ->connect()
             ->then(
                 function ($stream) use ($requestData, &$streamRef, &$stateRef) {
@@ -89,12 +96,11 @@ public function write($data)
             return $this->stream->write($data);
         }
 
-        $this->on('headers-written', function ($this) use ($data) {
-            $this->write($data);
-        });
-
         if (self::STATE_WRITING_HEAD > $this->state) {
-            $this->writeHead();
+            $this->writeHead()
+                ->then(function () use ($data) {
+                    $this->stream->write($data);
+                });
         }
 
         return false;
@@ -108,7 +114,7 @@ public function end($data = null)
 
         if (null !== $data) {
             $this->write($data);
-        } else if (self::STATE_WRITING_HEAD > $this->state) {
+        } elseif (self::STATE_WRITING_HEAD > $this->state) {
             $this->writeHead();
         }
     }
@@ -132,6 +138,45 @@ public function handleData($data)
             $this->stream->removeListener('end', array($this, 'handleEnd'));
             $this->stream->removeListener('error', array($this, 'handleError'));
 
+            //Should we respond to any redirects?
+            if ($this->isRedirectCode($response->getCode())
+                && $this->requestOptions->shouldFollowRedirects()) {
+
+                //Have we reached our maximum redirects?
+                if ($this->requestOptions->getMaxRedirects() > 0
+                    && $this->redirectCount >= $this->requestOptions->getMaxRedirects()) {
+                    $this->closeError(new \RuntimeException(
+                        sprintf("Too many redirects: %u", $this->redirectCount)
+                    ));
+                }
+
+                //Is the location a cyclic redirect?
+                $headers = $response->getHeaders();
+                if (in_array($headers['Location'], $this->redirectLocations)) {
+                    $this->closeError(new \RuntimeException(
+                        "Cyclic redirect detected"
+                    ));
+                }
+
+                //Store the next location to prevent cyclic redirects.
+                $this->redirectLocations[] = $headers['Location'];
+                $this->redirectCount++;
+
+                //Recalibrate to this new location.
+                $this->requestData->redirect($response->getCode(), $headers['Location']);
+                $this->connector = $this->connectorPair->getConnectorForScheme($this->requestData->getScheme());
+
+                //Clean up and rewind.
+                $this->stream->close();
+                $this->responseFactory = null;
+                $this->state = self::STATE_INIT;
+
+                //Perform the same tricks.
+                $this->end();
+
+                return;
+            }
+
             $this->response = $response;
 
             $response->on('end', function () {
@@ -218,6 +263,12 @@ protected function connect()
             ->create($host, $port);
     }
 
+    protected function isRedirectCode($code)
+    {
+        //Note: 303 status is currently unsupported.
+        return in_array($code, [301, 302, 307, 308]);
+    }
+
     public function setResponseFactory($factory)
     {
         $this->responseFactory = $factory;
diff --git a/src/RequestData.php b/src/RequestData.php
index 7073ce5..e908cc2 100644
--- a/src/RequestData.php
+++ b/src/RequestData.php
@@ -32,6 +32,11 @@ private function mergeDefaultheaders(array $headers)
         );
     }
 
+    public function getUrl()
+    {
+        return $this->url;
+    }
+
     public function getScheme()
     {
         return parse_url($this->url, PHP_URL_SCHEME);
@@ -78,4 +83,25 @@ public function __toString()
 
         return $data;
     }
+
+    /**
+     * Processes a redirect request, updating internal values to represent the new request data.
+     *
+     * @param integer $code     HTTP status code for the redirect.
+     * @param string  $location The Location header received.
+     */
+    public function redirect($code, $location)
+    {
+        switch ($code) {
+            case 301:
+            case 302:
+            case 307:
+            case 308:
+                $this->url = $location;
+                break;
+
+            default:
+                throw new InvalidArgumentException(sprintf("Redirect code %u is not supported", $code));
+        }
+    }
 }
diff --git a/src/RequestOptions.php b/src/RequestOptions.php
new file mode 100644
index 0000000..0a1a346
--- /dev/null
+++ b/src/RequestOptions.php
@@ -0,0 +1,89 @@
+<?php
+
+namespace React\HttpClient;
+
+use InvalidArgumentException;
+
+class RequestOptions
+{
+    /**
+     * Whether or not to follow redirects for this requests.
+     *
+     * @var boolean
+     */
+    private $followRedirects = false;
+
+    /**
+     * Maximum amount of redirects.
+     * Note: -1 is unlimited, 0 means no redirects will be accepted.
+     *
+     * @var integer
+     */
+    private $maxRedirects = 5;
+
+    /**
+     * Creates a new instance.
+     *
+     * @param array|null $options The optionally provided options.
+     */
+    public function __construct(array $options = null)
+    {
+        if (!empty($options)) {
+            $this->parseOptions($options);
+        }
+    }
+
+    /**
+     * This will enforce only known options are used and in the right format.
+     *
+     * @param array $options The provided options.
+     */
+    protected function parseOptions(array $options)
+    {
+        foreach ($options as $option => $value) {
+            switch ($option) {
+                case 'followRedirects':
+                    if (!is_bool($value)) {
+                        throw new InvalidArgumentException('Option "followRedirects" should be a boolean');
+                    }
+                    $this->followRedirects = $value;
+                    break;
+
+                case 'maxRedirects':
+                    if (!is_int($value)) {
+                        throw new InvalidArgumentException('Option "maxRedirects" should be an integer');
+                    }
+                    if ($value < -1) {
+                        throw new InvalidArgumentException('Option "maxRedirects" should be -1 or greater');
+                    }
+                    $this->maxRedirects = $value;
+                    break;
+
+                default:
+                    throw new InvalidArgumentException(sprintf('Unknown option "%s"', $option));
+                    break;
+            }
+        }
+    }
+
+    /**
+     * Whether or not to follow redirects for this requests.
+     *
+     * @return boolean
+     */
+    public function shouldFollowRedirects()
+    {
+        return $this->followRedirects;
+    }
+
+    /**
+     * Maximum amount of redirects.
+     * Note: -1 is unlimited, 0 means no redirects will be accepted.
+     *
+     * @return integer
+     */
+    public function getMaxRedirects()
+    {
+        return $this->maxRedirects;
+    }
+}
diff --git a/tests/RequestOptionsTest.php b/tests/RequestOptionsTest.php
new file mode 100644
index 0000000..bcf89ff
--- /dev/null
+++ b/tests/RequestOptionsTest.php
@@ -0,0 +1,84 @@
+<?php
+
+namespace React\Tests\HttpClient;
+
+use React\HttpClient\RequestOptions;
+
+class RequestOptionsTest extends TestCase
+{
+    /** @test */
+    public function requestOptionsShouldConstructWithoutOptions()
+    {
+        new RequestOptions();
+    }
+
+    /**
+     * @test
+     * @expectedException InvalidArgumentException
+     * @expectedExceptionMessage Unknown option "I'm a teapot"
+     */
+    public function requestOptionsShouldRejectUnknownOptions()
+    {
+        new RequestOptions(array(
+            "I'm a teapot" => true,
+        ));
+    }
+
+    /** @test */
+    public function requestOptionsShouldSupportFollowRedirectsOption()
+    {
+        //Default value should be FALSE.
+        $requestOptions = new RequestOptions();
+        $this->assertFalse($requestOptions->shouldFollowRedirects());
+
+        //Providing a different value should be respected.
+        $requestOptions = new RequestOptions(array(
+            'followRedirects' => true,
+        ));
+        $this->assertTrue($requestOptions->shouldFollowRedirects());
+
+        //Should check for boolean type.
+        $this->setExpectedException(
+          'InvalidArgumentException',
+          'Option "followRedirects" should be a boolean'
+        );
+        new RequestOptions(array(
+            'followRedirects' => 7,
+        ));
+    }
+
+    /** @test */
+    public function requestOptionsShouldSupportMaxRedirectsOption()
+    {
+        //Default value should be 5.
+        $requestOptions = new RequestOptions();
+        $this->assertSame(5, $requestOptions->getMaxRedirects());
+
+        //Providing a different value should be respected.
+        $requestOptions = new RequestOptions(array(
+            'maxRedirects' => 42,
+        ));
+        $this->assertSame(42, $requestOptions->getMaxRedirects());
+
+        //Should check for integer type.
+        $this->setExpectedException(
+          'InvalidArgumentException',
+          'Option "maxRedirects" should be an integer'
+        );
+        new RequestOptions(array(
+            'maxRedirects' => 7.5,
+        ));
+    }
+
+    /**
+     * @test
+     * @expectedException InvalidArgumentException
+     * @expectedExceptionMessage Option "maxRedirects" should be -1 or greater
+     */
+    public function requestOptionsShouldCheckValidMaxRedirectsRange()
+    {
+        new RequestOptions(array(
+            'maxRedirects' => -3,
+        ));
+    }
+}
diff --git a/tests/RequestTest.php b/tests/RequestTest.php
index 5d2d595..d8899eb 100644
--- a/tests/RequestTest.php
+++ b/tests/RequestTest.php
@@ -2,8 +2,10 @@
 
 namespace React\Tests\HttpClient;
 
+use React\HttpClient\ConnectorPair;
 use React\HttpClient\Request;
 use React\HttpClient\RequestData;
+use React\HttpClient\RequestOptions;
 use React\Stream\Stream;
 use React\Promise\FulfilledPromise;
 use React\Promise\RejectedPromise;
@@ -30,7 +32,9 @@ public function setUp()
     public function requestShouldBindToStreamEventsAndUseconnector()
     {
         $requestData = new RequestData('GET', 'http://www.example.com');
-        $request = new Request($this->connector, $requestData);
+        $requestOptions = new RequestOptions();
+        $connectorPair = new ConnectorPair($this->connector, $this->connector);
+        $request = new Request($connectorPair, $requestData, $requestOptions);
 
         $this->successfulConnectionMock();
 
@@ -74,6 +78,10 @@ public function requestShouldBindToStreamEventsAndUseconnector()
             ->with('data', array('body'));
 
         $response->expects($this->at(0))
+            ->method('getCode')
+            ->will($this->returnValue(200));
+
+        $response->expects($this->at(1))
             ->method('on')
             ->with('end', $this->anything())
             ->will($this->returnCallback(function ($event, $cb) use (&$endCallback) {
@@ -120,7 +128,9 @@ public function requestShouldBindToStreamEventsAndUseconnector()
     public function requestShouldEmitErrorIfConnectionFails()
     {
         $requestData = new RequestData('GET', 'http://www.example.com');
-        $request = new Request($this->connector, $requestData);
+        $requestOptions = new RequestOptions();
+        $connectorPair = new ConnectorPair($this->connector, $this->connector);
+        $request = new Request($connectorPair, $requestData, $requestOptions);
 
         $this->rejectedConnectionMock();
 
@@ -153,7 +163,9 @@ public function requestShouldEmitErrorIfConnectionFails()
     public function requestShouldEmitErrorIfConnectionEndsBeforeResponseIsParsed()
     {
         $requestData = new RequestData('GET', 'http://www.example.com');
-        $request = new Request($this->connector, $requestData);
+        $requestOptions = new RequestOptions();
+        $connectorPair = new ConnectorPair($this->connector, $this->connector);
+        $request = new Request($connectorPair, $requestData, $requestOptions);
 
         $this->successfulConnectionMock();
 
@@ -187,7 +199,9 @@ public function requestShouldEmitErrorIfConnectionEndsBeforeResponseIsParsed()
     public function requestShouldEmitErrorIfConnectionEmitsError()
     {
         $requestData = new RequestData('GET', 'http://www.example.com');
-        $request = new Request($this->connector, $requestData);
+        $requestOptions = new RequestOptions();
+        $connectorPair = new ConnectorPair($this->connector, $this->connector);
+        $request = new Request($connectorPair, $requestData, $requestOptions);
 
         $this->successfulConnectionMock();
 
@@ -221,7 +235,9 @@ public function requestShouldEmitErrorIfConnectionEmitsError()
     public function postRequestShouldSendAPostRequest()
     {
         $requestData = new RequestData('POST', 'http://www.example.com');
-        $request = new Request($this->connector, $requestData);
+        $requestOptions = new RequestOptions();
+        $connectorPair = new ConnectorPair($this->connector, $this->connector);
+        $request = new Request($connectorPair, $requestData, $requestOptions);
 
         $this->successfulConnectionMock();
 
@@ -251,7 +267,9 @@ public function postRequestShouldSendAPostRequest()
     public function writeWithAPostRequestShouldSendToTheStream()
     {
         $requestData = new RequestData('POST', 'http://www.example.com');
-        $request = new Request($this->connector, $requestData);
+        $requestOptions = new RequestOptions();
+        $connectorPair = new ConnectorPair($this->connector, $this->connector);
+        $request = new Request($connectorPair, $requestData, $requestOptions);
 
         $this->successfulConnectionMock();
 
@@ -292,7 +310,9 @@ public function writeWithAPostRequestShouldSendToTheStream()
     public function pipeShouldPipeDataIntoTheRequestBody()
     {
         $requestData = new RequestData('POST', 'http://www.example.com');
-        $request = new Request($this->connector, $requestData);
+        $requestOptions = new RequestOptions();
+        $connectorPair = new ConnectorPair($this->connector, $this->connector);
+        $request = new Request($connectorPair, $requestData, $requestOptions);
 
         $this->successfulConnectionMock();
 
@@ -343,7 +363,9 @@ public function pipeShouldPipeDataIntoTheRequestBody()
     public function endShouldOnlyAcceptScalars()
     {
         $requestData = new RequestData('POST', 'http://www.example.com');
-        $request = new Request($this->connector, $requestData);
+        $requestOptions = new RequestOptions();
+        $connectorPair = new ConnectorPair($this->connector, $this->connector);
+        $request = new Request($connectorPair, $requestData, $requestOptions);
 
         $request->end(array());
     }
@@ -352,16 +374,23 @@ public function endShouldOnlyAcceptScalars()
     public function requestShouldRelayErrorEventsFromResponse()
     {
         $requestData = new RequestData('GET', 'http://www.example.com');
-        $request = new Request($this->connector, $requestData);
+        $requestOptions = new RequestOptions();
+        $connectorPair = new ConnectorPair($this->connector, $this->connector);
+        $request = new Request($connectorPair, $requestData, $requestOptions);
 
         $this->successfulConnectionMock();
 
         $response = $this->response;
 
         $response->expects($this->at(0))
+            ->method('getCode')
+            ->will($this->returnValue(200));
+
+        $response->expects($this->at(1))
             ->method('on')
             ->with('end', $this->anything());
-        $response->expects($this->at(1))
+
+        $response->expects($this->at(2))
             ->method('on')
             ->with('error', $this->anything())
             ->will($this->returnCallback(function ($event, $cb) use (&$errorCallback) {
@@ -403,4 +432,3 @@ private function rejectedConnectionMock()
             ->will($this->returnValue(new RejectedPromise(new \RuntimeException())));
     }
 }
-

From ebf3d91b65ab110886c0da24571b78830c8c9f89 Mon Sep 17 00:00:00 2001
From: Beanow <beanow@oscp.info>
Date: Sat, 28 Feb 2015 00:14:44 +0100
Subject: [PATCH 2/6] Since we sent HTTP/1.0 headers, don't support HTTP/1.1
 responses.

---
 src/Request.php     |  4 ++--
 src/RequestData.php | 18 ++++++++++++++++--
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/src/Request.php b/src/Request.php
index 94b86d2..bcd9ca0 100644
--- a/src/Request.php
+++ b/src/Request.php
@@ -265,8 +265,8 @@ protected function connect()
 
     protected function isRedirectCode($code)
     {
-        //Note: 303 status is currently unsupported.
-        return in_array($code, [301, 302, 307, 308]);
+        //Note: 303, 307, 308 status is not supported in HTTP/1.0.
+        return in_array($code, [301, 302]);
     }
 
     public function setResponseFactory($factory)
diff --git a/src/RequestData.php b/src/RequestData.php
index e908cc2..38bb48f 100644
--- a/src/RequestData.php
+++ b/src/RequestData.php
@@ -32,6 +32,11 @@ private function mergeDefaultheaders(array $headers)
         );
     }
 
+    public function getMethod()
+    {
+        return strtoupper($this->method);
+    }
+
     public function getUrl()
     {
         return $this->url;
@@ -93,10 +98,19 @@ public function __toString()
     public function redirect($code, $location)
     {
         switch ($code) {
+
+            //These cases require that we switch to the GET method.
+            //@see https://github.com/bagder/curl/blob/cc28bc472ec421cec2ba26d653e53892998a248d/lib/transfer.c#L1736
             case 301:
             case 302:
-            case 307:
-            case 308:
+                $this->method = 'GET';
+
+            //Note: 303, 307, 308 status is not supported in HTTP/1.0.
+            // case 303:
+            // case 307:
+            // case 308:
+
+                //Of course switch the location.
                 $this->url = $location;
                 break;
 

From 2d83fedb2b02581be33d459c25d67874ae8486a2 Mon Sep 17 00:00:00 2001
From: Beanow <beanow@oscp.info>
Date: Sat, 28 Feb 2015 02:13:59 +0100
Subject: [PATCH 3/6] Improved test coverage.

---
 tests/RequestTest.php  | 198 +++++++++++++++++++++++++++++++++++++++++
 tests/ResponseTest.php |  41 ++++++++-
 2 files changed, 238 insertions(+), 1 deletion(-)

diff --git a/tests/RequestTest.php b/tests/RequestTest.php
index d8899eb..7cef306 100644
--- a/tests/RequestTest.php
+++ b/tests/RequestTest.php
@@ -263,6 +263,204 @@ public function postRequestShouldSendAPostRequest()
         $request->handleData("\r\nbody");
     }
 
+    /** @test */
+    public function redirectingPostRequestShouldSwitchToGet()
+    {
+        $requestData = new RequestData('POST', 'http://www.example.com');
+        $requestOptions = new RequestOptions([
+            'followRedirects' => true,
+            'maxRedirects' => 1,
+        ]);
+        $connectorPair = new ConnectorPair($this->connector, $this->connector);
+        $request = new Request($connectorPair, $requestData, $requestOptions);
+
+        $this->connector
+            ->expects($this->at(0))
+            ->method('create')
+            ->with('www.example.com', 80)
+            ->will($this->returnValue(new FulfilledPromise($this->stream)));
+
+        $secondStream = $this->getMockBuilder('React\Stream\Stream')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $secondStream
+            ->expects($this->at(4))
+            ->method('write')
+            ->with($this->matchesRegularExpression("#^GET / HTTP/1\.0\r\nHost: www.foo.bar\r\nUser-Agent:.*\r\n\r\n$#"));
+        $secondStream
+            ->expects($this->at(5))
+            ->method('close');
+
+        $this->connector
+            ->expects($this->at(1))
+            ->method('create')
+            ->with('www.foo.bar', 80)
+            ->will($this->returnValue(new FulfilledPromise($secondStream)));
+
+        $this->stream
+            ->expects($this->at(4))
+            ->method('write')
+            ->with($this->matchesRegularExpression("#^POST / HTTP/1\.0\r\nHost: www.example.com\r\nUser-Agent:.*\r\n\r\n$#"));
+        $this->stream
+            ->expects($this->at(5))
+            ->method('write')
+            ->with($this->identicalTo("some post data"));
+        $this->stream
+            ->expects($this->at(6))
+            ->method('close');
+
+        $factory = $this->createCallableMock();
+        $factory->expects($this->at(0))
+            ->method('__invoke')
+            ->will($this->returnValue($this->response));
+
+        $this->response
+            ->expects($this->exactly(2))
+            ->method('getCode')
+            ->will($this->returnValue(302));
+
+        $this->response
+            ->expects($this->exactly(1))
+            ->method('getHeaders')
+            ->will($this->returnValue([
+                'Location' => 'http://www.foo.bar',
+                'Content-Type' => 'text/plain',
+            ]));
+
+        $request->setResponseFactory($factory);
+        $request->end('some post data');
+
+        $request->handleData("HTTP/1.0 302 Found\r\n");
+        $request->handleData("Location: http://www.foo.bar\r\n");
+        $request->handleData("Content-Type: text/plain\r\n");
+        $request->handleData("\r\nbody");
+
+        $request->handleData("HTTP/1.0 200 OK\r\n");
+        $request->handleData("Content-Type: text/plain\r\n");
+        $request->handleData("\r\nbody");
+    }
+
+    /** @test */
+    public function shouldDetectCyclicRedirect()
+    {
+        $requestData = new RequestData('GET', 'http://www.example.com');
+        $requestOptions = new RequestOptions([
+            'followRedirects' => true,
+            'maxRedirects' => 5,
+        ]);
+        $connectorPair = new ConnectorPair($this->connector, $this->connector);
+        $request = new Request($connectorPair, $requestData, $requestOptions);
+
+        $this->connector
+            ->expects($this->at(0))
+            ->method('create')
+            ->with('www.example.com', 80)
+            ->will($this->returnValue(new FulfilledPromise($this->stream)));
+
+        $this->stream
+            ->expects($this->at(4))
+            ->method('write')
+            ->with($this->matchesRegularExpression("#^GET / HTTP/1\.0\r\nHost: www.example.com\r\nUser-Agent:.*\r\n\r\n$#"));
+        $this->stream
+            ->expects($this->at(5))
+            ->method('close');
+
+        $factory = $this->createCallableMock();
+        $factory->expects($this->at(0))
+            ->method('__invoke')
+            ->will($this->returnValue($this->response));
+
+        $this->response
+            ->expects($this->exactly(1))
+            ->method('getCode')
+            ->will($this->returnValue(302));
+
+        $this->response
+            ->expects($this->exactly(1))
+            ->method('getHeaders')
+            ->will($this->returnValue([
+                'Location' => 'http://www.example.com',
+                'Content-Type' => 'text/plain',
+            ]));
+
+        $handler = $this->createCallableMock();
+        $handler->expects($this->once())
+            ->method('__invoke')
+            ->with(
+                $this->isInstanceOf('RuntimeException'),
+                null,
+                $this->isInstanceOf('React\HttpClient\Request')
+            );
+
+        $request->on('end', $handler);
+        $request->on('close', $this->expectCallableNever());
+
+        $request->setResponseFactory($factory);
+        $request->end();
+
+        $request->handleData("HTTP/1.0 302 Found\r\n");
+        $request->handleData("Location: http://www.example.com\r\n");
+        $request->handleData("Content-Type: text/plain\r\n");
+        $request->handleData("\r\nbody");
+    }
+
+    /** @test */
+    public function shouldDetectTooManyRedirects()
+    {
+        $requestData = new RequestData('GET', 'http://www.example.com');
+        $requestOptions = new RequestOptions([
+            'followRedirects' => true,
+            'maxRedirects' => 0,
+        ]);
+        $connectorPair = new ConnectorPair($this->connector, $this->connector);
+        $request = new Request($connectorPair, $requestData, $requestOptions);
+
+        $this->connector
+            ->expects($this->at(0))
+            ->method('create')
+            ->with('www.example.com', 80)
+            ->will($this->returnValue(new FulfilledPromise($this->stream)));
+
+        $this->stream
+            ->expects($this->at(4))
+            ->method('write')
+            ->with($this->matchesRegularExpression("#^GET / HTTP/1\.0\r\nHost: www.example.com\r\nUser-Agent:.*\r\n\r\n$#"));
+        $this->stream
+            ->expects($this->at(5))
+            ->method('close');
+
+        $factory = $this->createCallableMock();
+        $factory->expects($this->at(0))
+            ->method('__invoke')
+            ->will($this->returnValue($this->response));
+
+        $this->response
+            ->expects($this->exactly(1))
+            ->method('getCode')
+            ->will($this->returnValue(302));
+
+        $handler = $this->createCallableMock();
+        $handler->expects($this->once())
+            ->method('__invoke')
+            ->with(
+                $this->isInstanceOf('RuntimeException'),
+                null,
+                $this->isInstanceOf('React\HttpClient\Request')
+            );
+
+        $request->on('end', $handler);
+        $request->on('close', $this->expectCallableNever());
+
+        $request->setResponseFactory($factory);
+        $request->end();
+
+        $request->handleData("HTTP/1.0 302 Found\r\n");
+        $request->handleData("Location: http://www.foo.bar\r\n");
+        $request->handleData("Content-Type: text/plain\r\n");
+        $request->handleData("\r\nbody");
+    }
+
     /** @test */
     public function writeWithAPostRequestShouldSendToTheStream()
     {
diff --git a/tests/ResponseTest.php b/tests/ResponseTest.php
index 5b86e19..ae508d9 100644
--- a/tests/ResponseTest.php
+++ b/tests/ResponseTest.php
@@ -73,5 +73,44 @@ public function closedResponseShouldNotBeResumedOrPaused()
         $response->resume();
         $response->pause();
     }
-}
 
+    /** @test */
+    public function responseShouldOfferHeaders()
+    {
+        $headers = array('content-type' => 'text/plain');
+        $response = new Response($this->stream, 'http', '1.0', '200', 'ok', $headers);
+        $this->assertSame($headers, $response->getHeaders());
+    }
+
+    /** @test */
+    public function responseShouldOfferProtocol()
+    {
+        $protocol = 'http';
+        $response = new Response($this->stream, $protocol, '1.0', '200', 'ok', array('content-type' => 'text/plain'));
+        $this->assertSame($protocol, $response->getProtocol());
+    }
+
+    /** @test */
+    public function responseShouldOfferVersion()
+    {
+        $version = '1.0';
+        $response = new Response($this->stream, 'http', $version, '200', 'ok', array('content-type' => 'text/plain'));
+        $this->assertSame($version, $response->getVersion());
+    }
+
+    /** @test */
+    public function responseShouldOfferReasonPhrase()
+    {
+        $reasonphrase = 'OK';
+        $response = new Response($this->stream, 'http', '1.0', '200', $reasonphrase, array('content-type' => 'text/plain'));
+        $this->assertSame($reasonphrase, $response->getReasonPhrase());
+    }
+
+    /** @test */
+    public function responseShouldOfferCode()
+    {
+        $code = 'OK';
+        $response = new Response($this->stream, 'http', '1.0', $code, 'ok', array('content-type' => 'text/plain'));
+        $this->assertSame($code, $response->getCode());
+    }
+}

From 372388afbf544948c3e19af04cdf4f02ce99e9db Mon Sep 17 00:00:00 2001
From: Beanow <beanow@oscp.info>
Date: Sat, 28 Feb 2015 02:14:18 +0100
Subject: [PATCH 4/6] Bugfixes based on tests.

---
 src/Request.php     | 6 +++++-
 src/RequestData.php | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/Request.php b/src/Request.php
index bcd9ca0..829f283 100644
--- a/src/Request.php
+++ b/src/Request.php
@@ -143,11 +143,13 @@ public function handleData($data)
                 && $this->requestOptions->shouldFollowRedirects()) {
 
                 //Have we reached our maximum redirects?
-                if ($this->requestOptions->getMaxRedirects() > 0
+                if ($this->requestOptions->getMaxRedirects() >= 0
                     && $this->redirectCount >= $this->requestOptions->getMaxRedirects()) {
                     $this->closeError(new \RuntimeException(
                         sprintf("Too many redirects: %u", $this->redirectCount)
                     ));
+
+                    return;
                 }
 
                 //Is the location a cyclic redirect?
@@ -156,6 +158,8 @@ public function handleData($data)
                     $this->closeError(new \RuntimeException(
                         "Cyclic redirect detected"
                     ));
+
+                    return;
                 }
 
                 //Store the next location to prevent cyclic redirects.
diff --git a/src/RequestData.php b/src/RequestData.php
index 38bb48f..04d30e2 100644
--- a/src/RequestData.php
+++ b/src/RequestData.php
@@ -2,6 +2,8 @@
 
 namespace React\HttpClient;
 
+use InvalidArgumentException;
+
 class RequestData
 {
     private $method;

From f885413376c8a1180fff0a5de7ec816bd955be58 Mon Sep 17 00:00:00 2001
From: Beanow <beanow@oscp.info>
Date: Sat, 28 Feb 2015 02:48:02 +0100
Subject: [PATCH 5/6] Missed the case of allowing POST->GET redirect without
 being detected as cyclic.

---
 src/Request.php       | 27 ++++++++++-----
 tests/RequestTest.php | 80 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 98 insertions(+), 9 deletions(-)

diff --git a/src/Request.php b/src/Request.php
index 829f283..4073970 100644
--- a/src/Request.php
+++ b/src/Request.php
@@ -43,7 +43,8 @@ public function __construct(ConnectorPair $connectorPair, RequestData $requestDa
         $this->requestOptions = $requestOptions;
         $this->connector = $connectorPair->getConnectorForScheme($requestData->getScheme());
         $this->redirectCount = 0;
-        $this->redirectLocations = [$requestData->getUrl()];
+        $this->redirectLocations = [];
+        $this->trackLocation($requestData);
     }
 
     public function isWritable()
@@ -152,9 +153,13 @@ public function handleData($data)
                     return;
                 }
 
-                //Is the location a cyclic redirect?
+                //Recalibrate to this new location.
                 $headers = $response->getHeaders();
-                if (in_array($headers['Location'], $this->redirectLocations)) {
+                $this->requestData->redirect($response->getCode(), $headers['Location']);
+                $this->connector = $this->connectorPair->getConnectorForScheme($this->requestData->getScheme());
+
+                //Is the location a cyclic redirect?
+                if ($this->isKnownLocation($this->requestData->getMethod(), $headers['Location'])) {
                     $this->closeError(new \RuntimeException(
                         "Cyclic redirect detected"
                     ));
@@ -163,13 +168,9 @@ public function handleData($data)
                 }
 
                 //Store the next location to prevent cyclic redirects.
-                $this->redirectLocations[] = $headers['Location'];
+                $this->trackLocation($this->requestData);
                 $this->redirectCount++;
 
-                //Recalibrate to this new location.
-                $this->requestData->redirect($response->getCode(), $headers['Location']);
-                $this->connector = $this->connectorPair->getConnectorForScheme($this->requestData->getScheme());
-
                 //Clean up and rewind.
                 $this->stream->close();
                 $this->responseFactory = null;
@@ -273,6 +274,16 @@ protected function isRedirectCode($code)
         return in_array($code, [301, 302]);
     }
 
+    protected function trackLocation(RequestData $requestData)
+    {
+        $this->redirectLocations[] = $requestData->getMethod().' '.$requestData->getUrl();
+    }
+
+    protected function isKnownLocation($method, $url)
+    {
+        return in_array($method.' '.$url, $this->redirectLocations);
+    }
+
     public function setResponseFactory($factory)
     {
         $this->responseFactory = $factory;
diff --git a/tests/RequestTest.php b/tests/RequestTest.php
index 7cef306..8a10c5e 100644
--- a/tests/RequestTest.php
+++ b/tests/RequestTest.php
@@ -372,7 +372,7 @@ public function shouldDetectCyclicRedirect()
             ->will($this->returnValue($this->response));
 
         $this->response
-            ->expects($this->exactly(1))
+            ->expects($this->exactly(2))
             ->method('getCode')
             ->will($this->returnValue(302));
 
@@ -405,6 +405,84 @@ public function shouldDetectCyclicRedirect()
         $request->handleData("\r\nbody");
     }
 
+    /** @test */
+    public function shouldAllowRedirectFromPostToGet()
+    {
+        $requestData = new RequestData('POST', 'http://www.example.com');
+        $requestOptions = new RequestOptions([
+            'followRedirects' => true,
+            'maxRedirects' => 1,
+        ]);
+        $connectorPair = new ConnectorPair($this->connector, $this->connector);
+        $request = new Request($connectorPair, $requestData, $requestOptions);
+
+        $this->connector
+            ->expects($this->at(0))
+            ->method('create')
+            ->with('www.example.com', 80)
+            ->will($this->returnValue(new FulfilledPromise($this->stream)));
+
+        $secondStream = $this->getMockBuilder('React\Stream\Stream')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $secondStream
+            ->expects($this->at(4))
+            ->method('write')
+            ->with($this->matchesRegularExpression("#^GET / HTTP/1\.0\r\nHost: www.example.com\r\nUser-Agent:.*\r\n\r\n$#"));
+        $secondStream
+            ->expects($this->at(5))
+            ->method('close');
+
+        $this->connector
+            ->expects($this->at(1))
+            ->method('create')
+            ->with('www.example.com', 80)
+            ->will($this->returnValue(new FulfilledPromise($secondStream)));
+
+        $this->stream
+            ->expects($this->at(4))
+            ->method('write')
+            ->with($this->matchesRegularExpression("#^POST / HTTP/1\.0\r\nHost: www.example.com\r\nUser-Agent:.*\r\n\r\n$#"));
+        $this->stream
+            ->expects($this->at(5))
+            ->method('write')
+            ->with($this->identicalTo("some post data"));
+        $this->stream
+            ->expects($this->at(6))
+            ->method('close');
+
+        $factory = $this->createCallableMock();
+        $factory->expects($this->exactly(1))
+            ->method('__invoke')
+            ->will($this->returnValue($this->response));
+
+        $this->response
+            ->expects($this->exactly(2))
+            ->method('getCode')
+            ->will($this->returnValue(302));
+
+        $this->response
+            ->expects($this->exactly(1))
+            ->method('getHeaders')
+            ->will($this->returnValue([
+                'Location' => 'http://www.example.com',
+                'Content-Type' => 'text/plain',
+            ]));
+
+        $request->setResponseFactory($factory);
+        $request->end('some post data');
+
+        $request->handleData("HTTP/1.0 302 Found\r\n");
+        $request->handleData("Location: http://www.example.com\r\n");
+        $request->handleData("Content-Type: text/plain\r\n");
+        $request->handleData("\r\nbody");
+
+        $request->handleData("HTTP/1.0 200 OK\r\n");
+        $request->handleData("Content-Type: text/plain\r\n");
+        $request->handleData("\r\nbody");
+    }
+
     /** @test */
     public function shouldDetectTooManyRedirects()
     {

From ad493838eeba64ed467cf0119061b93b3f1caeed Mon Sep 17 00:00:00 2001
From: Beanow <beanow@oscp.info>
Date: Sat, 28 Feb 2015 02:52:34 +0100
Subject: [PATCH 6/6] Updated documentation some.

---
 CHANGELOG.md |  5 +++--
 README.md    | 10 ++++------
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2ab07c3..d63e980 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,8 +1,9 @@
 # Changelog
 
-## 0.5.0 (2015-02-27)
+## Development (not released yet)
 
-* New feature: follow redirect option
+* New feature: followRedirects option
+* New feature: maxRedirects option
 * BC break: `Client` and `Request` take different constructor parameters.
 
 ## 0.4.0 (2014-02-02)
diff --git a/README.md b/README.md
index fcf7a4f..311a412 100644
--- a/README.md
+++ b/README.md
@@ -41,7 +41,10 @@ $dnsResolver = $dnsResolverFactory->createCached('8.8.8.8', $loop);
 $factory = new React\HttpClient\Factory();
 $client = $factory->create($loop, $dnsResolver);
 
-$request = $client->request('GET', 'https://github.com/');
+$request = $client->request('GET', 'https://github.com/', [], [
+    'followRedirects' => true,
+    'maxRedirects' => 5
+]);
 $request->on('response', function ($response) {
     $response->on('data', function ($data) {
         // ...
@@ -53,11 +56,6 @@ $loop->run();
 ?>
 ```
 
-## Work in progress
-
-* followRedirects option
-* maxRedirects option
-
 ## TODO
 
 * gzip content encoding