Rough implementation of withFile() & withDownload())

src/Network/Response.php

@@ -1770,6 +1770,7 @@ class Response implements ResponseInterface
      *
      * @param string $filename The name of the file as the browser will download the response
      * @return void
+     * @deprecated 3.4.0 Use withDownload() instead.
      */
     public function download($filename)
     {
@@ -1777,6 +1778,17 @@ class Response implements ResponseInterface
     }
 
     /**
+     * Create a new instance with the Content-Disposition header set.
+     *
+     * @param string $filename The name of the file as the browser will download the response
+     * @return static
+     */
+    public function withDownload($filename)
+    {
+        return $this->withHeader('Content-Disposition', 'attachment; filename="' . $filename . '"');
+    }
+
+    /**
      * Sets the protocol to be used when sending the response. Defaults to HTTP/1.1
      * If called with no arguments, it will return the current configured protocol
      *
@@ -2148,6 +2160,72 @@ class Response implements ResponseInterface
         $this->_file = $file;
     }
 
+    public function withFile($path, array $options = [])
+    {
+        // TODO move validation into a helper method.
+        if (strpos($path, '../') !== false || strpos($path, '..\\') !== false) {
+            throw new NotFoundException('The requested file contains `..` and will not be read.');
+        }
+        if (!is_file($path)) {
+            $path = APP . $path;
+        }
+
+        $file = new File($path);
+        if (!$file->exists() || !$file->readable()) {
+            if (Configure::read('debug')) {
+                throw new NotFoundException(sprintf('The requested file %s was not found or not readable', $path));
+            }
+            throw new NotFoundException(__d('cake', 'The requested file was not found'));
+        }
+        // end refactor.
+
+        $options += [
+            'name' => null,
+            'download' => null
+        ];
+
+        $extension = strtolower($file->ext());
+        if ((!$extension || $this->type($extension) === false) && $options['download'] === null) {
+            $options['download'] = true;
+        }
+
+        $new = clone $this;
+
+        $fileSize = $file->size();
+        if ($options['download']) {
+            $agent = env('HTTP_USER_AGENT');
+
+            if (preg_match('%Opera(/| )([0-9].[0-9]{1,2})%', $agent)) {
+                $contentType = 'application/octet-stream';
+            } elseif (preg_match('/MSIE ([0-9].[0-9]{1,2})/', $agent)) {
+                $contentType = 'application/force-download';
+            }
+
+            if (!empty($contentType)) {
+                $new = $new->withType($contentType);
+            }
+            if ($options['name'] === null) {
+                $name = $file->name;
+            } else {
+                $name = $options['name'];
+            }
+            $new = $new->withDownload($name)
+                ->withHeader('Content-Transfer-Encoding', 'binary');
+        }
+
+        $new = $new->withHeader('Accept-Ranges', 'bytes');
+        $httpRange = env('HTTP_RANGE');
+        if (isset($httpRange)) {
+            $new->_fileRange($file, $httpRange);
+        } else {
+            $new = $new->withHeader('Content-Length', (string)$fileSize);
+        }
+        $new->_file = $file;
+        $new->stream = new Stream($file->path, 'rb');
+
+        return $new;
+    }
+
     /**
      * Get the current file if one exists.
      *
@@ -2167,6 +2245,8 @@ class Response implements ResponseInterface
      * @param \Cake\Filesystem\File $file The file to set a range on.
      * @param string $httpRange The range to use.
      * @return void
+     * @deprecated 3.4.0 Long term this needs to be refactored to follow immutable paradigms.
+     *   However for now, it is simpler to leave this alone.
      */
     protected function _fileRange($file, $httpRange)
     {

tests/TestCase/Network/ResponseTest.php

@@ -14,6 +14,7 @@
  */
 namespace Cake\Test\TestCase\Network;
 
+use Cake\Network\Exception\NotFoundException;
 use Cake\Network\Request;
 use Cake\Network\Response;
 use Cake\TestSuite\TestCase;
@@ -1535,55 +1536,62 @@ class ResponseTest extends TestCase
     }
 
     /**
-     * test file with ../
+     * test withFile() not found
      *
      * @expectedException \Cake\Network\Exception\NotFoundException
-     * @expectedExceptionMessage The requested file contains `..` and will not be read.
      * @return void
      */
-    public function testFileWithForwardSlashPathTraversal()
+    public function testWithFileNotFound()
     {
         $response = new Response();
-        $response->file('my/../cat.gif');
+        $response->withFile('/some/missing/folder/file.jpg');
     }
 
     /**
-     * test file with ..\
+     * Provider for various kinds of unacceptable files.
      *
-     * @expectedException \Cake\Network\Exception\NotFoundException
-     * @expectedExceptionMessage The requested file contains `..` and will not be read.
-     * @return void
+     * @return array
      */
-    public function testFileWithBackwardSlashPathTraversal()
+    public function invalidFileProvider()
     {
-        $response = new Response();
-        $response->file('my\..\cat.gif');
+        return [
+            ['my/../cat.gif', 'The requested file contains `..` and will not be read.'],
+            ['my\..\cat.gif', 'The requested file contains `..` and will not be read.'],
+            ['my/ca..t.gif', 'my/ca..t.gif was not found or not readable'],
+            ['my/ca..t/image.gif', 'my/ca..t/image.gif was not found or not readable'],
+        ];
     }
 
     /**
-     * test file with ..
+     * test invalid file paths.
      *
-     * @expectedException \Cake\Network\Exception\NotFoundException
-     * @expectedExceptionMessage my/ca..t.gif was not found or not readable
+     * @dataProvider invalidFileProvider
      * @return void
      */
-    public function testFileWithDotsInTheFilename()
+    public function testFileInvalidPath($path, $expectedMessage)
     {
         $response = new Response();
-        $response->file('my/ca..t.gif');
+        try {
+            $response->file($path);
+        } catch (NotFoundException $e) {
+            $this->assertContains($expectedMessage, $e->getMessage());
+        }
     }
 
     /**
-     * test file with .. in a path fragment
+     * test withFile and invalid paths
      *
-     * @expectedException \Cake\Network\Exception\NotFoundException
-     * @expectedExceptionMessage my/ca..t/image.gif was not found or not readable
+     * @dataProvider invalidFileProvider
      * @return void
      */
-    public function testFileWithDotsInAPathFragment()
+    public function testWithFileInvalidPath($path, $expectedMessage)
     {
         $response = new Response();
-        $response->file('my/ca..t/image.gif');
+        try {
+            $response->withFile($path);
+        } catch (NotFoundException $e) {
+            $this->assertContains($expectedMessage, $e->getMessage());
+        }
     }
 
     /**
@@ -1626,6 +1634,38 @@ class ResponseTest extends TestCase
     }
 
     /**
+     * test withFile() + download & name
+     *
+     * @return void
+     */
+    public function testWithFileDownloadAndName()
+    {
+        $response = new Response();
+        $new = $response->withFile(
+            TEST_APP . 'vendor' . DS . 'css' . DS . 'test_asset.css',
+            [
+                'name' => 'something_special.css',
+                'download' => true,
+            ]
+        );
+        $this->assertEquals(
+            'text/css; charset=UTF-8',
+            $new->getHeaderLine('Content-Type')
+        );
+        $this->assertEquals(
+            'attachment; filename="something_special.css"',
+            $new->getHeaderLine('Content-Disposition')
+        );
+        $this->assertEquals('bytes', $new->getHeaderLine('Accept-Ranges'));
+        $body = $new->getBody();
+        $this->assertInstanceOf('Zend\Diactoros\Stream', $body);
+
+        $expected = "/* this is the test asset css file */";
+        $this->assertEquals($expected, trim($body->getContents()));
+        $this->assertEquals($expected, trim($new->getFile()->read()));
+    }
+
+    /**
      * testFileWithDownloadAndName
      *
      * @return void
@@ -1734,6 +1774,26 @@ class ResponseTest extends TestCase
     }
 
     /**
+     * test withFile() + a generic agent
+     *
+     * @return void
+     */
+    public function testWithFileUnknownFileTypeGeneric()
+    {
+        $response = new Response();
+        $new = $response->withFile(CONFIG . 'no_section.ini');
+        $this->assertEquals('text/html; charset=UTF-8', $new->getHeaderLine('Content-Type'));
+        $this->assertEquals(
+            'attachment; filename="no_section.ini"',
+            $new->getHeaderLine('Content-Disposition')
+        );
+        $this->assertEquals('bytes', $new->getHeaderLine('Accept-Ranges'));
+        $body = $new->getBody();
+        $expected = "some_key = some_value\nbool_key = 1\n";
+        $this->assertEquals($expected, $body->getContents());
+    }
+
+    /**
      * testFileWithUnknownFileTypeOpera method
      *
      * @return void
@@ -1793,6 +1853,27 @@ class ResponseTest extends TestCase
     }
 
     /**
+     * test withFile() + opera
+     *
+     * @return void
+     */
+    public function testWithFileUnknownFileTypeOpera()
+    {
+        $currentUserAgent = isset($_SERVER['HTTP_USER_AGENT']) ? $_SERVER['HTTP_USER_AGENT'] : null;
+        $_SERVER['HTTP_USER_AGENT'] = 'Opera/9.80 (Windows NT 6.0; U; en) Presto/2.8.99 Version/11.10';
+        $response = new Response();
+
+        $new = $response->withFile(CONFIG . 'no_section.ini');
+        $this->assertEquals('application/octet-stream', $new->getHeaderLine('Content-Type'));
+        $this->assertEquals(
+            'attachment; filename="no_section.ini"',
+            $new->getHeaderLine('Content-Disposition')
+        );
+
+        $_SERVER['HTTP_USER_AGENT'] = $currentUserAgent;
+    }
+
+    /**
      * testFileWithUnknownFileTypeIE method
      *
      * @return void
@@ -1852,6 +1933,24 @@ class ResponseTest extends TestCase
             $_SERVER['HTTP_USER_AGENT'] = $currentUserAgent;
         }
     }
+
+    /**
+     * test withFile() + old IE
+     *
+     * @return void
+     */
+    public function testWithFileUnknownFileTypeOldIe()
+    {
+        $currentUserAgent = isset($_SERVER['HTTP_USER_AGENT']) ? $_SERVER['HTTP_USER_AGENT'] : null;
+        $_SERVER['HTTP_USER_AGENT'] = 'Mozilla/5.0 (compatible; MSIE 8.0; Windows NT 5.2; Trident/4.0; Media Center PC 4.0; SLCC1; .NET CLR 3.0.04320)';
+        $response = new Response();
+
+        $new = $response->withFile(CONFIG . 'no_section.ini');
+        $this->assertEquals('application/force-download', $new->getHeaderLine('Content-Type'));
+
+        $_SERVER['HTTP_USER_AGENT'] = $currentUserAgent;
+    }
+
     /**
      * testFileWithUnknownFileNoDownload method
      *
@@ -1895,6 +1994,24 @@ class ResponseTest extends TestCase
     }
 
     /**
+     * test withFile() + no download
+     *
+     * @return void
+     */
+    public function testWithFileNoDownload()
+    {
+        $response = new Response();
+        $new = $response->withFile(CONFIG . 'no_section.ini', [
+            'download' => false
+        ]);
+        $this->assertEquals(
+            'text/html; charset=UTF-8',
+            $new->getHeaderLine('Content-Type')
+        );
+        $this->assertFalse($new->hasHeader('Content-Disposition'));
+    }
+
+    /**
      * test getFile method
      *
      * @return void
@@ -1974,6 +2091,18 @@ class ResponseTest extends TestCase
     }
 
     /**
+     * Test that uppercase extensions result in correct content-types
+     *
+     * @return void
+     */
+    public function testWithFileUpperExtension()
+    {
+        $response = new Response();
+        $new = $response->withFile(TEST_APP . 'vendor/img/test_2.JPG');
+        $this->assertEquals('image/jpeg', $new->getHeaderLine('Content-Type'));
+    }
+
+    /**
      * Test downloading files with extension not explicitly set.
      *
      * @return void
@@ -2088,6 +2217,30 @@ class ResponseTest extends TestCase
     }
 
     /**
+     * Test withFile() & the various range offset types.
+     *
+     * @dataProvider rangeProvider
+     * @return void
+     */
+    public function testWithFileRangeOffsets($range, $length, $offsetResponse)
+    {
+        $_SERVER['HTTP_RANGE'] = $range;
+        $response = new Response();
+        $new = $response->withFile(
+            TEST_APP . 'vendor' . DS . 'css' . DS . 'test_asset.css',
+            ['download' => true]
+        );
+        $this->assertEquals(
+            'attachment; filename="test_asset.css"',
+            $new->getHeaderLine('Content-Disposition')
+        );
+        $this->assertEquals('binary', $new->getHeaderLine('Content-Transfer-Encoding'));
+        $this->assertEquals('bytes', $new->getHeaderLine('Accept-Ranges'));
+        $this->assertEquals($length, $new->getHeaderLine('Content-Length'));
+        $this->assertEquals($offsetResponse, $new->getHeaderLine('Content-Range'));
+    }
+
+    /**
      * Test fetching ranges from a file.
      *
      * @return void
@@ -2147,6 +2300,31 @@ class ResponseTest extends TestCase
     }
 
     /**
+     * Test withFile() fetching ranges from a file.
+     *
+     * @return void
+     */
+    public function testWithFileRange()
+    {
+        $_SERVER['HTTP_RANGE'] = 'bytes=8-25';
+        $response = new Response();
+        $new = $response->withFile(
+            TEST_APP . 'vendor' . DS . 'css' . DS . 'test_asset.css',
+            ['download' => true]
+        );
+
+        $this->assertEquals(
+            'attachment; filename="test_asset.css"',
+            $new->getHeaderLine('Content-Disposition')
+        );
+        $this->assertEquals('binary', $new->getHeaderLine('Content-Transfer-Encoding'));
+        $this->assertEquals('bytes', $new->getHeaderLine('Accept-Ranges'));
+        $this->assertEquals('18', $new->getHeaderLine('Content-Length'));
+        $this->assertEquals('bytes 8-25/38', $new->getHeaderLine('Content-Range'));
+        $this->assertEquals(206, $new->getStatusCode());
+    }
+
+    /**
      * Provider for invalid range header values.
      *
      * @return array
@@ -2199,6 +2377,32 @@ class ResponseTest extends TestCase
     }
 
     /**
+     * Test withFile() and invalid ranges
+     *
+     * @dataProvider invalidFileRangeProvider
+     * @return void
+     */
+    public function testWithFileInvalidRange($range)
+    {
+        $_SERVER['HTTP_RANGE'] = $range;
+        $response = new Response();
+        $new = $response->withFile(
+            TEST_APP . 'vendor' . DS . 'css' . DS . 'test_asset.css',
+            ['download' => true]
+        );
+
+        $this->assertEquals(
+            'attachment; filename="test_asset.css"',
+            $new->getHeaderLine('Content-Disposition')
+        );
+        $this->assertEquals('binary', $new->getHeaderLine('Content-Transfer-Encoding'));
+        $this->assertEquals('bytes', $new->getHeaderLine('Accept-Ranges'));
+        $this->assertEquals('38', $new->getHeaderLine('Content-Length'));
+        $this->assertEquals('bytes 0-37/38', $new->getHeaderLine('Content-Range'));
+        $this->assertEquals(206, $new->getStatusCode());
+    }
+
+    /**
      * Test reversed file ranges.
      *
      * @return void
@@ -2243,6 +2447,30 @@ class ResponseTest extends TestCase
     }
 
     /**
+     * Test withFile() and a reversed range
+     *
+     * @return void
+     */
+    public function testWithFileReversedRange()
+    {
+        $_SERVER['HTTP_RANGE'] = 'bytes=30-2';
+        $response = new Response();
+        $new = $response->withFile(
+            TEST_APP . 'vendor' . DS . 'css' . DS . 'test_asset.css',
+            ['download' => true]
+        );
+
+        $this->assertEquals(
+            'attachment; filename="test_asset.css"',
+            $new->getHeaderLine('Content-Disposition')
+        );
+        $this->assertEquals('binary', $new->getHeaderLine('Content-Transfer-Encoding'));
+        $this->assertEquals('bytes', $new->getHeaderLine('Accept-Ranges'));
+        $this->assertEquals('bytes 0-37/38', $new->getHeaderLine('Content-Range'));
+        $this->assertEquals(416, $new->getStatusCode());
+    }
+
+    /**
      * testFileRangeOffsetsNoDownload method
      *
      * @dataProvider rangeProvider