Backport range parsing resiliancy fixes from 3.x

Refs #8723
cakephp · Apr 29, 2016 · cf55767 · cf55767
1 parent cbd5425
commit cf55767
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 28 deletions.
diff --git a/lib/Cake/Network/CakeResponse.php b/lib/Cake/Network/CakeResponse.php
@@ -1406,11 +1406,16 @@ public function file($path, $options = array()) {
  * @return void
  */
 	protected function _fileRange($file, $httpRange) {
-		list(, $range) = explode('=', $httpRange);
-		list($start, $end) = explode('-', $range);
-
 		$fileSize = $file->size();
 		$lastByte = $fileSize - 1;
+		$start = 0;
+		$end = $lastByte;
+
+		preg_match('/^bytes\s*=\s*(\d+)?\s*-\s*(\d+)?$/', $httpRange, $matches);
+		if ($matches) {
+			$start = $matches[1];
+			$end = isset($matches[2]) ? $matches[2] : '';
+		}
 
 		if ($start === '') {
 			$start = $fileSize - $end;

diff --git a/lib/Cake/Test/Case/Network/CakeResponseTest.php b/lib/Cake/Test/Case/Network/CakeResponseTest.php
@@ -1705,48 +1705,81 @@ public function testFileRange() {
 		$this->assertNotSame(false, $result);
 	}
 
+/**
+ * Provider for invalid range header values.
+ *
+ * @return array
+ */
+	public function invalidFileRangeProvider() {
+		return array(
+			// malformed range
+			array(
+				'bytes=0,38'
+			),
+
+			// malformed punctuation
+			array(
+				'bytes: 0 - 32'
+			),
+			array(
+				'garbage: poo - poo'
+			),
+		);
+	}
+
 /**
  * Test invalid file ranges.
  *
+ * @dataProvider invalidFileRangeProvider
  * @return void
  */
-	public function testFileRangeInvalid() {
-		$_SERVER['HTTP_RANGE'] = 'bytes=30-2';
-		$response = $this->getMock('CakeResponse', array(
-			'header',
-			'type',
+	public function testFileRangeInvalid($range) {
+		$_SERVER['HTTP_RANGE'] = $range;
+		$response = $this->getMock('CakeResponse', [
 			'_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('Content-Transfer-Encoding', 'binary');
+		$response->file(
+			CAKE . 'Test' . DS . 'test_app' . DS . 'Vendor' . DS . 'css' . DS . 'test_asset.css',
+			array('download' => true)
+		);
 
-		$response->expects($this->at(3))
-			->method('header')
-			->with('Accept-Ranges', 'bytes');
+		$expected = array(
+			'Content-Disposition' => 'attachment; filename="test_asset.css"',
+			'Content-Transfer-Encoding' => 'binary',
+			'Accept-Ranges' => 'bytes',
+			'Content-Range' => 'bytes 0-37/38',
+			'Content-Length' => 38,
+		);
+		$this->assertEquals($expected, $response->header());
+	}
 
-		$response->expects($this->at(4))
-			->method('header')
-			->with(array(
-				'Content-Range' => 'bytes 0-37/38',
-			));
+/**
+ * Test backwards file range
+ *
+ * @return void
+ */
+	public function testFileRangeReversed() {
+		$_SERVER['HTTP_RANGE'] = 'bytes=30-5';
+		$response = $this->getMock('CakeResponse', [
+			'_sendHeader',
+			'_isActive',
+		]);
 
 		$response->file(
 			CAKE . 'Test' . DS . 'test_app' . DS . 'Vendor' . DS . 'css' . DS . 'test_asset.css',
 			array('download' => true)
 		);
 
+		$expected = array(
+			'Content-Disposition' => 'attachment; filename="test_asset.css"',
+			'Content-Transfer-Encoding' => 'binary',
+			'Accept-Ranges' => 'bytes',
+			'Content-Range' => 'bytes 0-37/38',
+		);
+		$this->assertEquals($expected, $response->header());
 		$this->assertEquals(416, $response->statusCode());
-		$response->send();
 	}
 
 /**