bug #35827 [BrowserKit] Nested file array prevents uploading file (af…

…ilina)

This PR was merged into the 4.4 branch.

Discussion
----------

[BrowserKit] Nested file array prevents uploading file

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        | n/a

The HttpBrowser doesn't play nicely with Symfony\Component\DomCrawler::getPhpFiles().
The former assumes a flat array structure, while the latter explicitly un-flattens it, causing files to silently get ignored by the DomCrawler's submitForm.

Commits
-------

e15f05e [BrowserKit] Nested file array prevents uploading file

src/Symfony/Component/BrowserKit/HttpBrowser.php

@@ -73,18 +73,9 @@ private function getBodyAndExtraHeaders(Request $request): array
         }
 
         $fields = $request->getParameters();
-        $hasFile = false;
-        foreach ($request->getFiles() as $name => $file) {
-            if (!isset($file['tmp_name'])) {
-                continue;
-            }
-
-            $hasFile = true;
-            $fields[$name] = DataPart::fromPath($file['tmp_name'], $file['name']);
-        }
 
-        if ($hasFile) {
-            $part = new FormDataPart($fields);
+        if ($uploadedFiles = $this->getUploadedFiles($request->getFiles())) {
+            $part = new FormDataPart($uploadedFiles);
 
             return [$part->bodyToIterable(), $part->getPreparedHeaders()->toArray()];
         }
@@ -119,4 +110,26 @@ private function getHeaders(Request $request): array
 
         return $headers;
     }
+
+    /**
+     * Recursively go through the list. If the file has a tmp_name, convert it to a DataPart.
+     * Keep the original hierarchy.
+     */
+    private function getUploadedFiles(array $files): array
+    {
+        $uploadedFiles = [];
+        foreach ($files as $name => $file) {
+            if (!\is_array($file)) {
+                return $uploadedFiles;
+            }
+            if (!isset($file['tmp_name'])) {
+                $uploadedFiles[$name] = $this->getUploadedFiles($file);
+            }
+            if (isset($file['tmp_name'])) {
+                $uploadedFiles[$name] = DataPart::fromPath($file['tmp_name'], $file['name']);
+            }
+        }
+
+        return $uploadedFiles;
+    }
 }

src/Symfony/Component/BrowserKit/Tests/HttpBrowserTest.php

@@ -27,17 +27,17 @@ public function getBrowser(array $server = [], History $history = null, CookieJa
     /**
      * @dataProvider validContentTypes
      */
-    public function testRequestHeaders(array $request, array $exepectedCall)
+    public function testRequestHeaders(array $requestArguments, array $expectedArguments)
     {
         $client = $this->createMock(HttpClientInterface::class);
         $client
             ->expects($this->once())
             ->method('request')
-            ->with(...$exepectedCall)
+            ->with(...$expectedArguments)
             ->willReturn($this->createMock(ResponseInterface::class));
 
         $browser = new HttpBrowser($client);
-        $browser->request(...$request);
+        $browser->request(...$requestArguments);
     }
 
     public function validContentTypes()
@@ -61,7 +61,7 @@ public function validContentTypes()
         ];
     }
 
-    public function testMultiPartRequest()
+    public function testMultiPartRequestWithSingleFile()
     {
         $client = $this->createMock(HttpClientInterface::class);
         $client
@@ -81,4 +81,90 @@ public function testMultiPartRequest()
         file_put_contents($path, 'my_file');
         $browser->request('POST', 'http://example.com/', [], ['file' => ['tmp_name' => $path, 'name' => 'foo']]);
     }
+
+    public function testMultiPartRequestWithNormalFlatArray()
+    {
+        $client = $this->createMock(HttpClientInterface::class);
+        $this->expectClientToSendRequestWithFiles($client, ['file1_content', 'file2_content']);
+
+        $browser = new HttpBrowser($client);
+        $browser->request('POST', 'http://example.com/', [], [
+            'file1' => $this->getUploadedFile('file1'),
+            'file2' => $this->getUploadedFile('file2'),
+        ]);
+    }
+
+    public function testMultiPartRequestWithNormalNestedArray()
+    {
+        $client = $this->createMock(HttpClientInterface::class);
+        $this->expectClientToSendRequestWithFiles($client, ['file1_content', 'file2_content']);
+
+        $browser = new HttpBrowser($client);
+        $browser->request('POST', 'http://example.com/', [], [
+            'level1' => [
+                'level2' => [
+                    'file1' => $this->getUploadedFile('file1'),
+                    'file2' => $this->getUploadedFile('file2'),
+                ],
+            ],
+        ]);
+    }
+
+    public function testMultiPartRequestWithBracketedArray()
+    {
+        $client = $this->createMock(HttpClientInterface::class);
+        $this->expectClientToSendRequestWithFiles($client, ['file1_content', 'file2_content']);
+
+        $browser = new HttpBrowser($client);
+        $browser->request('POST', 'http://example.com/', [], [
+            'form[file1]' => $this->getUploadedFile('file1'),
+            'form[file2]' => $this->getUploadedFile('file2'),
+        ]);
+    }
+
+    public function testMultiPartRequestWithInvalidItem()
+    {
+        $client = $this->createMock(HttpClientInterface::class);
+        $this->expectClientToSendRequestWithFiles($client, ['file1_content']);
+
+        $browser = new HttpBrowser($client);
+        $browser->request('POST', 'http://example.com/', [], [
+            'file1' => $this->getUploadedFile('file1'),
+            'file2' => 'INVALID',
+        ]);
+    }
+
+    private function uploadFile(string $data): string
+    {
+        $path = tempnam(sys_get_temp_dir(), 'http');
+        file_put_contents($path, $data);
+
+        return $path;
+    }
+
+    private function getUploadedFile(string $name): array
+    {
+        return [
+            'tmp_name' => $this->uploadFile($name.'_content'),
+            'name' => $name.'_name',
+        ];
+    }
+
+    protected function expectClientToSendRequestWithFiles(HttpClientInterface $client, $fileContents)
+    {
+        $client
+            ->expects($this->once())
+            ->method('request')
+            ->with('POST', 'http://example.com/', $this->callback(function ($options) use ($fileContents) {
+                $this->assertStringContainsString('Content-Type: multipart/form-data', implode('', $options['headers']));
+                $this->assertInstanceOf('\Generator', $options['body']);
+                $body = implode('', iterator_to_array($options['body'], false));
+                foreach ($fileContents as $content) {
+                    $this->assertStringContainsString($content, $body);
+                }
+
+                return true;
+            }))
+            ->willReturn($this->createMock(ResponseInterface::class));
+    }
 }