From b4719e165467e82d3e120824beff83f08f08cd31 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Mon, 4 Jul 2016 22:25:14 +0200 Subject: [PATCH 01/46] Chunked encoding decoder --- src/DecodeChunkedStream.php | 105 ++++++++++++++++++++++++++++++ tests/DecodeChunkedStreamTest.php | 46 +++++++++++++ 2 files changed, 151 insertions(+) create mode 100644 src/DecodeChunkedStream.php create mode 100644 tests/DecodeChunkedStreamTest.php diff --git a/src/DecodeChunkedStream.php b/src/DecodeChunkedStream.php new file mode 100644 index 0000000..ecb5647 --- /dev/null +++ b/src/DecodeChunkedStream.php @@ -0,0 +1,105 @@ +stream = $stream; + $this->stream->on('data', array($this, 'handleData')); + Util::forwardEvents($this->stream, $this, [ + 'error', + 'end', + ]); + } + + public function handleData($data) + { + $this->buffer .= $data; + + do { + $this->iterateBuffer(); + } while (strlen($this->buffer) > 0 && strpos($this->buffer, static::CRLF) !== false); + } + + protected function iterateBuffer() + { + if ($this->nextChunkIsLength) { + $this->nextChunkIsLength = false; + $this->remainingLength = hexdec(substr($this->buffer, 0, strpos($this->buffer, static::CRLF))); + $this->buffer = substr($this->buffer, strpos($this->buffer, static::CRLF) + 2); + } + + if ($this->remainingLength > 0) { + $chunkLength = $this->getChunkLength(); + $this->emit('data', array( + substr($this->buffer, 0, $chunkLength), + $this + )); + $this->remainingLength -= $chunkLength; + $this->buffer = substr($this->buffer, $chunkLength); + return; + } + + $this->nextChunkIsLength = true; + $this->buffer = substr($this->buffer, 2); + } + + protected function getChunkLength() + { + $bufferLength = strlen($this->buffer); + + if ($bufferLength >= $this->remainingLength) { + return $this->remainingLength; + } + + return $bufferLength; + } + + public function end($data = null) + { + $this->stream->end($data); + } + + public function pause() + { + $this->stream->pause(); + } + + public function resume() + { + $this->stream->resume(); + } +} diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php new file mode 100644 index 0000000..a6c4477 --- /dev/null +++ b/tests/DecodeChunkedStreamTest.php @@ -0,0 +1,46 @@ +on('data', function ($data) use (&$buffer) { + + echo PHP_EOL, '------------------' , PHP_EOL; + echo 'CHUNK: ', $data; + echo PHP_EOL, '------------------' , PHP_EOL; + + $buffer .= $data; + $this->assertTrue(true); + }); + foreach ($strings as $string) { + $stream->write($string); + } + $this->assertSame("Wikipedia in\r\n\r\nchunks.", $buffer); + } +} From 91cb3a58db9936dcb85b8f9c510a89ab241dccb7 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Mon, 4 Jul 2016 22:26:33 +0200 Subject: [PATCH 02/46] Renamed to more suitable class name --- src/{DecodeChunkedStream.php => ChunkedStreamDecoder.php} | 2 +- tests/DecodeChunkedStreamTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) rename src/{DecodeChunkedStream.php => ChunkedStreamDecoder.php} (98%) diff --git a/src/DecodeChunkedStream.php b/src/ChunkedStreamDecoder.php similarity index 98% rename from src/DecodeChunkedStream.php rename to src/ChunkedStreamDecoder.php index ecb5647..fbd1221 100644 --- a/src/DecodeChunkedStream.php +++ b/src/ChunkedStreamDecoder.php @@ -6,7 +6,7 @@ use React\Stream\DuplexStreamInterface; use React\Stream\Util; -class DecodeChunkedStream +class ChunkedStreamDecoder { const CRLF = "\r\n"; diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index a6c4477..312bc85 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -2,7 +2,7 @@ namespace React\Tests\HttpClient; -use React\HttpClient\DecodeChunkedStream; +use React\HttpClient\ChunkedStreamDecoder; use React\Stream\ThroughStream; class DecodeChunkedStreamTest extends TestCase @@ -27,7 +27,7 @@ public function provideChunkedEncoding() public function testChunkedEncoding(array $strings) { $stream = new ThroughStream(); - $response = new DecodeChunkedStream($stream); + $response = new ChunkedStreamDecoder($stream); $buffer = ''; $response->on('data', function ($data) use (&$buffer) { From fb14e6486ab779e97d1994a9e0640a6459a5952a Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Mon, 4 Jul 2016 22:26:48 +0200 Subject: [PATCH 03/46] Removed test debug --- tests/DecodeChunkedStreamTest.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index 312bc85..b4e2b38 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -30,11 +30,6 @@ public function testChunkedEncoding(array $strings) $response = new ChunkedStreamDecoder($stream); $buffer = ''; $response->on('data', function ($data) use (&$buffer) { - - echo PHP_EOL, '------------------' , PHP_EOL; - echo 'CHUNK: ', $data; - echo PHP_EOL, '------------------' , PHP_EOL; - $buffer .= $data; $this->assertTrue(true); }); From d6d2c1efcdb4ed6a8c30a85f042ab5857aea275a Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Mon, 4 Jul 2016 22:27:50 +0200 Subject: [PATCH 04/46] Removed assert true as it wasn't useful to the test --- tests/DecodeChunkedStreamTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index b4e2b38..6f1e471 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -31,7 +31,6 @@ public function testChunkedEncoding(array $strings) $buffer = ''; $response->on('data', function ($data) use (&$buffer) { $buffer .= $data; - $this->assertTrue(true); }); foreach ($strings as $string) { $stream->write($string); From dd04416771c90640535297d7b62c96d0fa30e274 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Mon, 4 Jul 2016 22:44:27 +0200 Subject: [PATCH 05/46] Swap stream when transfer encoding is chunked --- src/Response.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Response.php b/src/Response.php index 880d412..7662422 100644 --- a/src/Response.php +++ b/src/Response.php @@ -34,9 +34,13 @@ public function __construct(DuplexStreamInterface $stream, $protocol, $version, $this->reasonPhrase = $reasonPhrase; $this->headers = $headers; - $stream->on('data', array($this, 'handleData')); - $stream->on('error', array($this, 'handleError')); - $stream->on('end', array($this, 'handleEnd')); + if (isset($this->headers['transfer-encoding']) && $this->headers['transfer-encoding'] == 'chunked') { + $this->stream = new ChunkedStreamDecoder($stream); + } + + $this->stream->on('data', array($this, 'handleData')); + $this->stream->on('error', array($this, 'handleError')); + $this->stream->on('end', array($this, 'handleEnd')); } public function getProtocol() From 897c4a14c93a017eece4a9ed9c61cc1e1b6a17a0 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Mon, 4 Jul 2016 22:45:26 +0200 Subject: [PATCH 06/46] Removed chunked transfer encoding from todo --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index f56c660..554a145 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,5 @@ $loop->run(); ## TODO * gzip content encoding -* chunked transfer encoding * keep-alive connections * following redirections From b51cdb48d74152c106e51ae6e3b76c8417922057 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Tue, 5 Jul 2016 21:22:13 +0200 Subject: [PATCH 07/46] Response test to ensure chunked encoding decoder is used when it should --- tests/ResponseTest.php | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/ResponseTest.php b/tests/ResponseTest.php index 5b86e19..1521ca0 100644 --- a/tests/ResponseTest.php +++ b/tests/ResponseTest.php @@ -3,6 +3,7 @@ namespace React\Tests\HttpClient; use React\HttpClient\Response; +use React\Stream\ThroughStream; class ResponseTest extends TestCase { @@ -73,5 +74,32 @@ public function closedResponseShouldNotBeResumedOrPaused() $response->resume(); $response->pause(); } + + /** @test */ + public function chunkedEncodingResponse() + { + $stream = new ThroughStream(); + $response = new Response( + $stream, + 'http', + '1.0', + '200', + 'ok', + [ + 'content-type' => 'text/plain', + 'transfer-encoding' => 'chunked', + ] + ); + + $buffer = ''; + $response->on('data', function ($data) use (&$buffer) { + $buffer.= $data; + }); + $this->assertSame('', $buffer); + $stream->write("4\r\n"); + $this->assertSame('', $buffer); + $stream->write("Wiki\r\n"); + $this->assertSame('Wiki', $buffer); + } } From e1a370800f635540fb17d4d46026f4abed12c8a4 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Wed, 6 Jul 2016 19:08:33 +0200 Subject: [PATCH 08/46] Lower case checking as suggested by @jsor https://github.com/reactphp/http-client/pull/58#discussion_r69718043 --- src/Response.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Response.php b/src/Response.php index 7662422..2e0068d 100644 --- a/src/Response.php +++ b/src/Response.php @@ -33,8 +33,9 @@ public function __construct(DuplexStreamInterface $stream, $protocol, $version, $this->code = $code; $this->reasonPhrase = $reasonPhrase; $this->headers = $headers; + $normalizedHeaders = array_change_key_case($headers, CASE_LOWER); - if (isset($this->headers['transfer-encoding']) && $this->headers['transfer-encoding'] == 'chunked') { + if (isset($normalizedHeaders['transfer-encoding']) && strtolower($normalizedHeaders['transfer-encoding']) === 'chunked') { $this->stream = new ChunkedStreamDecoder($stream); } From bfba0371aa5e00833c069aa55d6f3ceceaf71d01 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Thu, 7 Jul 2016 07:58:16 +0200 Subject: [PATCH 09/46] Chunked encoding extensions as mentioned by @jsor https://github.com/reactphp/http-client/pull/58#discussion_r69712296 --- src/ChunkedStreamDecoder.php | 16 ++++++- tests/DecodeChunkedStreamTest.php | 75 +++++++++++++++++++++++++++---- 2 files changed, 80 insertions(+), 11 deletions(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index fbd1221..ea1dc21 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -17,6 +17,11 @@ class ChunkedStreamDecoder */ protected $buffer = ''; + /** + * @var string + */ + protected $chunkedExtension = ''; + /** * @var int */ @@ -58,7 +63,13 @@ protected function iterateBuffer() { if ($this->nextChunkIsLength) { $this->nextChunkIsLength = false; - $this->remainingLength = hexdec(substr($this->buffer, 0, strpos($this->buffer, static::CRLF))); + $this->chunkedExtension = ''; + $lengthChunk = substr($this->buffer, 0, strpos($this->buffer, static::CRLF)); + if (strpos($lengthChunk, ';') !== false) { + list($lengthChunk, $chunkedExtension) = explode(';', $lengthChunk, 2); + $this->chunkedExtension = trim($chunkedExtension); + } + $this->remainingLength = hexdec($lengthChunk); $this->buffer = substr($this->buffer, strpos($this->buffer, static::CRLF) + 2); } @@ -66,7 +77,8 @@ protected function iterateBuffer() $chunkLength = $this->getChunkLength(); $this->emit('data', array( substr($this->buffer, 0, $chunkLength), - $this + $this, + $this->chunkedExtension )); $this->remainingLength -= $chunkLength; $this->buffer = substr($this->buffer, $chunkLength); diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index 6f1e471..f36cef7 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -10,13 +10,67 @@ class DecodeChunkedStreamTest extends TestCase public function provideChunkedEncoding() { return [ - [["4\r\nWiki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"]], - [["4\r\nWiki\r\n", "5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"]], - [["4\r\nWiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"]], - [["4\r\nWiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n", "\r\nchunks.\r\n0\r\n\r\n"]], - [["4\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n", "\r\nchunks.\r\n0\r\n\r\n"]], - [["4\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne\r\n", " in\r\n", "\r\nchunks.\r\n0\r\n\r\n"]], - [["4\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne\r\n", " in\r\n", "\r\nchunks.\r\n", "0\r\n\r\n"]], + [ + ["4\r\nWiki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], + [ + '', + '', + '', + ], + ], + [ + ["4\r\nWiki\r\n", "5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], + [ + '', + '', + '', + ], + ], + [ + ["4\r\nWiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], + [ + + '', + '', + '', + ], + ], + [ + ["4\r\nWiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n", "\r\nchunks.\r\n0\r\n\r\n"], + [ + '', + '', + '', + '', + ], + ], + [ + ["4\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n", "\r\nchunks.\r\n0\r\n\r\n"], + [ + '', + '', + '', + '', + ], + ], + [ + ["4\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne; foo=[bar,beer,pool,cue,win,won]\r\n", " in\r\n", "\r\nchunks.\r\n0\r\n\r\n"], + [ + '', + '', + 'foo=[bar,beer,pool,cue,win,won]', + 'foo=[bar,beer,pool,cue,win,won]', + ], + ], + [ + ["4; foo=bar\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne\r\n", " in\r\n", "\r\nchunks.\r\n", "0\r\n\r\n"], + [ + 'foo=bar', + '', + '', + '', + ], + ], ]; } @@ -24,17 +78,20 @@ public function provideChunkedEncoding() * @test * @dataProvider provideChunkedEncoding */ - public function testChunkedEncoding(array $strings) + public function testChunkedEncoding(array $strings, array $extensions) { $stream = new ThroughStream(); $response = new ChunkedStreamDecoder($stream); $buffer = ''; - $response->on('data', function ($data) use (&$buffer) { + $exts = []; + $response->on('data', function ($data, $response, $ext) use (&$buffer, &$exts) { $buffer .= $data; + $exts[] = $ext; }); foreach ($strings as $string) { $stream->write($string); } $this->assertSame("Wikipedia in\r\n\r\nchunks.", $buffer); + $this->assertSame($extensions, $exts); } } From a3b9c80ae34eeff40d94df4d5e1ba12413150e79 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Thu, 7 Jul 2016 07:58:34 +0200 Subject: [PATCH 10/46] Don't emit empty data chunks --- src/ChunkedStreamDecoder.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index ea1dc21..c32ca46 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -75,6 +75,9 @@ protected function iterateBuffer() if ($this->remainingLength > 0) { $chunkLength = $this->getChunkLength(); + if ($chunkLength === 0) { + return; + } $this->emit('data', array( substr($this->buffer, 0, $chunkLength), $this, From 4ca65b36c55c4fc5adb26f41a1796d49d681c853 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Fri, 8 Jul 2016 17:30:43 +0200 Subject: [PATCH 11/46] Pass the extra's from the stream through the response --- src/Response.php | 4 ++-- tests/ResponseTest.php | 9 +++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Response.php b/src/Response.php index 2e0068d..dfce614 100644 --- a/src/Response.php +++ b/src/Response.php @@ -69,9 +69,9 @@ public function getHeaders() return $this->headers; } - public function handleData($data) + public function handleData($data, $stream = null, $extra = '') { - $this->emit('data', array($data, $this)); + $this->emit('data', array($data, $this, $extra)); } public function handleEnd() diff --git a/tests/ResponseTest.php b/tests/ResponseTest.php index 1521ca0..daa8c1f 100644 --- a/tests/ResponseTest.php +++ b/tests/ResponseTest.php @@ -92,14 +92,19 @@ public function chunkedEncodingResponse() ); $buffer = ''; - $response->on('data', function ($data) use (&$buffer) { + $exts = []; + $response->on('data', function ($data, $stream, $ext) use (&$buffer, &$exts) { $buffer.= $data; + $exts[] = $ext; }); $this->assertSame('', $buffer); - $stream->write("4\r\n"); + $this->assertSame([], $exts); + $stream->write("4; abc=def\r\n"); $this->assertSame('', $buffer); + $this->assertSame([], $exts); $stream->write("Wiki\r\n"); $this->assertSame('Wiki', $buffer); + $this->assertSame(['abc=def'], $exts); } } From 0eab14bb336d708e945f6e92556f031657d77c14 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Wed, 3 Aug 2016 07:47:03 +0200 Subject: [PATCH 12/46] Changed emitted extra to be more explicit as suggested by @jsor at https://github.com/reactphp/http-client/pull/58#issuecomment-237064370 --- src/ChunkedStreamDecoder.php | 4 ++- src/Response.php | 2 +- tests/DecodeChunkedStreamTest.php | 50 +++++++++++++++---------------- tests/ResponseTest.php | 2 +- 4 files changed, 30 insertions(+), 28 deletions(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index c32ca46..678014e 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -81,7 +81,9 @@ protected function iterateBuffer() $this->emit('data', array( substr($this->buffer, 0, $chunkLength), $this, - $this->chunkedExtension + [ + 'chunkedExtension' => $this->chunkedExtension, + ] )); $this->remainingLength -= $chunkLength; $this->buffer = substr($this->buffer, $chunkLength); diff --git a/src/Response.php b/src/Response.php index dfce614..dde9ed7 100644 --- a/src/Response.php +++ b/src/Response.php @@ -69,7 +69,7 @@ public function getHeaders() return $this->headers; } - public function handleData($data, $stream = null, $extra = '') + public function handleData($data, $stream = null, $extra = []) { $this->emit('data', array($data, $this, $extra)); } diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index f36cef7..3fa4789 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -13,62 +13,62 @@ public function provideChunkedEncoding() [ ["4\r\nWiki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], [ - '', - '', - '', + ['chunkedExtension' => ''], + ['chunkedExtension' => ''], + ['chunkedExtension' => ''], ], ], [ ["4\r\nWiki\r\n", "5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], [ - '', - '', - '', + ['chunkedExtension' => ''], + ['chunkedExtension' => ''], + ['chunkedExtension' => ''], ], ], [ ["4\r\nWiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], [ - '', - '', - '', + ['chunkedExtension' => ''], + ['chunkedExtension' => ''], + ['chunkedExtension' => ''], ], ], [ ["4\r\nWiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n", "\r\nchunks.\r\n0\r\n\r\n"], [ - '', - '', - '', - '', + ['chunkedExtension' => ''], + ['chunkedExtension' => ''], + ['chunkedExtension' => ''], + ['chunkedExtension' => ''], ], ], [ ["4\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n", "\r\nchunks.\r\n0\r\n\r\n"], [ - '', - '', - '', - '', + ['chunkedExtension' => ''], + ['chunkedExtension' => ''], + ['chunkedExtension' => ''], + ['chunkedExtension' => ''], ], ], [ ["4\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne; foo=[bar,beer,pool,cue,win,won]\r\n", " in\r\n", "\r\nchunks.\r\n0\r\n\r\n"], [ - '', - '', - 'foo=[bar,beer,pool,cue,win,won]', - 'foo=[bar,beer,pool,cue,win,won]', + ['chunkedExtension' => ''], + ['chunkedExtension' => ''], + ['chunkedExtension' => 'foo=[bar,beer,pool,cue,win,won]'], + ['chunkedExtension' => 'foo=[bar,beer,pool,cue,win,won]'], ], ], [ ["4; foo=bar\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne\r\n", " in\r\n", "\r\nchunks.\r\n", "0\r\n\r\n"], [ - 'foo=bar', - '', - '', - '', + ['chunkedExtension' => 'foo=bar'], + ['chunkedExtension' => ''], + ['chunkedExtension' => ''], + ['chunkedExtension' => ''], ], ], ]; diff --git a/tests/ResponseTest.php b/tests/ResponseTest.php index daa8c1f..1fe3bf0 100644 --- a/tests/ResponseTest.php +++ b/tests/ResponseTest.php @@ -104,7 +104,7 @@ public function chunkedEncodingResponse() $this->assertSame([], $exts); $stream->write("Wiki\r\n"); $this->assertSame('Wiki', $buffer); - $this->assertSame(['abc=def'], $exts); + $this->assertSame([['chunkedExtension' => 'abc=def']], $exts); } } From f9067ceb2cf26072881ff2815bd3b92ec938eeeb Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Wed, 3 Aug 2016 08:47:38 +0200 Subject: [PATCH 13/46] chunkedExtension => chunkExtension --- src/ChunkedStreamDecoder.php | 10 +++---- tests/DecodeChunkedStreamTest.php | 50 +++++++++++++++---------------- tests/ResponseTest.php | 2 +- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index 678014e..e57bb7e 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -20,7 +20,7 @@ class ChunkedStreamDecoder /** * @var string */ - protected $chunkedExtension = ''; + protected $chunkExtension = ''; /** * @var int @@ -63,11 +63,11 @@ protected function iterateBuffer() { if ($this->nextChunkIsLength) { $this->nextChunkIsLength = false; - $this->chunkedExtension = ''; + $this->chunkExtension = ''; $lengthChunk = substr($this->buffer, 0, strpos($this->buffer, static::CRLF)); if (strpos($lengthChunk, ';') !== false) { - list($lengthChunk, $chunkedExtension) = explode(';', $lengthChunk, 2); - $this->chunkedExtension = trim($chunkedExtension); + list($lengthChunk, $chunkExtension) = explode(';', $lengthChunk, 2); + $this->chunkExtension = trim($chunkExtension); } $this->remainingLength = hexdec($lengthChunk); $this->buffer = substr($this->buffer, strpos($this->buffer, static::CRLF) + 2); @@ -82,7 +82,7 @@ protected function iterateBuffer() substr($this->buffer, 0, $chunkLength), $this, [ - 'chunkedExtension' => $this->chunkedExtension, + 'chunkExtension' => $this->chunkExtension, ] )); $this->remainingLength -= $chunkLength; diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index 3fa4789..6979557 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -13,62 +13,62 @@ public function provideChunkedEncoding() [ ["4\r\nWiki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], [ - ['chunkedExtension' => ''], - ['chunkedExtension' => ''], - ['chunkedExtension' => ''], + ['chunkExtension' => ''], + ['chunkExtension' => ''], + ['chunkExtension' => ''], ], ], [ ["4\r\nWiki\r\n", "5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], [ - ['chunkedExtension' => ''], - ['chunkedExtension' => ''], - ['chunkedExtension' => ''], + ['chunkExtension' => ''], + ['chunkExtension' => ''], + ['chunkExtension' => ''], ], ], [ ["4\r\nWiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], [ - ['chunkedExtension' => ''], - ['chunkedExtension' => ''], - ['chunkedExtension' => ''], + ['chunkExtension' => ''], + ['chunkExtension' => ''], + ['chunkExtension' => ''], ], ], [ ["4\r\nWiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n", "\r\nchunks.\r\n0\r\n\r\n"], [ - ['chunkedExtension' => ''], - ['chunkedExtension' => ''], - ['chunkedExtension' => ''], - ['chunkedExtension' => ''], + ['chunkExtension' => ''], + ['chunkExtension' => ''], + ['chunkExtension' => ''], + ['chunkExtension' => ''], ], ], [ ["4\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n", "\r\nchunks.\r\n0\r\n\r\n"], [ - ['chunkedExtension' => ''], - ['chunkedExtension' => ''], - ['chunkedExtension' => ''], - ['chunkedExtension' => ''], + ['chunkExtension' => ''], + ['chunkExtension' => ''], + ['chunkExtension' => ''], + ['chunkExtension' => ''], ], ], [ ["4\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne; foo=[bar,beer,pool,cue,win,won]\r\n", " in\r\n", "\r\nchunks.\r\n0\r\n\r\n"], [ - ['chunkedExtension' => ''], - ['chunkedExtension' => ''], - ['chunkedExtension' => 'foo=[bar,beer,pool,cue,win,won]'], - ['chunkedExtension' => 'foo=[bar,beer,pool,cue,win,won]'], + ['chunkExtension' => ''], + ['chunkExtension' => ''], + ['chunkExtension' => 'foo=[bar,beer,pool,cue,win,won]'], + ['chunkExtension' => 'foo=[bar,beer,pool,cue,win,won]'], ], ], [ ["4; foo=bar\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne\r\n", " in\r\n", "\r\nchunks.\r\n", "0\r\n\r\n"], [ - ['chunkedExtension' => 'foo=bar'], - ['chunkedExtension' => ''], - ['chunkedExtension' => ''], - ['chunkedExtension' => ''], + ['chunkExtension' => 'foo=bar'], + ['chunkExtension' => ''], + ['chunkExtension' => ''], + ['chunkExtension' => ''], ], ], ]; diff --git a/tests/ResponseTest.php b/tests/ResponseTest.php index 1fe3bf0..f2076c6 100644 --- a/tests/ResponseTest.php +++ b/tests/ResponseTest.php @@ -104,7 +104,7 @@ public function chunkedEncodingResponse() $this->assertSame([], $exts); $stream->write("Wiki\r\n"); $this->assertSame('Wiki', $buffer); - $this->assertSame([['chunkedExtension' => 'abc=def']], $exts); + $this->assertSame([['chunkExtension' => 'abc=def']], $exts); } } From 85b0c93c0e144353f219b0a40b3f05289ec983d8 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Wed, 3 Aug 2016 19:28:58 +0200 Subject: [PATCH 14/46] Store CRLF position into a variable, suggested by @clue at https://github.com/reactphp/http-client/pull/58#discussion_r73316833 --- src/ChunkedStreamDecoder.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index e57bb7e..26d0793 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -64,13 +64,14 @@ protected function iterateBuffer() if ($this->nextChunkIsLength) { $this->nextChunkIsLength = false; $this->chunkExtension = ''; - $lengthChunk = substr($this->buffer, 0, strpos($this->buffer, static::CRLF)); + $crlfPosition = strpos($this->buffer, static::CRLF); + $lengthChunk = substr($this->buffer, 0, $crlfPosition); if (strpos($lengthChunk, ';') !== false) { list($lengthChunk, $chunkExtension) = explode(';', $lengthChunk, 2); $this->chunkExtension = trim($chunkExtension); } $this->remainingLength = hexdec($lengthChunk); - $this->buffer = substr($this->buffer, strpos($this->buffer, static::CRLF) + 2); + $this->buffer = substr($this->buffer, $crlfPosition + 2); } if ($this->remainingLength > 0) { From 03d271a2403b307059a8933b8b7b5c6e4efb2ebe Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Wed, 3 Aug 2016 19:31:48 +0200 Subject: [PATCH 15/46] Downgraded to a ReadableStreamInterface from the duplex as suggested by @clue at https://github.com/reactphp/http-client/pull/58#discussion_r73316509 --- src/ChunkedStreamDecoder.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index 26d0793..cec87a9 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -3,7 +3,7 @@ namespace React\HttpClient; use Evenement\EventEmitterTrait; -use React\Stream\DuplexStreamInterface; +use React\Stream\ReadableStreamInterface; use React\Stream\Util; class ChunkedStreamDecoder @@ -33,14 +33,14 @@ class ChunkedStreamDecoder protected $nextChunkIsLength = true; /** - * @var DuplexStreamInterface + * @var ReadableStreamInterface */ protected $stream; /** - * @param DuplexStreamInterface $stream + * @param ReadableStreamInterface $stream */ - public function __construct(DuplexStreamInterface $stream) + public function __construct(ReadableStreamInterface $stream) { $this->stream = $stream; $this->stream->on('data', array($this, 'handleData')); From 18f044a93f10897f053ce8585139c02a271989c1 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Wed, 3 Aug 2016 19:40:46 +0200 Subject: [PATCH 16/46] Removed write stream `end` event forward as suggested by @clue at https://github.com/reactphp/http-client/pull/58#discussion_r73316691 --- src/ChunkedStreamDecoder.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index cec87a9..98fdee4 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -46,7 +46,6 @@ public function __construct(ReadableStreamInterface $stream) $this->stream->on('data', array($this, 'handleData')); Util::forwardEvents($this->stream, $this, [ 'error', - 'end', ]); } From cd4ba40b9f1be6a6246d888d30bb1d40a78cceca Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Wed, 3 Aug 2016 21:46:55 +0200 Subject: [PATCH 17/46] Removed chunk extensions as suggested by @clue as they don't have a use case at the moment at https://github.com/reactphp/http-client/pull/58#discussion_r73317364 --- src/ChunkedStreamDecoder.php | 14 ++------- tests/DecodeChunkedStreamTest.php | 47 ++----------------------------- tests/ResponseTest.php | 7 +---- 3 files changed, 5 insertions(+), 63 deletions(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index 98fdee4..181984d 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -17,11 +17,6 @@ class ChunkedStreamDecoder */ protected $buffer = ''; - /** - * @var string - */ - protected $chunkExtension = ''; - /** * @var int */ @@ -62,12 +57,10 @@ protected function iterateBuffer() { if ($this->nextChunkIsLength) { $this->nextChunkIsLength = false; - $this->chunkExtension = ''; $crlfPosition = strpos($this->buffer, static::CRLF); $lengthChunk = substr($this->buffer, 0, $crlfPosition); if (strpos($lengthChunk, ';') !== false) { - list($lengthChunk, $chunkExtension) = explode(';', $lengthChunk, 2); - $this->chunkExtension = trim($chunkExtension); + list($lengthChunk) = explode(';', $lengthChunk, 2); } $this->remainingLength = hexdec($lengthChunk); $this->buffer = substr($this->buffer, $crlfPosition + 2); @@ -80,10 +73,7 @@ protected function iterateBuffer() } $this->emit('data', array( substr($this->buffer, 0, $chunkLength), - $this, - [ - 'chunkExtension' => $this->chunkExtension, - ] + $this )); $this->remainingLength -= $chunkLength; $this->buffer = substr($this->buffer, $chunkLength); diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index 6979557..06dc392 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -12,64 +12,24 @@ public function provideChunkedEncoding() return [ [ ["4\r\nWiki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], - [ - ['chunkExtension' => ''], - ['chunkExtension' => ''], - ['chunkExtension' => ''], - ], ], [ ["4\r\nWiki\r\n", "5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], - [ - ['chunkExtension' => ''], - ['chunkExtension' => ''], - ['chunkExtension' => ''], - ], ], [ ["4\r\nWiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], - [ - - ['chunkExtension' => ''], - ['chunkExtension' => ''], - ['chunkExtension' => ''], - ], ], [ ["4\r\nWiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n", "\r\nchunks.\r\n0\r\n\r\n"], - [ - ['chunkExtension' => ''], - ['chunkExtension' => ''], - ['chunkExtension' => ''], - ['chunkExtension' => ''], - ], ], [ ["4\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n", "\r\nchunks.\r\n0\r\n\r\n"], - [ - ['chunkExtension' => ''], - ['chunkExtension' => ''], - ['chunkExtension' => ''], - ['chunkExtension' => ''], - ], ], [ ["4\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne; foo=[bar,beer,pool,cue,win,won]\r\n", " in\r\n", "\r\nchunks.\r\n0\r\n\r\n"], - [ - ['chunkExtension' => ''], - ['chunkExtension' => ''], - ['chunkExtension' => 'foo=[bar,beer,pool,cue,win,won]'], - ['chunkExtension' => 'foo=[bar,beer,pool,cue,win,won]'], - ], ], [ ["4; foo=bar\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne\r\n", " in\r\n", "\r\nchunks.\r\n", "0\r\n\r\n"], - [ - ['chunkExtension' => 'foo=bar'], - ['chunkExtension' => ''], - ['chunkExtension' => ''], - ['chunkExtension' => ''], - ], ], ]; } @@ -78,20 +38,17 @@ public function provideChunkedEncoding() * @test * @dataProvider provideChunkedEncoding */ - public function testChunkedEncoding(array $strings, array $extensions) + public function testChunkedEncoding(array $strings) { $stream = new ThroughStream(); $response = new ChunkedStreamDecoder($stream); $buffer = ''; - $exts = []; - $response->on('data', function ($data, $response, $ext) use (&$buffer, &$exts) { + $response->on('data', function ($data, $response) use (&$buffer) { $buffer .= $data; - $exts[] = $ext; }); foreach ($strings as $string) { $stream->write($string); } $this->assertSame("Wikipedia in\r\n\r\nchunks.", $buffer); - $this->assertSame($extensions, $exts); } } diff --git a/tests/ResponseTest.php b/tests/ResponseTest.php index f2076c6..7d84860 100644 --- a/tests/ResponseTest.php +++ b/tests/ResponseTest.php @@ -92,19 +92,14 @@ public function chunkedEncodingResponse() ); $buffer = ''; - $exts = []; - $response->on('data', function ($data, $stream, $ext) use (&$buffer, &$exts) { + $response->on('data', function ($data, $stream) use (&$buffer) { $buffer.= $data; - $exts[] = $ext; }); $this->assertSame('', $buffer); - $this->assertSame([], $exts); $stream->write("4; abc=def\r\n"); $this->assertSame('', $buffer); - $this->assertSame([], $exts); $stream->write("Wiki\r\n"); $this->assertSame('Wiki', $buffer); - $this->assertSame([['chunkExtension' => 'abc=def']], $exts); } } From 537de4337b9494b19168aaf3afbb5889fc7630c0 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Thu, 4 Aug 2016 17:38:06 +0200 Subject: [PATCH 18/46] Enure chunk length header is in as suggested by @clue at https://github.com/reactphp/http-client/pull/58#discussion_r73316917 --- src/ChunkedStreamDecoder.php | 5 ++++- tests/DecodeChunkedStreamTest.php | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index 181984d..0e837dc 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -56,8 +56,11 @@ public function handleData($data) protected function iterateBuffer() { if ($this->nextChunkIsLength) { - $this->nextChunkIsLength = false; $crlfPosition = strpos($this->buffer, static::CRLF); + if ($crlfPosition === false) { + return; // Chunk header hasn't completely come in yet + } + $this->nextChunkIsLength = false; $lengthChunk = substr($this->buffer, 0, $crlfPosition); if (strpos($lengthChunk, ';') !== false) { list($lengthChunk) = explode(';', $lengthChunk, 2); diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index 06dc392..8fd58e8 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -31,6 +31,9 @@ public function provideChunkedEncoding() [ ["4; foo=bar\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne\r\n", " in\r\n", "\r\nchunks.\r\n", "0\r\n\r\n"], ], + [ + str_split("4\r\nWiki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"), + ], ]; } From 42201c86d215981b51c427f95db84e95704bf83c Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Thu, 4 Aug 2016 19:09:31 +0200 Subject: [PATCH 19/46] Ensure CRLF in the middle of bodies don't cause unexpected behavior by @clue at https://github.com/reactphp/http-client/pull/58#discussion_r73317481 --- src/ChunkedStreamDecoder.php | 8 +++++++- tests/DecodeChunkedStreamTest.php | 20 ++++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index 0e837dc..6bb3f22 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -49,8 +49,14 @@ public function handleData($data) $this->buffer .= $data; do { + $bufferLength = strlen($this->buffer); $this->iterateBuffer(); - } while (strlen($this->buffer) > 0 && strpos($this->buffer, static::CRLF) !== false); + $iteratedBufferLength = strlen($this->buffer); + } while ( + $bufferLength !== $iteratedBufferLength && + $iteratedBufferLength > 0 && + strpos($this->buffer, static::CRLF) !== false + ); } protected function iterateBuffer() diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index 8fd58e8..81ea9f4 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -34,6 +34,22 @@ public function provideChunkedEncoding() [ str_split("4\r\nWiki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"), ], + [ + str_split("6\r\nWi\r\nki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"), + "Wi\r\nkipedia in\r\n\r\nchunks." + ], + [ + ["6\r\nWi\r\n", "ki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], + "Wi\r\nkipedia in\r\n\r\nchunks." + ], + [ + ["4\r\nWi\r\n", "ki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], + "Wi\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n" + ], + [ + str_split("4\r\nWi\r\nki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"), + "Wi\r\npedia in\r\n\r\nchunks." + ], ]; } @@ -41,7 +57,7 @@ public function provideChunkedEncoding() * @test * @dataProvider provideChunkedEncoding */ - public function testChunkedEncoding(array $strings) + public function testChunkedEncoding(array $strings, $expected = "Wikipedia in\r\n\r\nchunks.") { $stream = new ThroughStream(); $response = new ChunkedStreamDecoder($stream); @@ -52,6 +68,6 @@ public function testChunkedEncoding(array $strings) foreach ($strings as $string) { $stream->write($string); } - $this->assertSame("Wikipedia in\r\n\r\nchunks.", $buffer); + $this->assertSame($expected, $buffer); } } From 48763cfe218dcec5b548b3af148cd4f42d70adeb Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Tue, 9 Aug 2016 16:22:43 +0200 Subject: [PATCH 20/46] Removed surplus arguments via @clue https://github.com/reactphp/http-client/pull/58#discussion_r74047543 --- src/Response.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Response.php b/src/Response.php index dde9ed7..2e0068d 100644 --- a/src/Response.php +++ b/src/Response.php @@ -69,9 +69,9 @@ public function getHeaders() return $this->headers; } - public function handleData($data, $stream = null, $extra = []) + public function handleData($data) { - $this->emit('data', array($data, $this, $extra)); + $this->emit('data', array($data, $this)); } public function handleEnd() From b32a1889177da46c1b953d7e3b4d18d6e974a7b5 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Tue, 9 Aug 2016 16:23:31 +0200 Subject: [PATCH 21/46] Marking method internal via @clue https://github.com/reactphp/http-client/pull/58#discussion_r74046741 --- src/ChunkedStreamDecoder.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index 6bb3f22..f24d4f9 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -44,6 +44,7 @@ public function __construct(ReadableStreamInterface $stream) ]); } + /** @internal */ public function handleData($data) { $this->buffer .= $data; From 6e1ebcc99d92456b8033bcd990c82e8eddad14ac Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Tue, 9 Aug 2016 16:23:58 +0200 Subject: [PATCH 22/46] Removed dead end method via @clue https://github.com/reactphp/http-client/pull/58#discussion_r74046531 --- src/ChunkedStreamDecoder.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index f24d4f9..eaaceb4 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -105,11 +105,6 @@ protected function getChunkLength() return $bufferLength; } - public function end($data = null) - { - $this->stream->end($data); - } - public function pause() { $this->stream->pause(); From 69ab9c6f137b18c968455b9aaf0625844055b387 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Tue, 9 Aug 2016 16:26:24 +0200 Subject: [PATCH 23/46] Implement ReadableStreamInterface and added missing methods for it via @clue https://github.com/reactphp/http-client/pull/58#discussion_r74046454 --- src/ChunkedStreamDecoder.php | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index eaaceb4..a6c8ae5 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -5,8 +5,9 @@ use Evenement\EventEmitterTrait; use React\Stream\ReadableStreamInterface; use React\Stream\Util; +use React\Stream\WritableStreamInterface; -class ChunkedStreamDecoder +class ChunkedStreamDecoder implements ReadableStreamInterface { const CRLF = "\r\n"; @@ -114,4 +115,21 @@ public function resume() { $this->stream->resume(); } + + public function isReadable() + { + return $this->stream->isReadable(); + } + + public function pipe(WritableStreamInterface $dest, array $options = array()) + { + Util::pipe($this, $dest, $options); + + return $dest; + } + + public function close() + { + return $this->stream->close(); + } } From d4eda4d66c49ad120de555c6bc3cd463e979f4d0 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Mon, 15 Aug 2016 23:12:42 +0200 Subject: [PATCH 24/46] Removed transfer encoding header as suggested by @joelwurtz at https://github.com/reactphp/http-client/pull/58#discussion_r74048306 --- src/Response.php | 8 ++++++++ tests/ResponseTest.php | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/Response.php b/src/Response.php index 2e0068d..8a90e28 100644 --- a/src/Response.php +++ b/src/Response.php @@ -37,6 +37,14 @@ public function __construct(DuplexStreamInterface $stream, $protocol, $version, if (isset($normalizedHeaders['transfer-encoding']) && strtolower($normalizedHeaders['transfer-encoding']) === 'chunked') { $this->stream = new ChunkedStreamDecoder($stream); + + unset($normalizedHeaders['transfer-encoding']); + foreach ($this->headers as $key => $value) { + if (strcasecmp('transfer-encoding', $key) === 0) { + unset($this->headers[$key]); + break; + } + } } $this->stream->on('data', array($this, 'handleData')); diff --git a/tests/ResponseTest.php b/tests/ResponseTest.php index 7d84860..9ed4009 100644 --- a/tests/ResponseTest.php +++ b/tests/ResponseTest.php @@ -55,6 +55,13 @@ public function responseShouldEmitEndEventOnEnd() $response->handleData('some data'); $response->handleEnd(); + + $this->assertSame( + [ + 'Content-Type' => 'text/plain' + ], + $response->getHeaders() + ); } /** @test */ @@ -73,6 +80,13 @@ public function closedResponseShouldNotBeResumedOrPaused() $response->resume(); $response->pause(); + + $this->assertSame( + [ + 'content-type' => 'text/plain', + ], + $response->getHeaders() + ); } /** @test */ @@ -100,6 +114,13 @@ public function chunkedEncodingResponse() $this->assertSame('', $buffer); $stream->write("Wiki\r\n"); $this->assertSame('Wiki', $buffer); + + $this->assertSame( + [ + 'content-type' => 'text/plain', + ], + $response->getHeaders() + ); } } From 804902f4fe73981a1d2789842585f963f1c2862b Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Mon, 15 Aug 2016 23:13:21 +0200 Subject: [PATCH 25/46] No need to unset when we're not using it from there on --- src/Response.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Response.php b/src/Response.php index 8a90e28..68cb3a6 100644 --- a/src/Response.php +++ b/src/Response.php @@ -38,7 +38,6 @@ public function __construct(DuplexStreamInterface $stream, $protocol, $version, if (isset($normalizedHeaders['transfer-encoding']) && strtolower($normalizedHeaders['transfer-encoding']) === 'chunked') { $this->stream = new ChunkedStreamDecoder($stream); - unset($normalizedHeaders['transfer-encoding']); foreach ($this->headers as $key => $value) { if (strcasecmp('transfer-encoding', $key) === 0) { unset($this->headers[$key]); From 29e283b17fd45fe66803729f696da2c9ab8dc180 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Tue, 16 Aug 2016 21:43:31 +0200 Subject: [PATCH 26/46] Implement chunked encoding overflow vulnerability prevention suggested by @clue at https://github.com/reactphp/http-client/pull/58#discussion_r74048046 --- src/ChunkedStreamDecoder.php | 20 +++++++++++--- tests/DecodeChunkedStreamTest.php | 43 ++++++++++++++++++++++++------- 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index a6c8ae5..64e6766 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -3,6 +3,7 @@ namespace React\HttpClient; use Evenement\EventEmitterTrait; +use Exception; use React\Stream\ReadableStreamInterface; use React\Stream\Util; use React\Stream\WritableStreamInterface; @@ -52,9 +53,10 @@ public function handleData($data) do { $bufferLength = strlen($this->buffer); - $this->iterateBuffer(); + $continue = $this->iterateBuffer(); $iteratedBufferLength = strlen($this->buffer); } while ( + $continue && $bufferLength !== $iteratedBufferLength && $iteratedBufferLength > 0 && strpos($this->buffer, static::CRLF) !== false @@ -66,21 +68,30 @@ protected function iterateBuffer() if ($this->nextChunkIsLength) { $crlfPosition = strpos($this->buffer, static::CRLF); if ($crlfPosition === false) { - return; // Chunk header hasn't completely come in yet + return false; // Chunk header hasn't completely come in yet } $this->nextChunkIsLength = false; $lengthChunk = substr($this->buffer, 0, $crlfPosition); if (strpos($lengthChunk, ';') !== false) { list($lengthChunk) = explode(';', $lengthChunk, 2); } + $lengthChunk = trim($lengthChunk, "\r\n"); + if (!ctype_xdigit($lengthChunk)) { + $this->stream->close(); + $this->emit('error', [ + new Exception($this->buffer.'Unable to validate "' . $lengthChunk . '" as chunk length header"' . $crlfPosition), + ]); + return false; + } $this->remainingLength = hexdec($lengthChunk); $this->buffer = substr($this->buffer, $crlfPosition + 2); + return true; } if ($this->remainingLength > 0) { $chunkLength = $this->getChunkLength(); if ($chunkLength === 0) { - return; + return true; } $this->emit('data', array( substr($this->buffer, 0, $chunkLength), @@ -88,11 +99,12 @@ protected function iterateBuffer() )); $this->remainingLength -= $chunkLength; $this->buffer = substr($this->buffer, $chunkLength); - return; + return true; } $this->nextChunkIsLength = true; $this->buffer = substr($this->buffer, 2); + return true; } protected function getChunkLength() diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index 81ea9f4..c077f20 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -2,6 +2,7 @@ namespace React\Tests\HttpClient; +use Exception; use React\HttpClient\ChunkedStreamDecoder; use React\Stream\ThroughStream; @@ -42,14 +43,6 @@ public function provideChunkedEncoding() ["6\r\nWi\r\n", "ki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], "Wi\r\nkipedia in\r\n\r\nchunks." ], - [ - ["4\r\nWi\r\n", "ki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], - "Wi\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n" - ], - [ - str_split("4\r\nWi\r\nki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"), - "Wi\r\npedia in\r\n\r\nchunks." - ], ]; } @@ -62,12 +55,44 @@ public function testChunkedEncoding(array $strings, $expected = "Wikipedia in\r\ $stream = new ThroughStream(); $response = new ChunkedStreamDecoder($stream); $buffer = ''; - $response->on('data', function ($data, $response) use (&$buffer) { + $response->on('data', function ($data) use (&$buffer) { $buffer .= $data; }); + $response->on('error', function (Exception $exception) { + throw $exception; + }); foreach ($strings as $string) { $stream->write($string); } $this->assertSame($expected, $buffer); } + + public function provideInvalidChunkedEncoding() + { + return [ + [ + ["4\r\nWiwot40n98w3498tw3049nyn039409t34\r\n", "ki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], + ], + [ + str_split("xyz\r\nWi\r\nki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n") + ], + ]; + } + + /** + * @test + * @dataProvider provideInvalidChunkedEncoding + * @expectedException Exception + */ + public function testInvalidChunkedEncoding(array $strings) + { + $stream = new ThroughStream(); + $response = new ChunkedStreamDecoder($stream); + $response->on('error', function (Exception $exception) { + throw $exception; + }); + foreach ($strings as $string) { + $stream->write($string); + } + } } From 0b43386895f8595e1e1d2e8a28f29b1067255094 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Tue, 16 Aug 2016 21:45:45 +0200 Subject: [PATCH 27/46] Removed accidental committed debug code --- src/ChunkedStreamDecoder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index 64e6766..16967f0 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -79,7 +79,7 @@ protected function iterateBuffer() if (!ctype_xdigit($lengthChunk)) { $this->stream->close(); $this->emit('error', [ - new Exception($this->buffer.'Unable to validate "' . $lengthChunk . '" as chunk length header"' . $crlfPosition), + new Exception('Unable to validate "' . $lengthChunk . '" as chunk length header"'), ]); return false; } From 07f649ae9dedb60efd8258b89190ff7cf3e7bdc3 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Wed, 17 Aug 2016 07:31:29 +0200 Subject: [PATCH 28/46] Close decoder stream after emitting error via @clue https://github.com/reactphp/http-client/pull/58#discussion_r75012063 --- src/ChunkedStreamDecoder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index 16967f0..cfea7dc 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -77,10 +77,10 @@ protected function iterateBuffer() } $lengthChunk = trim($lengthChunk, "\r\n"); if (!ctype_xdigit($lengthChunk)) { - $this->stream->close(); $this->emit('error', [ new Exception('Unable to validate "' . $lengthChunk . '" as chunk length header"'), ]); + $this->close(); return false; } $this->remainingLength = hexdec($lengthChunk); From 3ca85d6761d2fb500423288b23dfc98a9ed0ebe3 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Wed, 17 Aug 2016 07:58:57 +0200 Subject: [PATCH 29/46] Implemented max content length limit, by default set at 32KB, va @clue https://github.com/reactphp/http-client/pull/58#discussion_r75013742 & https://github.com/reactphp/http-client/pull/58#discussion_r75014645 --- src/ChunkedStreamDecoder.php | 32 +++++++++++++++++++++++++++---- tests/DecodeChunkedStreamTest.php | 6 ++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index cfea7dc..d84258f 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -11,6 +11,7 @@ class ChunkedStreamDecoder implements ReadableStreamInterface { const CRLF = "\r\n"; + const MAX_BUFFER_LENGTH = 32768; // 32KB use EventEmitterTrait; @@ -24,6 +25,11 @@ class ChunkedStreamDecoder implements ReadableStreamInterface */ protected $remainingLength = 0; + /** + * @var int + */ + protected $maxBufferLength; + /** * @var bool */ @@ -36,14 +42,16 @@ class ChunkedStreamDecoder implements ReadableStreamInterface /** * @param ReadableStreamInterface $stream + * @param int $maxBufferLength */ - public function __construct(ReadableStreamInterface $stream) + public function __construct(ReadableStreamInterface $stream, $maxBufferLength = self::MAX_BUFFER_LENGTH) { $this->stream = $stream; $this->stream->on('data', array($this, 'handleData')); Util::forwardEvents($this->stream, $this, [ 'error', ]); + $this->maxBufferLength = $maxBufferLength; } /** @internal */ @@ -59,8 +67,17 @@ public function handleData($data) $continue && $bufferLength !== $iteratedBufferLength && $iteratedBufferLength > 0 && - strpos($this->buffer, static::CRLF) !== false + strpos($this->buffer, static::CRLF) !== false && + $iteratedBufferLength <= $this->maxBufferLength ); + + if ($iteratedBufferLength >= $this->maxBufferLength) { + $this->emit('error', [ + new Exception('The current buffer is longer then the ' . $this->maxBufferLength / 1024 . 'KB we have set as maximum'), + ]); + $this->close(); + return false; + } } protected function iterateBuffer() @@ -75,8 +92,8 @@ protected function iterateBuffer() if (strpos($lengthChunk, ';') !== false) { list($lengthChunk) = explode(';', $lengthChunk, 2); } - $lengthChunk = trim($lengthChunk, "\r\n"); - if (!ctype_xdigit($lengthChunk)) { + $lengthChunk = trim($lengthChunk); + if (dechex(hexdec($lengthChunk)) !== $lengthChunk) { $this->emit('error', [ new Exception('Unable to validate "' . $lengthChunk . '" as chunk length header"'), ]); @@ -84,6 +101,13 @@ protected function iterateBuffer() return false; } $this->remainingLength = hexdec($lengthChunk); + if ($this->remainingLength >= $this->maxBufferLength) { + $this->emit('error', [ + new Exception('Expected chunk length is longer then the ' . $this->maxBufferLength / 1024 . 'KB we have set as maximum'), + ]); + $this->close(); + return false; + } $this->buffer = substr($this->buffer, $crlfPosition + 2); return true; } diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index c077f20..b9e1cc7 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -76,6 +76,12 @@ public function provideInvalidChunkedEncoding() [ str_split("xyz\r\nWi\r\nki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n") ], + [ + str_split("1\r\n" . str_repeat('a', ChunkedStreamDecoder::MAX_BUFFER_LENGTH + 1) . "\r\n0\r\n\r\n") + ], + [ + [dechex(ChunkedStreamDecoder::MAX_BUFFER_LENGTH + 1) . "\r\n0\r\n\r\n"], + ], ]; } From 7f303d8f4fb46620565ffe04018c1c5ac0c1b953 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Wed, 17 Aug 2016 17:43:58 +0200 Subject: [PATCH 30/46] Don't iterate the buffer when it is smaller then three bytes (removed trimming) https://github.com/reactphp/http-client/pull/58#issuecomment-240343025 --- src/ChunkedStreamDecoder.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index d84258f..c55339c 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -82,6 +82,10 @@ public function handleData($data) protected function iterateBuffer() { + if (strlen($this->buffer) <= 2) { + return false; + } + if ($this->nextChunkIsLength) { $crlfPosition = strpos($this->buffer, static::CRLF); if ($crlfPosition === false) { @@ -92,7 +96,6 @@ protected function iterateBuffer() if (strpos($lengthChunk, ';') !== false) { list($lengthChunk) = explode(';', $lengthChunk, 2); } - $lengthChunk = trim($lengthChunk); if (dechex(hexdec($lengthChunk)) !== $lengthChunk) { $this->emit('error', [ new Exception('Unable to validate "' . $lengthChunk . '" as chunk length header"'), From 01ecdcb57e50503f57ef59bb6a8557e7585635f6 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Wed, 17 Aug 2016 17:45:14 +0200 Subject: [PATCH 31/46] Made it smaller then two instead of three, as two can still still be the last CRLF of the body --- src/ChunkedStreamDecoder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index c55339c..415c13d 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -82,7 +82,7 @@ public function handleData($data) protected function iterateBuffer() { - if (strlen($this->buffer) <= 2) { + if (strlen($this->buffer) <= 1) { return false; } From 37dfdf90f39f0e13986324b8261ba6d14d2d1b32 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 20 Aug 2016 13:25:39 +0200 Subject: [PATCH 32/46] Renamed $maxBufferLength to $maxChunkLength --- src/ChunkedStreamDecoder.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index 415c13d..12a1ad7 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -28,7 +28,7 @@ class ChunkedStreamDecoder implements ReadableStreamInterface /** * @var int */ - protected $maxBufferLength; + protected $maxChunkLength; /** * @var bool @@ -42,16 +42,16 @@ class ChunkedStreamDecoder implements ReadableStreamInterface /** * @param ReadableStreamInterface $stream - * @param int $maxBufferLength + * @param int $maxChunkLength */ - public function __construct(ReadableStreamInterface $stream, $maxBufferLength = self::MAX_BUFFER_LENGTH) + public function __construct(ReadableStreamInterface $stream, $maxChunkLength = self::MAX_BUFFER_LENGTH) { $this->stream = $stream; $this->stream->on('data', array($this, 'handleData')); Util::forwardEvents($this->stream, $this, [ 'error', ]); - $this->maxBufferLength = $maxBufferLength; + $this->maxChunkLength = $maxChunkLength; } /** @internal */ @@ -68,12 +68,12 @@ public function handleData($data) $bufferLength !== $iteratedBufferLength && $iteratedBufferLength > 0 && strpos($this->buffer, static::CRLF) !== false && - $iteratedBufferLength <= $this->maxBufferLength + $iteratedBufferLength <= $this->maxChunkLength ); - if ($iteratedBufferLength >= $this->maxBufferLength) { + if ($iteratedBufferLength >= $this->maxChunkLength) { $this->emit('error', [ - new Exception('The current buffer is longer then the ' . $this->maxBufferLength / 1024 . 'KB we have set as maximum'), + new Exception('The current buffer is longer then the ' . $this->maxChunkLength / 1024 . 'KB we have set as maximum'), ]); $this->close(); return false; @@ -104,9 +104,9 @@ protected function iterateBuffer() return false; } $this->remainingLength = hexdec($lengthChunk); - if ($this->remainingLength >= $this->maxBufferLength) { + if ($this->remainingLength >= $this->maxChunkLength) { $this->emit('error', [ - new Exception('Expected chunk length is longer then the ' . $this->maxBufferLength / 1024 . 'KB we have set as maximum'), + new Exception('Expected chunk length is longer then the ' . $this->maxChunkLength / 1024 . 'KB we have set as maximum'), ]); $this->close(); return false; From 61fb8a3aa28a0e81a8abf786e965de7571ce40d9 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 20 Aug 2016 13:59:47 +0200 Subject: [PATCH 33/46] Removed maximum chunk length as data is flowing through and not buffered in the decoder --- src/ChunkedStreamDecoder.php | 28 ++-------------------------- tests/DecodeChunkedStreamTest.php | 6 ------ 2 files changed, 2 insertions(+), 32 deletions(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index 12a1ad7..4de9aa8 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -11,7 +11,6 @@ class ChunkedStreamDecoder implements ReadableStreamInterface { const CRLF = "\r\n"; - const MAX_BUFFER_LENGTH = 32768; // 32KB use EventEmitterTrait; @@ -25,11 +24,6 @@ class ChunkedStreamDecoder implements ReadableStreamInterface */ protected $remainingLength = 0; - /** - * @var int - */ - protected $maxChunkLength; - /** * @var bool */ @@ -42,16 +36,14 @@ class ChunkedStreamDecoder implements ReadableStreamInterface /** * @param ReadableStreamInterface $stream - * @param int $maxChunkLength */ - public function __construct(ReadableStreamInterface $stream, $maxChunkLength = self::MAX_BUFFER_LENGTH) + public function __construct(ReadableStreamInterface $stream) { $this->stream = $stream; $this->stream->on('data', array($this, 'handleData')); Util::forwardEvents($this->stream, $this, [ 'error', ]); - $this->maxChunkLength = $maxChunkLength; } /** @internal */ @@ -67,17 +59,8 @@ public function handleData($data) $continue && $bufferLength !== $iteratedBufferLength && $iteratedBufferLength > 0 && - strpos($this->buffer, static::CRLF) !== false && - $iteratedBufferLength <= $this->maxChunkLength + strpos($this->buffer, static::CRLF) !== false ); - - if ($iteratedBufferLength >= $this->maxChunkLength) { - $this->emit('error', [ - new Exception('The current buffer is longer then the ' . $this->maxChunkLength / 1024 . 'KB we have set as maximum'), - ]); - $this->close(); - return false; - } } protected function iterateBuffer() @@ -104,13 +87,6 @@ protected function iterateBuffer() return false; } $this->remainingLength = hexdec($lengthChunk); - if ($this->remainingLength >= $this->maxChunkLength) { - $this->emit('error', [ - new Exception('Expected chunk length is longer then the ' . $this->maxChunkLength / 1024 . 'KB we have set as maximum'), - ]); - $this->close(); - return false; - } $this->buffer = substr($this->buffer, $crlfPosition + 2); return true; } diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index b9e1cc7..c077f20 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -76,12 +76,6 @@ public function provideInvalidChunkedEncoding() [ str_split("xyz\r\nWi\r\nki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n") ], - [ - str_split("1\r\n" . str_repeat('a', ChunkedStreamDecoder::MAX_BUFFER_LENGTH + 1) . "\r\n0\r\n\r\n") - ], - [ - [dechex(ChunkedStreamDecoder::MAX_BUFFER_LENGTH + 1) . "\r\n0\r\n\r\n"], - ], ]; } From e4bf1ea0257e6f6d7ff8a364bf6ed84f14700ef2 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 20 Aug 2016 14:34:46 +0200 Subject: [PATCH 34/46] Limit chunk header length to 1024 bytes https://github.com/reactphp/http-client/pull/58/files#r75577917 --- src/ChunkedStreamDecoder.php | 7 +++++++ tests/DecodeChunkedStreamTest.php | 3 +++ 2 files changed, 10 insertions(+) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index 4de9aa8..fa480ff 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -71,6 +71,13 @@ protected function iterateBuffer() if ($this->nextChunkIsLength) { $crlfPosition = strpos($this->buffer, static::CRLF); + if ($crlfPosition === false && strlen($this->buffer) > 1024) { + $this->emit('error', [ + new Exception('Chunk length header longer then 1024 bytes'), + ]); + $this->close(); + return false; + } if ($crlfPosition === false) { return false; // Chunk header hasn't completely come in yet } diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index c077f20..d64fce0 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -76,6 +76,9 @@ public function provideInvalidChunkedEncoding() [ str_split("xyz\r\nWi\r\nki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n") ], + [ + str_split(str_repeat('a', 2015) . "\r\nWi\r\nki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n") + ], ]; } From 69e33358decc01ec83bac9c83f6c67cbdf9b7616 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 20 Aug 2016 14:35:21 +0200 Subject: [PATCH 35/46] Unnecessary " --- src/ChunkedStreamDecoder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index fa480ff..1a512e6 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -88,7 +88,7 @@ protected function iterateBuffer() } if (dechex(hexdec($lengthChunk)) !== $lengthChunk) { $this->emit('error', [ - new Exception('Unable to validate "' . $lengthChunk . '" as chunk length header"'), + new Exception('Unable to validate "' . $lengthChunk . '" as chunk length header'), ]); $this->close(); return false; From 6aa86dbc6114b6733d36391801e19ffff17af9d3 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 20 Aug 2016 15:16:22 +0200 Subject: [PATCH 36/46] When the underlying stream ends emit an error when we have unprocessed data in our buffer https://github.com/reactphp/http-client/pull/58#discussion_r75577757 --- src/ChunkedStreamDecoder.php | 29 +++++++++++++++++++++++++++++ tests/DecodeChunkedStreamTest.php | 28 ++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index 1a512e6..db38c1c 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -34,6 +34,11 @@ class ChunkedStreamDecoder implements ReadableStreamInterface */ protected $stream; + /** + * @var bool + */ + protected $closed = false; + /** * @param ReadableStreamInterface $stream */ @@ -41,6 +46,7 @@ public function __construct(ReadableStreamInterface $stream) { $this->stream = $stream; $this->stream->on('data', array($this, 'handleData')); + $this->stream->on('end', array($this, 'handleEnd')); Util::forwardEvents($this->stream, $this, [ 'error', ]); @@ -152,6 +158,29 @@ public function pipe(WritableStreamInterface $dest, array $options = array()) public function close() { + $this->closed = true; return $this->stream->close(); } + + /** @internal */ + public function handleEnd() + { + if ($this->closed) { + return; + } + + if ($this->buffer === '') { + $this->emit('end'); + $this->close(); + return; + } + + $this->emit( + 'error', + [ + new Exception('Stream ended with incomplete control code') + ] + ); + $this->close(); + } } diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index d64fce0..ede1256 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -98,4 +98,32 @@ public function testInvalidChunkedEncoding(array $strings) $stream->write($string); } } + + public function testHandleEnd() + { + $ended = false; + $stream = new ThroughStream(); + $response = new ChunkedStreamDecoder($stream); + $response->on('end', function () use (&$ended) { + $ended = true; + }); + + $stream->end("4\r\nWiki\r\n0\r\n\r\n"); + + $this->assertTrue($ended); + } + + public function testHandleEndIncomplete() + { + $exception = null; + $stream = new ThroughStream(); + $response = new ChunkedStreamDecoder($stream); + $response->on('error', function ($e) use (&$exception) { + $exception = $e; + }); + + $stream->end("4\r\nWiki"); + + $this->assertInstanceOf('Exception', $exception); + } } From eab3c718427d7bf9bb227f3dfe32d1e149bb0e24 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 20 Aug 2016 15:28:07 +0200 Subject: [PATCH 37/46] In case a substr fails reinitialize buffer as empty string --- src/ChunkedStreamDecoder.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index db38c1c..ffe67e4 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -67,6 +67,10 @@ public function handleData($data) $iteratedBufferLength > 0 && strpos($this->buffer, static::CRLF) !== false ); + + if ($this->buffer === false) { + $this->buffer = ''; + } } protected function iterateBuffer() From 34ee75992223f07dd70d039fe294ec261140c1c9 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 20 Aug 2016 15:32:09 +0200 Subject: [PATCH 38/46] Added note about chunked encoded response to readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 554a145..75662e7 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ Interesting events emitted by Request: Interesting events emitted by Response: * `data`: Passes a chunk of the response body as first argument and a Response - object itself as second argument. + object itself as second argument. When a response encounters a chunked encoded response it will parse it transparently for the user of `Response` and removing the `Transfer-Encoding` header. * `error`: An error occurred. * `end`: The response has been fully received. If an error occurred, it is passed as first argument. From 4f955693c0bf8cff7b51a858708bd6a793924ccf Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sun, 21 Aug 2016 21:41:12 +0200 Subject: [PATCH 39/46] Use write instead of end to let the stream detect the end properly --- tests/DecodeChunkedStreamTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index ede1256..41d6a8b 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -108,7 +108,7 @@ public function testHandleEnd() $ended = true; }); - $stream->end("4\r\nWiki\r\n0\r\n\r\n"); + $stream->write("4\r\nWiki\r\n0\r\n\r\n"); $this->assertTrue($ended); } From 09cde2226a678d1fbc194d1cd859419c354a763a Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sun, 21 Aug 2016 21:55:32 +0200 Subject: [PATCH 40/46] End reading from this stream when the end of the chunked encoding has been found via @clue at https://github.com/reactphp/http-client/pull/58/files/01ecdcb57e50503f57ef59bb6a8557e7585635f6#r75576751 --- src/ChunkedStreamDecoder.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index ffe67e4..1e776c2 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -124,6 +124,13 @@ protected function iterateBuffer() $this->nextChunkIsLength = true; $this->buffer = substr($this->buffer, 2); + + if (substr($this->buffer, 0, 5) === "0\r\n\r\n") { + $this->stream->removeListener('data', array($this, 'handleData')); + $this->stream->removeListener('end', array($this, 'handleEnd')); + $this->emit('end'); + return false; + } return true; } From d7d047389b9cdd867a59f79bee0ae31ce8aec9ea Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Tue, 23 Aug 2016 17:23:40 +0200 Subject: [PATCH 41/46] Close stream regardless https://github.com/reactphp/http-client/pull/58#discussion_r75830632 --- src/ChunkedStreamDecoder.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index 1e776c2..0e0462f 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -126,9 +126,8 @@ protected function iterateBuffer() $this->buffer = substr($this->buffer, 2); if (substr($this->buffer, 0, 5) === "0\r\n\r\n") { - $this->stream->removeListener('data', array($this, 'handleData')); - $this->stream->removeListener('end', array($this, 'handleEnd')); $this->emit('end'); + $this->close(); return false; } return true; From 95277ff71a20f3bd471b42f75e0e145c5de2678f Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Tue, 23 Aug 2016 17:27:24 +0200 Subject: [PATCH 42/46] Prevent CRLF DOS https://github.com/reactphp/http-client/pull/58#discussion_r75013734 --- src/ChunkedStreamDecoder.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index 0e0462f..0fb7367 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -39,6 +39,11 @@ class ChunkedStreamDecoder implements ReadableStreamInterface */ protected $closed = false; + /** + * @var bool + */ + protected $reachedEnd = false; + /** * @param ReadableStreamInterface $stream */ @@ -64,8 +69,7 @@ public function handleData($data) } while ( $continue && $bufferLength !== $iteratedBufferLength && - $iteratedBufferLength > 0 && - strpos($this->buffer, static::CRLF) !== false + $iteratedBufferLength > 0 ); if ($this->buffer === false) { @@ -126,6 +130,7 @@ protected function iterateBuffer() $this->buffer = substr($this->buffer, 2); if (substr($this->buffer, 0, 5) === "0\r\n\r\n") { + $this->reachedEnd = true; $this->emit('end'); $this->close(); return false; @@ -179,7 +184,7 @@ public function handleEnd() return; } - if ($this->buffer === '') { + if ($this->buffer === '' && $this->reachedEnd) { $this->emit('end'); $this->close(); return; From 9d4896f381b7f1d7e65ea8746f5098342b037036 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Tue, 23 Aug 2016 17:45:55 +0200 Subject: [PATCH 43/46] Ignore chunked encoding trailers https://github.com/reactphp/http-client/pull/58/files/09cde2226a678d1fbc194d1cd859419c354a763a#r75830632 --- src/ChunkedStreamDecoder.php | 2 +- tests/DecodeChunkedStreamTest.php | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index 0fb7367..bdee1e6 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -129,7 +129,7 @@ protected function iterateBuffer() $this->nextChunkIsLength = true; $this->buffer = substr($this->buffer, 2); - if (substr($this->buffer, 0, 5) === "0\r\n\r\n") { + if (substr($this->buffer, 0, 3) === "0\r\n") { $this->reachedEnd = true; $this->emit('end'); $this->close(); diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index 41d6a8b..c689fd7 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -126,4 +126,18 @@ public function testHandleEndIncomplete() $this->assertInstanceOf('Exception', $exception); } + + public function testHandleEndTrailers() + { + $ended = false; + $stream = new ThroughStream(); + $response = new ChunkedStreamDecoder($stream); + $response->on('end', function () use (&$ended) { + $ended = true; + }); + + $stream->write("4\r\nWiki\r\n0\r\nabc: def\r\nghi: klm\r\n\r\n"); + + $this->assertTrue($ended); + } } From 8540b4e0d5b87bb05b12649e4801677b3e4f8492 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Tue, 13 Sep 2016 09:35:51 +0200 Subject: [PATCH 44/46] Mark ChunkedStreamDecoder as internal --- src/ChunkedStreamDecoder.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ChunkedStreamDecoder.php b/src/ChunkedStreamDecoder.php index bdee1e6..d5cd8a9 100644 --- a/src/ChunkedStreamDecoder.php +++ b/src/ChunkedStreamDecoder.php @@ -8,6 +8,9 @@ use React\Stream\Util; use React\Stream\WritableStreamInterface; +/** + * @internal + */ class ChunkedStreamDecoder implements ReadableStreamInterface { const CRLF = "\r\n"; From 415d79089ad7a878567a4fa8b831673a5ba11fce Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Tue, 13 Sep 2016 10:23:14 +0200 Subject: [PATCH 45/46] Gave names to exception test data --- tests/DecodeChunkedStreamTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index c689fd7..512eb1f 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -70,13 +70,13 @@ public function testChunkedEncoding(array $strings, $expected = "Wikipedia in\r\ public function provideInvalidChunkedEncoding() { return [ - [ + 'chunk-body-longer-than-header-suggests' => [ ["4\r\nWiwot40n98w3498tw3049nyn039409t34\r\n", "ki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], ], - [ + 'invalid-header-charactrrs' => [ str_split("xyz\r\nWi\r\nki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n") ], - [ + 'header-chunk-to-long' => [ str_split(str_repeat('a', 2015) . "\r\nWi\r\nki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n") ], ]; From 49e9e9eabed2558467b986b2050ecd7f0b392a66 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Tue, 13 Sep 2016 16:38:51 +0200 Subject: [PATCH 46/46] Gave names to non-exception test data --- tests/DecodeChunkedStreamTest.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/DecodeChunkedStreamTest.php b/tests/DecodeChunkedStreamTest.php index 512eb1f..e227542 100644 --- a/tests/DecodeChunkedStreamTest.php +++ b/tests/DecodeChunkedStreamTest.php @@ -11,35 +11,35 @@ class DecodeChunkedStreamTest extends TestCase public function provideChunkedEncoding() { return [ - [ + 'data-set-1' => [ ["4\r\nWiki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], ], - [ + 'data-set-2' => [ ["4\r\nWiki\r\n", "5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], ], - [ + 'data-set-3' => [ ["4\r\nWiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], ], - [ + 'data-set-4' => [ ["4\r\nWiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n", "\r\nchunks.\r\n0\r\n\r\n"], ], - [ + 'data-set-5' => [ ["4\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne\r\n in\r\n", "\r\nchunks.\r\n0\r\n\r\n"], ], - [ + 'data-set-6' => [ ["4\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne; foo=[bar,beer,pool,cue,win,won]\r\n", " in\r\n", "\r\nchunks.\r\n0\r\n\r\n"], ], - [ + 'header-fields' => [ ["4; foo=bar\r\n", "Wiki\r\n", "5\r\n", "pedia\r\ne\r\n", " in\r\n", "\r\nchunks.\r\n", "0\r\n\r\n"], ], - [ + 'character-for-charactrr' => [ str_split("4\r\nWiki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"), ], - [ + 'extra-newline-in-wiki-character-for-chatacter' => [ str_split("6\r\nWi\r\nki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"), "Wi\r\nkipedia in\r\n\r\nchunks." ], - [ + 'extra-newline-in-wiki' => [ ["6\r\nWi\r\n", "ki\r\n5\r\npedia\r\ne\r\n in\r\n\r\nchunks.\r\n0\r\n\r\n"], "Wi\r\nkipedia in\r\n\r\nchunks." ],