Fix $_FILES parsing

The old tests were just straight up wrong. I built them wrong when
converting the code from 2.x to now. In 2.x all files were preceded by
the `data` name which made files parsing much simpler as we had already
stripped off the first name element.

The new method feels better to me as well. It doesn't use reference
arguments and I think is a tiny bit simpler.

Refs #3999

src/Network/Request.php

@@ -402,10 +402,10 @@ class Request implements \ArrayAccess {
 	protected function _processFiles($post, $files) {
 		if (is_array($files)) {
 			foreach ($files as $key => $data) {
-				if (!is_numeric($key)) {
-					$this->_processFileData($post, '', $data, $key);
-				} else {
+				if (isset($data['tmp_name']) && is_string($data['tmp_name'])) {
 					$post[$key] = $data;
+				} else {
+					$post[$key] = $this->_processFileData([], $data);
 				}
 			}
 		}
@@ -414,31 +414,32 @@ class Request implements \ArrayAccess {
 
 /**
  * Recursively walks the FILES array restructuring the data
- * into something sane and useable.
+ * into something sane and usable.
  *
- * @param array &$post The post data having files inserted into
+ * @param array $data The data being built
+ * @param array $post The post data being traversed
  * @param string $path The dot separated path to insert $data into.
- * @param array $data The data to traverse/insert.
- * @param string $field The terminal field name, which is the top level key in $_FILES.
+ * @param string $field The terminal field in the path. This is one of the
+ *   $_FILES properties e.g. name, tmp_name, size, error
  * @return void
  */
-	protected function _processFileData(&$post, $path, $data, $field) {
-		foreach ($data as $key => $fields) {
-			$newPath = $key;
-			if (!empty($path)) {
-				$newPath = $path . '.' . $key;
+	protected function _processFileData($data, $post, $path = '', $field = '') {
+		foreach ($post as $key => $fields) {
+			$newField = $field;
+			if ($path === '' && $newField === '') {
+				$newField = $key;
+			}
+			if ($field === $newField) {
+				$path .= '.' . $key;
 			}
 			if (is_array($fields)) {
-				$this->_processFileData($post, $newPath, $fields, $field);
+				$data = $this->_processFileData($data, $fields, $path, $newField);
 			} else {
-				if (strpos($newPath, '.') === false) {
-					$newPath = $field . '.' . $key;
-				} else {
-					$newPath .= '.' . $field;
-				}
-				$post = Hash::insert($post, $newPath, $fields);
+				$path = trim($path . '.' . $field, '.');
+				$data = Hash::insert($data, $path, $fields);
 			}
 		}
+		return $data;
 	}
 
 /**

tests/TestCase/Network/RequestTest.php

@@ -259,235 +259,76 @@ class RequestTest extends TestCase {
 	}
 
 /**
- * Test parsing of FILES array
+ * Test processing files with `file` field names.
  *
  * @return void
  */
-	public function testFilesParsing() {
-		$files = array(
-			'name' => array(
-				'File' => array(
-						array('data' => 'cake_sqlserver_patch.patch'),
-						array('data' => 'controller.diff'),
-						array('data' => ''),
-						array('data' => ''),
-					),
-					'Post' => array('attachment' => 'jquery-1.2.1.js'),
-				),
-			'type' => array(
-				'File' => array(
-					array('data' => ''),
-					array('data' => ''),
-					array('data' => ''),
-					array('data' => ''),
-				),
-				'Post' => array('attachment' => 'application/x-javascript'),
-			),
-			'tmp_name' => array(
-				'File' => array(
-					array('data' => '/private/var/tmp/phpy05Ywj'),
-					array('data' => '/private/var/tmp/php7MBztY'),
-					array('data' => ''),
-					array('data' => ''),
-				),
-				'Post' => array('attachment' => '/private/var/tmp/phpEwlrIo'),
-			),
-			'error' => array(
-				'File' => array(
-					array('data' => 0),
-					array('data' => 0),
-					array('data' => 4),
-					array('data' => 4)
-				),
-				'Post' => array('attachment' => 0)
-			),
-			'size' => array(
-				'File' => array(
-					array('data' => 6271),
-					array('data' => 350),
-					array('data' => 0),
-					array('data' => 0),
-				),
-				'Post' => array('attachment' => 80469)
-			),
-		);
-
+	public function testProcessFilesNested() {
+		$files = [
+			'image_main' => [
+				'name' => ['file' => 'born on.txt'],
+				'type' => ['file' => 'text/plain'],
+				'tmp_name' => ['file' => '/private/var/tmp/php'],
+				'error' => ['file' => 0],
+				'size' => ['file' => 17178]
+			],
+			0 => [
+				'name' => ['image' => 'scratch.text'],
+				'type' => ['image' => 'text/plain'],
+				'tmp_name' => ['image' => '/private/var/tmp/phpChIZPb'],
+				'error' => ['image' => 0],
+				'size' => ['image' => 1490]
+			],
+			'pictures' => [
+				'name' => [
+					0 => ['file' => 'a-file.png']
+				],
+				'type' => [
+					0 => ['file' => 'image/png']
+				],
+				'tmp_name' => [
+					0 => ['file' => '/tmp/file123']
+				],
+				'error' => [
+					0 => ['file' => '0']
+				],
+				'size' => [
+					0 => ['file' => 17188]
+				],
+			]
+		];
 		$request = new Request(compact('files'));
-		$expected = array(
-			'File' => array(
-				array(
-					'data' => array(
-						'name' => 'cake_sqlserver_patch.patch',
-						'type' => '',
-						'tmp_name' => '/private/var/tmp/phpy05Ywj',
-						'error' => 0,
-						'size' => 6271,
-					)
-				),
-				array(
-					'data' => array(
-						'name' => 'controller.diff',
-						'type' => '',
-						'tmp_name' => '/private/var/tmp/php7MBztY',
-						'error' => 0,
-						'size' => 350,
-					)
-				),
-				array(
-					'data' => array(
-						'name' => '',
-						'type' => '',
-						'tmp_name' => '',
-						'error' => 4,
-						'size' => 0,
-					)
-				),
-				array(
-					'data' => array(
-						'name' => '',
-						'type' => '',
-						'tmp_name' => '',
-						'error' => 4,
-						'size' => 0,
-					)
-				),
-			),
-			'Post' => array(
-				'attachment' => array(
-					'name' => 'jquery-1.2.1.js',
-					'type' => 'application/x-javascript',
-					'tmp_name' => '/private/var/tmp/phpEwlrIo',
+		$expected = [
+			'image_main' => [
+				'file' => [
+					'name' => 'born on.txt',
+					'type' => 'text/plain',
+					'tmp_name' => '/private/var/tmp/php',
 					'error' => 0,
-					'size' => 80469,
-				)
-			)
-		);
-		$this->assertEquals($expected, $request->data);
-
-		$files = array(
-			'name' => array(
-				'Document' => array(
-					1 => array(
-						'birth_cert' => 'born on.txt',
-						'passport' => 'passport.txt',
-						'drivers_license' => 'ugly pic.jpg'
-					),
-					2 => array(
-						'birth_cert' => 'aunt betty.txt',
-						'passport' => 'betty-passport.txt',
-						'drivers_license' => 'betty-photo.jpg'
-					),
-				),
-			),
-			'type' => array(
-				'Document' => array(
-					1 => array(
-						'birth_cert' => 'application/octet-stream',
-						'passport' => 'application/octet-stream',
-						'drivers_license' => 'application/octet-stream',
-					),
-					2 => array(
-						'birth_cert' => 'application/octet-stream',
-						'passport' => 'application/octet-stream',
-						'drivers_license' => 'application/octet-stream',
-					)
-				)
-			),
-			'tmp_name' => array(
-				'Document' => array(
-					1 => array(
-						'birth_cert' => '/private/var/tmp/phpbsUWfH',
-						'passport' => '/private/var/tmp/php7f5zLt',
-						'drivers_license' => '/private/var/tmp/phpMXpZgT',
-					),
-					2 => array(
-						'birth_cert' => '/private/var/tmp/php5kHZt0',
-						'passport' => '/private/var/tmp/phpnYkOuM',
-						'drivers_license' => '/private/var/tmp/php9Rq0P3',
-					)
-				)
-			),
-			'error' => array(
-				'Document' => array(
-					1 => array(
-						'birth_cert' => 0,
-						'passport' => 0,
-						'drivers_license' => 0,
-					),
-					2 => array(
-						'birth_cert' => 0,
-						'passport' => 0,
-						'drivers_license' => 0,
-					)
-				)
-			),
-			'size' => array(
-				'Document' => array(
-					1 => array(
-						'birth_cert' => 123,
-						'passport' => 458,
-						'drivers_license' => 875,
-					),
-					2 => array(
-						'birth_cert' => 876,
-						'passport' => 976,
-						'drivers_license' => 9783,
-					)
-				)
-			)
-		);
-
-		$request = new Request(compact('files'));
-		$expected = array(
-			'Document' => array(
-				1 => array(
-					'birth_cert' => array(
-						'name' => 'born on.txt',
-						'tmp_name' => '/private/var/tmp/phpbsUWfH',
-						'error' => 0,
-						'size' => 123,
-						'type' => 'application/octet-stream',
-					),
-					'passport' => array(
-						'name' => 'passport.txt',
-						'tmp_name' => '/private/var/tmp/php7f5zLt',
-						'error' => 0,
-						'size' => 458,
-						'type' => 'application/octet-stream',
-					),
-					'drivers_license' => array(
-						'name' => 'ugly pic.jpg',
-						'tmp_name' => '/private/var/tmp/phpMXpZgT',
-						'error' => 0,
-						'size' => 875,
-						'type' => 'application/octet-stream',
-					),
-				),
-				2 => array(
-					'birth_cert' => array(
-						'name' => 'aunt betty.txt',
-						'tmp_name' => '/private/var/tmp/php5kHZt0',
-						'error' => 0,
-						'size' => 876,
-						'type' => 'application/octet-stream',
-					),
-					'passport' => array(
-						'name' => 'betty-passport.txt',
-						'tmp_name' => '/private/var/tmp/phpnYkOuM',
-						'error' => 0,
-						'size' => 976,
-						'type' => 'application/octet-stream',
-					),
-					'drivers_license' => array(
-						'name' => 'betty-photo.jpg',
-						'tmp_name' => '/private/var/tmp/php9Rq0P3',
-						'error' => 0,
-						'size' => 9783,
-						'type' => 'application/octet-stream',
-					),
-				),
-			)
-		);
+					'size' => 17178,
+				]
+			],
+			'pictures' => [
+				0 => [
+					'file' => [
+						'name' => 'a-file.png',
+						'type' => 'image/png',
+						'tmp_name' => '/tmp/file123',
+						'error' => '0',
+						'size' => 17188,
+					]
+				]
+			],
+			0 => [
+				'image' => [
+					'name' => 'scratch.text',
+					'type' => 'text/plain',
+					'tmp_name' => '/private/var/tmp/phpChIZPb',
+					'error' => 0,
+					'size' => 1490
+				]
+			]
+		];
 		$this->assertEquals($expected, $request->data);
 	}