From f97a4af4638f7fea76f2fefdbe2a808aeb6aca4f Mon Sep 17 00:00:00 2001 From: Stephen Holdaway Date: Sun, 4 May 2014 15:45:30 +1200 Subject: [PATCH 1/3] Refactor decoder to read from an interface instead of a string Using a string as input to the decoder meant it was required to read the whole length of data to decode into memory, which meant decoding was a potentially very heavy on memory. This change abstracts out the input to the decoder, and adds a DataSource called String to replace it. This change breaks backwards compatibility. --- src/DataSource.php | 30 ++++++++++++ src/DataSource/String.php | 59 +++++++++++++++++++++++ src/Decoder.php | 98 +++++++++++++++++++++------------------ test/unit/StringTest.php | 55 ++++++++++++++++++++++ 4 files changed, 197 insertions(+), 45 deletions(-) create mode 100644 src/DataSource.php create mode 100644 src/DataSource/String.php create mode 100644 test/unit/StringTest.php diff --git a/src/DataSource.php b/src/DataSource.php new file mode 100644 index 0000000..9e5d1c6 --- /dev/null +++ b/src/DataSource.php @@ -0,0 +1,30 @@ + + * @copyright Copyright (c) 2014, Ryan Chouinard + * @license MIT License - http://www.opensource.org/licenses/mit-license.php + */ + +namespace Rych\Bencode; + +interface DataSource +{ + /** + * Get the length of the data source + * + * @return integer + */ + public function getLength(); + + /** + * Read bytes from the data source + * + * @param integer $offset + * @param integer $length + * @return string + */ + public function read($offset, $length = null); +} \ No newline at end of file diff --git a/src/DataSource/String.php b/src/DataSource/String.php new file mode 100644 index 0000000..f64e27a --- /dev/null +++ b/src/DataSource/String.php @@ -0,0 +1,59 @@ + + * @copyright Copyright (c) 2014, Ryan Chouinard + * @license MIT License - http://www.opensource.org/licenses/mit-license.php + */ + +namespace Rych\Bencode\DataSource; + +use Rych\Bencode\DataSource; +use Rych\Bencode\Exception\RuntimeException; + +class String implements DataSource +{ + protected $string; + + protected $length; + + public function __construct($string) + { + if (!is_string($string)) { + throw new RuntimeException('Expected string, got '. gettype($string)); + } + + $this->string = $string; + } + + public function getLength() + { + if (is_null($this->length)) { + $this->length = strlen($this->string); + } + + return $this->length; + } + + public function read($offset, $length = 1) + { + $length = intval($length); + + if ($offset < 0 || $offset > $this->getLength() - 1) { + throw new RuntimeException('Invalid offset'); + } + + if ($length < 1) { + throw new RuntimeException('Invalid length'); + } + + if ($length === 1) { + return $this->string[$offset]; + } else { + return substr($this->string, $offset, $length); + } + } +} \ No newline at end of file diff --git a/src/Decoder.php b/src/Decoder.php index 69fbc63..5bb04da 100644 --- a/src/Decoder.php +++ b/src/Decoder.php @@ -27,28 +27,28 @@ class Decoder * * @var string */ - private $source; + protected $source; /** * The length of the encoded source string * * @var integer */ - private $sourceLength; + protected $decodeType; /** * The return type for the decoded value * * @var Bencode::TYPE_ARRAY|Bencode::TYPE_OBJECT */ - private $decodeType; + protected $sourceLength; /** * The current offset of the parser. * * @var integer */ - private $offset = 0; + protected $offset = 0; /** * Decoder constructor @@ -58,13 +58,14 @@ class Decoder * value should be returned as an object or an array. * @return void */ - private function __construct($source, $decodeType) + protected function __construct(DataSource $source, $decodeType) { $this->source = $source; - $this->sourceLength = strlen($this->source); - $this->decodeType = in_array($decodeType, array(Bencode::TYPE_ARRAY, Bencode::TYPE_OBJECT)) - ? $decodeType - : Bencode::TYPE_ARRAY; + $this->sourceLength = $source->getLength($source); + if ($decodeType != Bencode::TYPE_ARRAY && $decodeType != Bencode::TYPE_OBJECT) { + $decodeType = Bencode::TYPE_ARRAY; + } + $this->decodeType = $decodeType; } /** @@ -76,12 +77,8 @@ private function __construct($source, $decodeType) * @return mixed Returns the appropriate data type for the decoded data. * @throws RuntimeException */ - public static function decode($source, $decodeType = Bencode::TYPE_ARRAY) + public static function decode(DataSource $source, $decodeType = Bencode::TYPE_ARRAY) { - if (!is_string($source)) { - throw new RuntimeException("Argument expected to be a string; Got " . gettype($source)); - } - $decoder = new self($source, $decodeType); $decoded = $decoder->doDecode(); @@ -98,7 +95,7 @@ public static function decode($source, $decodeType = Bencode::TYPE_ARRAY) * @return mixed Returns the decoded value. * @throws RuntimeException */ - private function doDecode() + protected function doDecode() { switch ($this->getChar()) { @@ -130,30 +127,29 @@ private function doDecode() * @return integer Returns the decoded integer. * @throws RuntimeException */ - private function decodeInteger() + protected function decodeInteger() { - $offsetOfE = strpos($this->source, "e", $this->offset); - if (false === $offsetOfE) { - throw new RuntimeException("Unterminated integer entity at offset $this->offset"); - } - $currentOffset = $this->offset; - if ("-" == $this->getChar($currentOffset)) { + $value = ''; + + if ('-' == $this->getChar($currentOffset)) { + $value = '-'; ++$currentOffset; } - if ($offsetOfE === $currentOffset) { - throw new RuntimeException("Empty integer entity at offset $this->offset"); - } + $char = $this->getChar($currentOffset); - while ($currentOffset < $offsetOfE) { - if (!ctype_digit($this->getChar($currentOffset))) { - throw new RuntimeException("Non-numeric character found in integer entity at offset $this->offset"); + while ($char !== 'e') { + if (!ctype_digit($char)) { + throw new RuntimeException('Non-numeric character found in integer entity at offset ' . $this->offset); } - ++$currentOffset; + $value .= $char; + $char = $this->getChar(++$currentOffset); } - $value = substr($this->source, $this->offset, $offsetOfE - $this->offset); + if (trim($value,'-') === '') { + throw new RuntimeException('Integer was empty at offset ' . $this->offset); + } // One last check to make sure zero-padded integers don't slip by, as // they're not allowed per bencode specification. @@ -162,7 +158,7 @@ private function decodeInteger() throw new RuntimeException("Illegal zero-padding found in integer entity at offset $this->offset"); } - $this->offset = $offsetOfE + 1; + $this->offset += $currentOffset; // The +0 auto-casts the chunk to either an integer or a float(in cases // where an integer would overrun the max limits of integer types) @@ -175,24 +171,31 @@ private function decodeInteger() * @return string Returns the decoded string. * @throws RuntimeException */ - private function decodeString() + protected function decodeString() { if ("0" === $this->getChar() && ":" != $this->getChar($this->offset + 1)) { throw new RuntimeException("Illegal zero-padding in string entity length declaration at offset $this->offset"); } - $offsetOfColon = strpos($this->source, ":", $this->offset); - if (false === $offsetOfColon) { - throw new RuntimeException("Unterminated string entity at offset $this->offset"); + $char = $this->getChar(); + $length = ''; + + while ($char !== ':') { + if (!ctype_digit($char)) { + throw new RuntimeException('Unterminated string entity at offset ' . $this->offset); + } + $length .= $char; + $char = $this->getChar(++$this->offset); } - $contentLength = (int) substr($this->source, $this->offset, $offsetOfColon); - if (($contentLength + $offsetOfColon + 1) > $this->sourceLength) { - throw new RuntimeException("Unexpected end of string entity at offset $this->offset"); + $length = (int) $length; + + if (($length + $this->offset + 1) > $this->sourceLength) { + throw new RuntimeException('Unexpected end of string entity at offset ' . $this->offset); } - $value = substr($this->source, $offsetOfColon + 1, $contentLength); - $this->offset = $offsetOfColon + $contentLength + 1; + $value = $this->getChar(++$this->offset, $length); + $this->offset += $length; return $value; } @@ -203,7 +206,7 @@ private function decodeString() * @return array Returns the decoded array. * @throws RuntimeException */ - private function decodeList() + protected function decodeList() { $list = array(); $terminated = false; @@ -233,7 +236,7 @@ private function decodeList() * @return array Returns the decoded array. * @throws RuntimeException */ - private function decodeDict() + protected function decodeDict() { $dict = array(); $terminated = false; @@ -276,17 +279,22 @@ private function decodeDict() * @return string|false Returns the character found at the specified * offset. If the specified offset is out of range, FALSE is returned. */ - private function getChar($offset = null) + protected function getChar($offset = null, $length = 1) { if (null === $offset) { $offset = $this->offset; } - if (empty ($this->source) || $this->offset >= $this->sourceLength) { + if ($this->offset >= $this->sourceLength) { return false; } - return $this->source[$offset]; + return $this->source->read($offset, $length); + } + + protected function getLength($source) + { + return strlen($source); } } diff --git a/test/unit/StringTest.php b/test/unit/StringTest.php new file mode 100644 index 0000000..3928650 --- /dev/null +++ b/test/unit/StringTest.php @@ -0,0 +1,55 @@ + + * @license MIT License - http://www.opensource.org/licenses/mit-license.php + */ + +namespace Rych\Bencode; + +use PHPUnit_Framework_TestCase as TestCase; +use Rych\Bencode\DataSource\String; + +class StringTest extends TestCase +{ + public function testGetChar() + { + $raw = 'hello world'; + $string = new String($raw); + + $this->assertEquals( + 'hello', + $string->read(0, 5) + ); + + $this->assertEquals( + 'world', + $string->read(6, 5) + ); + + $this->assertEquals( + 'ello wo', + $string->read(1, 7) + ); + + for ($i = 0, $c = strlen($raw); $i < $c; $i++) { + $this->assertEquals( + $raw[$i], + $string->read($i) + ); + } + } + + public function testGetLength() + { + $raw = 'hello world'; + $string = new String($raw); + + $this->assertEquals(11, $string->getLength()); + } +} \ No newline at end of file From fd1b271fbb59c76ed92eae16c4bc66cede291cff Mon Sep 17 00:00:00 2001 From: Stephen Holdaway Date: Sun, 4 May 2014 15:47:42 +1200 Subject: [PATCH 2/3] Add file handle data source This data source provides a way to decode a bencoded a file with only a handle to that file. This avoids the entire file into memory. --- README.md | 9 +++-- src/DataSource/FileHandle.php | 62 +++++++++++++++++++++++++++++++++++ src/Decoder.php | 2 +- test/FileHandleTest.php | 49 +++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 3 deletions(-) create mode 100644 src/DataSource/FileHandle.php create mode 100644 test/FileHandleTest.php diff --git a/README.md b/README.md index 7a8a80f..e57798a 100644 --- a/README.md +++ b/README.md @@ -49,11 +49,13 @@ The above outputs the bencode encoded string `d5:arrayl3:one3:two5:threee7:integ ```php + * @copyright Copyright (c) 2014, Ryan Chouinard + * @license MIT License - http://www.opensource.org/licenses/mit-license.php + */ + +namespace Rych\Bencode\DataSource; + +use Rych\Bencode\DataSource; +use Rych\Bencode\Exception\RuntimeException; + +class FileHandle implements DataSource +{ + protected $handle; + + protected $length; + + public function __construct($handle) + { + if (!is_resource($handle)) { + throw new RuntimeException('Expected resource, got '. gettype($string)); + } + + $this->handle = $handle; + } + + public function getLength() + { + if (is_null($this->length)) { + $stat = fstat($this->handle); + $this->length = $stat['size']; + } + + return $this->length; + } + + public function read($offset, $length = 1) + { + $length = intval($length); + + if ($offset < 0 || $offset > $this->getLength() - 1) { + throw new RuntimeException('Invalid offset'); + } + + if ($length < 1) { + throw new RuntimeException('Invalid length'); + } + + fseek($this->handle, $offset); + + if ($length === 1) { + return fgetc($this->handle); + } else { + return fread($this->handle, $length); + } + } +} \ No newline at end of file diff --git a/src/Decoder.php b/src/Decoder.php index 5bb04da..bbe4751 100644 --- a/src/Decoder.php +++ b/src/Decoder.php @@ -158,7 +158,7 @@ protected function decodeInteger() throw new RuntimeException("Illegal zero-padding found in integer entity at offset $this->offset"); } - $this->offset += $currentOffset; + $this->offset = $currentOffset + 1; // The +0 auto-casts the chunk to either an integer or a float(in cases // where an integer would overrun the max limits of integer types) diff --git a/test/FileHandleTest.php b/test/FileHandleTest.php new file mode 100644 index 0000000..5422bee --- /dev/null +++ b/test/FileHandleTest.php @@ -0,0 +1,49 @@ +assertEquals( + 'hello', + $file->read(0, 5) + ); + + $this->assertEquals( + 'world', + $file->read(6, 5) + ); + + for ($i = 0, $c = strlen($raw); $i < $c; $i++) { + $this->assertEquals( + $raw[$i], + $file->read($i) + ); + } + + fclose($handle); + } + + public function testGetLength() + { + $raw = 'hello world'; + $handle = fopen('php://memory', 'w+'); + fwrite($handle, $raw); + + $file = new FileHandle($handle); + + $this->assertEquals(11, $file->getLength()); + + fclose($handle); + } +} \ No newline at end of file From 97363712cabda4f825284936090bb26821663969 Mon Sep 17 00:00:00 2001 From: Stephen Holdaway Date: Fri, 12 Dec 2014 12:26:32 +1300 Subject: [PATCH 3/3] Make the addition of the DataSource interface backwards compatible The addition of the DataSource interface originally changed the signature of `Bencode::decode` to use a type hint. This broke backwards compatibility (the function previously took a string) and was clunky to use. This change implicitly converts non-DataSource inputs to `DataSource\String`. --- README.md | 18 +++++++++++++++--- src/Decoder.php | 21 ++++++++++++++++----- test/{ => unit}/FileHandleTest.php | 16 +++++++++++++++- test/unit/StringTest.php | 13 +++++++++++++ 4 files changed, 59 insertions(+), 9 deletions(-) rename test/{ => unit}/FileHandleTest.php (74%) diff --git a/README.md b/README.md index e57798a..ce35d4d 100644 --- a/README.md +++ b/README.md @@ -50,12 +50,11 @@ The above outputs the bencode encoded string `d5:arrayl3:one3:two5:threee7:integ source = $source; $this->sourceLength = $source->getLength($source); if ($decodeType != Bencode::TYPE_ARRAY && $decodeType != Bencode::TYPE_OBJECT) { @@ -77,8 +84,12 @@ protected function __construct(DataSource $source, $decodeType) * @return mixed Returns the appropriate data type for the decoded data. * @throws RuntimeException */ - public static function decode(DataSource $source, $decodeType = Bencode::TYPE_ARRAY) + public static function decode($source, $decodeType = Bencode::TYPE_ARRAY) { + if (!$source instanceof DataSource) { + $source = new String($source); + } + $decoder = new self($source, $decodeType); $decoded = $decoder->doDecode(); diff --git a/test/FileHandleTest.php b/test/unit/FileHandleTest.php similarity index 74% rename from test/FileHandleTest.php rename to test/unit/FileHandleTest.php index 5422bee..7d253fb 100644 --- a/test/FileHandleTest.php +++ b/test/unit/FileHandleTest.php @@ -2,10 +2,19 @@ namespace Rych\Bencode; +use PHPUnit_Framework_TestCase as TestCase; use Rych\Bencode\DataSource\FileHandle; -class FileHandleTest extends \PHPUnit_Framework_TestCase +/** + * Bencode file data source test + */ +class FileHandleTest extends TestCase { + /** + * Test that groups of characters can be read from a file + * + * @test + */ public function testGetChar() { $raw = 'hello world'; @@ -34,6 +43,11 @@ public function testGetChar() fclose($handle); } + /** + * Test that a file's length is reported correctly + * + * @test + */ public function testGetLength() { $raw = 'hello world'; diff --git a/test/unit/StringTest.php b/test/unit/StringTest.php index 3928650..335b8f1 100644 --- a/test/unit/StringTest.php +++ b/test/unit/StringTest.php @@ -15,8 +15,16 @@ use PHPUnit_Framework_TestCase as TestCase; use Rych\Bencode\DataSource\String; +/** + * Bencode string data source test + */ class StringTest extends TestCase { + /** + * Test that groups of characters can be read from a string + * + * @test + */ public function testGetChar() { $raw = 'hello world'; @@ -45,6 +53,11 @@ public function testGetChar() } } + /** + * Test that a string's length is reported correctly + * + * @test + */ public function testGetLength() { $raw = 'hello world';