diff --git a/apps/dav/lib/Upload/AssemblyStream.php b/apps/dav/lib/Upload/AssemblyStream.php index 5853adfa77..3a18f91677 100644 --- a/apps/dav/lib/Upload/AssemblyStream.php +++ b/apps/dav/lib/Upload/AssemblyStream.php @@ -50,6 +50,9 @@ class AssemblyStream implements \Icewind\Streams\File { /** @var int */ private $size; + /** @var resource */ + private $currentStream = null; + /** * @param string $path * @param string $mode @@ -102,16 +105,36 @@ class AssemblyStream implements \Icewind\Streams\File { * @return string */ public function stream_read($count) { + do { + if ($this->currentStream === null) { + list($node, $posInNode) = $this->getNodeForPosition($this->pos); + if (is_null($node)) { + // reached last node, no more data + return ''; + } + $this->currentStream = $this->getStream($node); + fseek($this->currentStream, $posInNode); + } - list($node, $posInNode) = $this->getNodeForPosition($this->pos); - if (is_null($node)) { - return null; - } - $stream = $this->getStream($node); + $data = fread($this->currentStream, $count); + // isset is faster than strlen + if (isset($data[$count - 1])) { + // we read the full count + $read = $count; + } else { + // reaching end of stream, which happens less often so strlen is ok + $read = strlen($data); + } - fseek($stream, $posInNode); - $data = fread($stream, $count); - $read = strlen($data); + if (feof($this->currentStream)) { + fclose($this->currentStream); + $this->currentNode = null; + $this->currentStream = null; + } + // if no data read, try again with the next node because + // returning empty data can make the caller think there is no more + // data left to read + } while ($read === 0); // update position $this->pos += $read; diff --git a/apps/dav/tests/unit/Upload/AssemblyStreamTest.php b/apps/dav/tests/unit/Upload/AssemblyStreamTest.php index ea875886af..66aea40ef2 100644 --- a/apps/dav/tests/unit/Upload/AssemblyStreamTest.php +++ b/apps/dav/tests/unit/Upload/AssemblyStreamTest.php @@ -35,18 +35,78 @@ class AssemblyStreamTest extends \Test\TestCase { $this->assertEquals($expected, $content); } + /** + * @dataProvider providesNodes() + */ + public function testGetContentsFread($expected, $nodes) { + $stream = \OCA\DAV\Upload\AssemblyStream::wrap($nodes); + + $content = ''; + while (!feof($stream)) { + $content .= fread($stream, 3); + } + + $this->assertEquals($expected, $content); + } + function providesNodes() { + $data8k = $this->makeData(8192); + $dataLess8k = $this->makeData(8191); return[ - 'one node only' => ['1234567890', [ + 'one node zero bytes' => [ + '', [ + $this->buildNode('0', '') + ]], + 'one node only' => [ + '1234567890', [ $this->buildNode('0', '1234567890') ]], - 'two nodes' => ['1234567890', [ + 'one node buffer boundary' => [ + $data8k, [ + $this->buildNode('0', $data8k) + ]], + 'two nodes' => [ + '1234567890', [ $this->buildNode('1', '67890'), $this->buildNode('0', '12345') - ]] + ]], + 'two nodes end on buffer boundary' => [ + $data8k . $data8k, [ + $this->buildNode('1', $data8k), + $this->buildNode('0', $data8k) + ]], + 'two nodes with one on buffer boundary' => [ + $data8k . $dataLess8k, [ + $this->buildNode('1', $dataLess8k), + $this->buildNode('0', $data8k) + ]], + 'two nodes on buffer boundary plus one byte' => [ + $data8k . 'X' . $data8k, [ + $this->buildNode('1', $data8k), + $this->buildNode('0', $data8k . 'X') + ]], + 'two nodes on buffer boundary plus one byte at the end' => [ + $data8k . $data8k . 'X', [ + $this->buildNode('1', $data8k . 'X'), + $this->buildNode('0', $data8k) + ]], ]; } + private function makeData($count) { + $data = ''; + $base = '1234567890'; + $j = 0; + for ($i = 0; $i < $count; $i++) { + $data .= $base[$j]; + $j++; + if (!isset($base[$j])) { + $j = 0; + } + } + return $data; + } + private function buildNode($name, $data) { $node = $this->getMockBuilder('\Sabre\DAV\File') ->setMethods(['getName', 'get', 'getSize'])