Browse Source

Add deprecation warning for non-absolute file paths.

Files being used with Response::withFile() should be absolute. Prefixing
with the application path is often seen as unwanted magic.

Refs #11921
Mark Story 8 years ago
parent
commit
55c78435d0
2 changed files with 21 additions and 10 deletions
  1. 3 2
      src/Http/Response.php
  2. 18 8
      tests/TestCase/Http/ResponseTest.php

+ 3 - 2
src/Http/Response.php

@@ -2430,7 +2430,7 @@ class Response implements ResponseInterface
      * - download: If `true` sets download header and forces file to be downloaded rather than displayed in browser
      *
      * @param string $path Path to file. If the path is not an absolute path that resolves
-     *   to a file, `APP` will be prepended to the path.
+     *   to a file, `APP` will be prepended to the path (this behavior is deprecated).
      * @param array $options Options See above.
      * @return void
      * @throws \Cake\Http\Exception\NotFoundException
@@ -2504,7 +2504,7 @@ class Response implements ResponseInterface
      *   be downloaded rather than displayed inline.
      *
      * @param string $path Path to file. If the path is not an absolute path that resolves
-     *   to a file, `APP` will be prepended to the path.
+     *   to a file, `APP` will be prepended to the path (this behavior is deprecated).
      * @param array $options Options See above.
      * @return static
      * @throws \Cake\Http\Exception\NotFoundException
@@ -2587,6 +2587,7 @@ class Response implements ResponseInterface
             throw new NotFoundException(__d('cake', 'The requested file contains `..` and will not be read.'));
         }
         if (!is_file($path)) {
+            deprecationWarning('Using non-absolute paths with Response::file() and withFile() is deprecated.');
             $path = APP . $path;
         }
 

+ 18 - 8
tests/TestCase/Http/ResponseTest.php

@@ -1919,13 +1919,18 @@ class ResponseTest extends TestCase
     /**
      * test withFile() not found
      *
+     * Don't remove this test when cleaning up deprecation warnings.
+     * Just remove the deprecated wrapper.
+     *
      * @return void
      */
     public function testWithFileNotFound()
     {
         $this->expectException(\Cake\Http\Exception\NotFoundException::class);
-        $response = new Response();
-        $response->withFile('/some/missing/folder/file.jpg');
+        $this->deprecated(function () {
+            $response = new Response();
+            $response->withFile('/some/missing/folder/file.jpg');
+        });
     }
 
     /**
@@ -1965,17 +1970,22 @@ class ResponseTest extends TestCase
     /**
      * test withFile and invalid paths
      *
+     * This test should not be removed when deprecation warnings are removed.
+     * Just remove the deprecated wrapper.
+     *
      * @dataProvider invalidFileProvider
      * @return void
      */
     public function testWithFileInvalidPath($path, $expectedMessage)
     {
-        $response = new Response();
-        try {
-            $response->withFile($path);
-        } catch (NotFoundException $e) {
-            $this->assertContains($expectedMessage, $e->getMessage());
-        }
+        $this->deprecated(function () use ($path, $expectedMessage) {
+            $response = new Response();
+            try {
+                $response->withFile($path);
+            } catch (NotFoundException $e) {
+                $this->assertContains($expectedMessage, $e->getMessage());
+            }
+        });
     }
 
     /**