Browse Source

Merge pull request #13555 from ndm2/feature/file-upload-objects-in-data

Add initial support for file upload objects in the request data.
ADmad 6 years ago
parent
commit
3fa0c979b8

+ 18 - 1
src/Http/ServerRequest.php

@@ -244,6 +244,13 @@ class ServerRequest implements ArrayAccess, ServerRequestInterface
     ];
 
     /**
+     * Whether to merge file uploads as objects (`true`) or arrays (`false`).
+     *
+     * @var bool
+     */
+    private $mergeFilesAsObjects = false;
+
+    /**
      * Wrapper method to create a new request from PHP superglobals.
      *
      * Uses the $_GET, $_POST, $_FILES, $_COOKIE, $_SERVER, $_ENV and php://input data to construct
@@ -281,6 +288,7 @@ class ServerRequest implements ArrayAccess, ServerRequestInterface
      * - `input` The data that would come from php://input this is useful for simulating
      *   requests with put, patch or delete data.
      * - `session` An instance of a Session object
+     * - `mergeFilesAsObjects` Whether to merge file uploads as objects (`true`) or arrays (`false`).
      *
      * @param string|array $config An array of request data to create a request with.
      *   The string version of this argument is *deprecated* and will be removed in 4.0.0
@@ -302,6 +310,7 @@ class ServerRequest implements ArrayAccess, ServerRequestInterface
             'base' => '',
             'webroot' => '',
             'input' => null,
+            'mergeFilesAsObjects' => false,
         ];
 
         $this->_setConfig($config);
@@ -364,6 +373,8 @@ class ServerRequest implements ArrayAccess, ServerRequestInterface
         }
         $this->stream = $stream;
 
+        $this->mergeFilesAsObjects = $config['mergeFilesAsObjects'];
+
         $config['post'] = $this->_processPost($config['post']);
         $this->data = $this->_processFiles($config['post'], $config['files']);
         $this->query = $this->_processGet($config['query'], $querystr);
@@ -436,7 +447,9 @@ class ServerRequest implements ArrayAccess, ServerRequestInterface
      */
     protected function _processFiles($post, $files)
     {
-        if (!is_array($files)) {
+        if (empty($files) ||
+            !is_array($files)
+        ) {
             return $post;
         }
         $fileData = [];
@@ -458,6 +471,10 @@ class ServerRequest implements ArrayAccess, ServerRequestInterface
         }
         $this->uploadedFiles = $fileData;
 
+        if ($this->mergeFilesAsObjects) {
+            return Hash::merge($post, $fileData);
+        }
+
         // Make a flat map that can be inserted into $post for BC.
         $fileMap = Hash::flatten($fileData);
         foreach ($fileMap as $key => $file) {

+ 1 - 0
src/Http/ServerRequestFactory.php

@@ -54,6 +54,7 @@ abstract class ServerRequestFactory extends BaseFactory
             'webroot' => $uri->webroot,
             'base' => $uri->base,
             'session' => $session,
+            'mergeFilesAsObjects' => Configure::read('App.uploadedFilesAsObjects', false),
         ]);
 
         return $request;

+ 97 - 0
tests/TestCase/Http/ServerRequestFactoryTest.php

@@ -18,6 +18,7 @@ use Cake\Core\Configure;
 use Cake\Http\ServerRequestFactory;
 use Cake\Http\Session;
 use Cake\TestSuite\TestCase;
+use Zend\Diactoros\UploadedFile;
 
 /**
  * Test case for the server factory.
@@ -280,4 +281,100 @@ class ServerRequestFactoryTest extends TestCase
         $this->assertEquals('/index.php', $res->getAttribute('base'));
         $this->assertEquals('/posts/add', $res->getUri()->getPath());
     }
+
+    /**
+     * Tests the default file upload merging behavior.
+     *
+     * @return void
+     */
+    public function testFromGlobalsWithFilesAsObjectsDefault()
+    {
+        $this->assertNull(Configure::read('App.uploadedFilesAsObjects'));
+
+        $files = [
+            'file' => [
+                'name' => 'file.txt',
+                'type' => 'text/plain',
+                'tmp_name' => __FILE__,
+                'error' => 0,
+                'size' => 1234,
+            ],
+        ];
+        $request = ServerRequestFactory::fromGlobals(null, null, null, null, $files);
+
+        $expected = [
+            'file' => [
+                'tmp_name' => __FILE__,
+                'error' => 0,
+                'name' => 'file.txt',
+                'type' => 'text/plain',
+                'size' => 1234
+            ],
+        ];
+        $this->assertEquals($expected, $request->getData());
+    }
+
+    /**
+     * Tests the "as arrays" file upload merging behavior.
+     *
+     * @return void
+     */
+    public function testFromGlobalsWithFilesAsObjectsDisabled()
+    {
+        Configure::write('App.uploadedFilesAsObjects', false);
+
+        $files = [
+            'file' => [
+                'name' => 'file.txt',
+                'type' => 'text/plain',
+                'tmp_name' => __FILE__,
+                'error' => 0,
+                'size' => 1234,
+            ],
+        ];
+        $request = ServerRequestFactory::fromGlobals(null, null, null, null, $files);
+
+        $expected = [
+            'file' => [
+                'tmp_name' => __FILE__,
+                'error' => 0,
+                'name' => 'file.txt',
+                'type' => 'text/plain',
+                'size' => 1234
+            ],
+        ];
+        $this->assertEquals($expected, $request->getData());
+    }
+
+    /**
+     * Tests the "as objects" file upload merging behavior.
+     *
+     * @return void
+     */
+    public function testFromGlobalsWithFilesAsObjectsEnabled()
+    {
+        Configure::write('App.uploadedFilesAsObjects', true);
+
+        $files = [
+            'file' => [
+                'name' => 'file.txt',
+                'type' => 'text/plain',
+                'tmp_name' => __FILE__,
+                'error' => 0,
+                'size' => 1234,
+            ],
+        ];
+        $request = ServerRequestFactory::fromGlobals(null, null, null, null, $files);
+
+        $expected = [
+            'file' => new UploadedFile(
+                __FILE__,
+                1234,
+                0,
+                'file.txt',
+                'text/plain'
+            ),
+        ];
+        $this->assertEquals($expected, $request->getData());
+    }
 }

+ 199 - 0
tests/TestCase/Http/ServerRequestTest.php

@@ -590,6 +590,205 @@ class ServerRequestTest extends TestCase
     }
 
     /**
+     * Test passing an invalid data type for the files list.
+     *
+     * @return void
+     */
+    public function testFilesWithInvalidDataType()
+    {
+        $request = new ServerRequest([
+            'files' => 'invalid',
+        ]);
+
+        $this->assertEmpty($request->getData());
+        $this->assertEmpty($request->getUploadedFiles());
+    }
+
+    /**
+     * Test passing an empty files list.
+     *
+     * @return void
+     */
+    public function testFilesWithEmptyList()
+    {
+        $request = new ServerRequest([
+            'files' => [],
+        ]);
+
+        $this->assertEmpty($request->getData());
+        $this->assertEmpty($request->getUploadedFiles());
+    }
+
+    /**
+     * Test passing invalid files list structure.
+     *
+     * @return void
+     */
+    public function testFilesWithInvalidStructure()
+    {
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage('Invalid value in FILES "{"invalid":["data"]}"');
+
+        new ServerRequest([
+            'files' => [
+                [
+                    'invalid' => [
+                        'data'
+                    ]
+                ]
+            ],
+        ]);
+    }
+
+    /**
+     * Tests that file uploads are merged into the post data as objects instead of as arrays.
+     *
+     * @return void
+     */
+    public function testFilesAsObjectsInRequestData()
+    {
+        $files = [
+            'flat' => [
+                'name' => 'flat.txt',
+                'type' => 'text/plain',
+                'tmp_name' => __FILE__,
+                'error' => 0,
+                'size' => 1,
+            ],
+            'nested' => [
+                'name' => ['file' => 'nested.txt'],
+                'type' => ['file' => 'text/plain'],
+                'tmp_name' => ['file' => __FILE__],
+                'error' => ['file' => 0],
+                'size' => ['file' => 12],
+            ],
+            0 => [
+                'name' => 'numeric.txt',
+                'type' => 'text/plain',
+                'tmp_name' => __FILE__,
+                'error' => 0,
+                'size' => 123,
+            ],
+            1 => [
+                'name' => ['file' => 'numeric-nested.txt'],
+                'type' => ['file' => 'text/plain'],
+                'tmp_name' => ['file' => __FILE__],
+                'error' => ['file' => 0],
+                'size' => ['file' => 1234],
+            ],
+            'deep' => [
+                'name' => [
+                    0 => ['file' => 'deep-1.txt'],
+                    1 => ['file' => 'deep-2.txt'],
+                ],
+                'type' => [
+                    0 => ['file' => 'text/plain'],
+                    1 => ['file' => 'text/plain'],
+                ],
+                'tmp_name' => [
+                    0 => ['file' => __FILE__],
+                    1 => ['file' => __FILE__],
+                ],
+                'error' => [
+                    0 => ['file' => 0],
+                    1 => ['file' => 0],
+                ],
+                'size' => [
+                    0 => ['file' => 12345],
+                    1 => ['file' => 123456],
+                ],
+            ],
+        ];
+
+        $post = [
+            'flat' => ['existing'],
+            'nested' => [
+                'name' => 'nested',
+                'file' => ['existing']
+            ],
+            'deep' => [
+                0 => [
+                    'name' => 'deep 1',
+                    'file' => ['existing']
+                ],
+                1 => [
+                    'name' => 'deep 2',
+                ],
+            ],
+            1 => [
+                'name' => 'numeric nested',
+            ],
+        ];
+
+        $expected = [
+            'flat' => new UploadedFile(
+                __FILE__,
+                1,
+                0,
+                'flat.txt',
+                'text/plain'
+            ),
+            'nested' => [
+                'name' => 'nested',
+                'file' => new UploadedFile(
+                    __FILE__,
+                    12,
+                    0,
+                    'nested.txt',
+                    'text/plain'
+                )
+            ],
+            'deep' => [
+                0 => [
+                    'name' => 'deep 1',
+                    'file' => new UploadedFile(
+                        __FILE__,
+                        12345,
+                        0,
+                        'deep-1.txt',
+                        'text/plain'
+                    )
+                ],
+                1 => [
+                    'name' => 'deep 2',
+                    'file' => new UploadedFile(
+                        __FILE__,
+                        123456,
+                        0,
+                        'deep-2.txt',
+                        'text/plain'
+                    )
+                ],
+            ],
+            0 => new UploadedFile(
+                __FILE__,
+                123,
+                0,
+                'numeric.txt',
+                'text/plain'
+            ),
+            1 => [
+                'name' => 'numeric nested',
+                'file' => new UploadedFile(
+                    __FILE__,
+                    1234,
+                    0,
+                    'numeric-nested.txt',
+                    'text/plain'
+                )
+            ],
+        ];
+
+        $request = new ServerRequest([
+            'files' => $files,
+            'post' => $post,
+            'mergeFilesAsObjects' => true
+        ]);
+
+        $this->assertEquals($expected, $request->getData());
+    }
+
+    /**
      * Test replacing files.
      *
      * @return void