Fix downloading Ranges in files.

Correctly handle ranges that don't terminate at the end of the file.
Also reject invalid ranges as described in RFC-2616.

Thanks to Kim Biesbjerg for the initial patch.

Fixes #3914

lib/Cake/Network/CakeResponse.php

@@ -345,6 +345,13 @@ class CakeResponse {
  */
 	protected $_file = null;
 
+/**
+ * File range. Used for requesting ranges of files.
+ *
+ * @var array
+ */
+	protected $_fileRange = null;
+
 /**
  * The charset the response body is encoded with
  *
@@ -413,8 +420,8 @@ class CakeResponse {
 			$this->_sendHeader($header, $value);
 		}
 		if ($this->_file) {
-			$this->_sendFile($this->_file);
+			$this->_sendFile($this->_file, $this->_fileRange);
-			$this->_file = null;
+			$this->_file = $this->_fileRange = null;
 		} else {
 			$this->_sendContent($this->_body);
 		}
@@ -1268,19 +1275,7 @@ class CakeResponse {
 
 			$httpRange = env('HTTP_RANGE');
 			if (isset($httpRange)) {
-				list(, $range) = explode('=', $httpRange);
+				$this->_fileRange($file, $httpRange);
-
-				$size = $fileSize - 1;
-				$length = $fileSize - $range;
-
-				$this->header(array(
-					'Content-Length' => $length,
-					'Content-Range' => 'bytes ' . $range . $size . '/' . $fileSize
-				));
-
-				$this->statusCode(206);
-				$file->open('rb', true);
-				$file->offset($range);
 			} else {
 				$this->header('Content-Length', $fileSize);
 			}
@@ -1291,22 +1286,73 @@ class CakeResponse {
 		$this->_file = $file;
 	}
 
+/**
+ * Apply a file range to a file and set the end offset.
+ *
+ * If an invalid range is requested a 416 Status code will be used
+ * in the response.
+ *
+ * @param File $file The file to set a range on.
+ * @param string $httpRange The range to use.
+ * @return void
+ */
+	protected function _fileRange($file, $httpRange) {
+		list(, $range) = explode('=', $httpRange);
+		list($start, $end) = explode('-', $range);
+
+		$fileSize = $file->size();
+		$lastByte = $fileSize - 1;
+		if ($start > $end || $end > $lastByte || $start > $lastByte) {
+			$this->statusCode(416);
+			$this->header(array(
+				'Content-Range' => 'bytes 0-' . $lastByte . '/' . $fileSize
+			));
+			return;
+		}
+
+		$this->header(array(
+			'Content-Length' => $end - $start + 1,
+			'Content-Range' => 'bytes ' . $start . '-' . $end . '/' . $fileSize
+		));
+
+		$this->statusCode(206);
+		$this->_fileRange = array($start, $end);
+	}
+
 /**
  * Reads out a file, and echos the content to the client.
  *
  * @param File $file File object
+ * @param array $range The range to read out of the file.
  * @return boolean True is whole file is echoed successfully or false if client connection is lost in between
  */
-	protected function _sendFile($file) {
+	protected function _sendFile($file, $range) {
 		$compress = $this->outputCompressed();
 		$file->open('rb');
+
+		$end = $start = false;
+		if ($range) {
+			list($start, $end) = $range;
+		}
+		if ($start !== false) {
+			$file->offset($start);
+		}
+
+		$bufferSize = 8192;
+		set_time_limit(0);
 		while (!feof($file->handle)) {
 			if (!$this->_isActive()) {
 				$file->close();
 				return false;
 			}
-			set_time_limit(0);
+			$offset = $file->offset();
-			echo fread($file->handle, 8192);
+			if ($end && $offset >= $end) {
+				break;
+			}
+			if ($end && $offset + $bufferSize >= $end) {
+				$bufferSize = $end - $offset;
+			}
+			echo fread($file->handle, $bufferSize);
 			if (!$compress) {
 				$this->_flushBuffer();
 			}

lib/Cake/Test/Case/Network/CakeResponseTest.php

@@ -1,9 +1,5 @@
 <?php
 /**
- * CakeResponse Test case file.
- *
- * PHP 5
- *
  * CakePHP(tm) : Rapid Development Framework (http://cakephp.org)
  * Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
  *
@@ -1389,4 +1385,100 @@ class CakeResponseTest extends CakeTestCase {
 		$response->file(CAKE . 'Test' . DS . 'test_app' . DS . 'Vendor' . DS . 'img' . DS . 'test_2.JPG');
 	}
 
+/**
+ * Test fetching ranges from a file.
+ *
+ * @return void
+ */
+	public function testFileRange() {
+		$_SERVER['HTTP_RANGE'] = 'bytes=8-25';
+		$response = $this->getMock('CakeResponse', array(
+			'header',
+			'type',
+			'_sendHeader',
+			'_setContentType',
+			'_isActive',
+			'_clearBuffer',
+			'_flushBuffer'
+		));
+
+		$response->expects($this->exactly(1))
+			->method('type')
+			->with('css')
+			->will($this->returnArgument(0));
+
+		$response->expects($this->at(1))
+			->method('header')
+			->with('Content-Disposition', 'attachment; filename="test_asset.css"');
+
+		$response->expects($this->at(2))
+			->method('header')
+			->with('Accept-Ranges', 'bytes');
+
+		$response->expects($this->at(3))
+			->method('header')
+			->with(array(
+				'Content-Length' => 18,
+				'Content-Range' => 'bytes 8-25/38',
+			));
+
+		$response->expects($this->once())->method('_clearBuffer');
+
+		$response->expects($this->any())
+			->method('_isActive')
+			->will($this->returnValue(true));
+
+		$response->file(
+			CAKE . 'Test' . DS . 'test_app' . DS . 'Vendor' . DS . 'css' . DS . 'test_asset.css',
+			array('download' => true)
+		);
+
+		ob_start();
+		$result = $response->send();
+		$output = ob_get_clean();
+		$this->assertEquals(206, $response->statusCode());
+		$this->assertEquals("is the test asset", $output);
+		$this->assertTrue($result !== false);
+	}
+
+/**
+ * Test invalid file ranges.
+ *
+ * @return void
+ */
+	public function testFileRangeInvalid() {
+		$_SERVER['HTTP_RANGE'] = 'bytes=30-2';
+		$response = $this->getMock('CakeResponse', array(
+			'header',
+			'type',
+			'_sendHeader',
+			'_setContentType',
+			'_isActive',
+			'_clearBuffer',
+			'_flushBuffer'
+		));
+
+		$response->expects($this->at(1))
+			->method('header')
+			->with('Content-Disposition', 'attachment; filename="test_asset.css"');
+
+		$response->expects($this->at(2))
+			->method('header')
+			->with('Accept-Ranges', 'bytes');
+
+		$response->expects($this->at(3))
+			->method('header')
+			->with(array(
+				'Content-Range' => 'bytes 0-37/38',
+			));
+
+		$response->file(
+			CAKE . 'Test' . DS . 'test_app' . DS . 'Vendor' . DS . 'css' . DS . 'test_asset.css',
+			array('download' => true)
+		);
+
+		$this->assertEquals(416, $response->statusCode());
+		$result = $response->send();
+	}
+
 }