Skip to content

Commit e6c260d

Browse files
fabpotfabpot
committed
feature symfony#52493 [HttpFoundation] Add UploadedFile::getClientOriginalPath() to support directory uploads (danielburger1337)
This PR was merged into the 7.1 branch. Discussion ---------- [HttpFoundation] Add `UploadedFile::getClientOriginalPath()` to support directory uploads | Q | A | ------------- | --- | Branch? | 7.1 | Bug fix? | no | New feature? | yes | Deprecations? | no | License | MIT This removes the "hotfix" symfony#42112 and now supports "full_path" PHP file uploads. Since PHP 8.1 is now the lowest allowed version of PHP, this shouldnt cause any problems. --- Although I'm quite certain that I understand the messy FileBag code, I'm very concerned for the potential impact this change might have because of the widespread use of http-foundation. To the best of my knowledge, this won't break Symfony BC (yet) because this is all done by an `optional` argument in the constructor of UploadedFile (this should eventually be replaced by a mandatory argument though). Also, I'm not quite happy with the tests. Since this is only a "passthrough" of the PHP value, this _shouldnt_ be a huge problem. Someone should definitly go through this change very carefully. The tests pass and I also threw some "real world" application tests at it and everything seems to work just fine. But because of the nature of HttpFoundation and its intangled mess, I can't guarantee that I accounted for every possible or even simple edge case. --- Though it would be nice to still see this in 7.0, I know that it will probably be pushed back to 7.1. Commits ------- dac592b [HttpFoundation] Add `UploadedFile::getClientOriginalPath()` to support directory uploads
2 parents 4644f47 + dac592b commit e6c260d

File tree

9 files changed

+120
-39
lines changed

9 files changed

+120
-39
lines changed

‎src/Symfony/Component/Form/NativeRequestHandler.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ class NativeRequestHandler implements RequestHandlerInterface
2929
*/
3030
private const FILE_KEYS = [
3131
'error',
32+
'full_path',
3233
'name',
3334
'size',
3435
'tmp_name',
@@ -186,9 +187,7 @@ private static function fixPhpFilesArray(mixed $data): mixed
186187
return $data;
187188
}
188189

189-
// Remove extra key added by PHP 8.1.
190-
unset($data['full_path']);
191-
$keys = array_keys($data);
190+
$keys = array_keys($data + ['full_path' => null]);
192191
sort($keys);
193192

194193
if (self::FILE_KEYS !== $keys || !isset($data['name']) || !\is_array($data['name'])) {
@@ -207,7 +206,9 @@ private static function fixPhpFilesArray(mixed $data): mixed
207206
'type' => $data['type'][$key],
208207
'tmp_name' => $data['tmp_name'][$key],
209208
'size' => $data['size'][$key],
210-
]);
209+
] + (isset($data['full_path'][$key]) ? [
210+
'full_path' => $data['full_path'][$key],
211+
] : []));
211212
}
212213

213214
return $files;
@@ -219,7 +220,7 @@ private static function stripEmptyFiles(mixed $data): mixed
219220
return $data;
220221
}
221222

222-
$keys = array_keys($data);
223+
$keys = array_keys($data + ['full_path' => null]);
223224
sort($keys);
224225

225226
if (self::FILE_KEYS === $keys) {

‎src/Symfony/Component/Form/Tests/NativeRequestHandlerTest.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,9 @@ public function testFixBuggyFilesArray()
9999
'name' => [
100100
'field' => 'upload.txt',
101101
],
102+
'full_path' => [
103+
'field' => 'path/to/file/upload.txt',
104+
],
102105
'type' => [
103106
'field' => 'text/plain',
104107
],
@@ -118,6 +121,7 @@ public function testFixBuggyFilesArray()
118121
$this->assertTrue($form->isSubmitted());
119122
$this->assertEquals([
120123
'name' => 'upload.txt',
124+
'full_path' => 'path/to/file/upload.txt',
121125
'type' => 'text/plain',
122126
'tmp_name' => 'owfdskjasdfsa',
123127
'error' => \UPLOAD_ERR_OK,

‎src/Symfony/Component/HttpFoundation/CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
CHANGELOG
22
=========
33

4+
7.1
5+
---
6+
7+
* Add `UploadedFile::getClientOriginalPath()`
8+
49
7.0
510
---
611

‎src/Symfony/Component/HttpFoundation/File/UploadedFile.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ class UploadedFile extends File
3535
private string $originalName;
3636
private string $mimeType;
3737
private int $error;
38+
private string $originalPath;
3839

3940
/**
4041
* Accepts the information of the uploaded file as provided by the PHP global $_FILES.
@@ -63,6 +64,7 @@ class UploadedFile extends File
6364
public function __construct(string $path, string $originalName, string $mimeType = null, int $error = null, bool $test = false)
6465
{
6566
$this->originalName = $this->getName($originalName);
67+
$this->originalPath = strtr($originalName, '\\', '/');
6668
$this->mimeType = $mimeType ?: 'application/octet-stream';
6769
$this->error = $error ?: \UPLOAD_ERR_OK;
6870
$this->test = $test;
@@ -92,6 +94,21 @@ public function getClientOriginalExtension(): string
9294
return pathinfo($this->originalName, \PATHINFO_EXTENSION);
9395
}
9496

97+
/**
98+
* Returns the original file full path.
99+
*
100+
* It is extracted from the request from which the file has been uploaded.
101+
* This should not be considered as a safe value to use for a file name/path on your servers.
102+
*
103+
* If this file was uploaded with the "webkitdirectory" upload directive, this will contain
104+
* the path of the file relative to the uploaded root directory. Otherwise this will be identical
105+
* to getClientOriginalName().
106+
*/
107+
public function getClientOriginalPath(): string
108+
{
109+
return $this->originalPath;
110+
}
111+
95112
/**
96113
* Returns the file mime type.
97114
*

‎src/Symfony/Component/HttpFoundation/FileBag.php‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
*/
2222
class FileBag extends ParameterBag
2323
{
24-
private const FILE_KEYS = ['error', 'name', 'size', 'tmp_name', 'type'];
24+
private const FILE_KEYS = ['error', 'full_path', 'name', 'size', 'tmp_name', 'type'];
2525

2626
/**
2727
* @param array|UploadedFile[] $parameters An array of HTTP files
@@ -65,18 +65,18 @@ protected function convertFileInformation(array|UploadedFile $file): array|Uploa
6565
}
6666

6767
$file = $this->fixPhpFilesArray($file);
68-
$keys = array_keys($file);
68+
$keys = array_keys($file + ['full_path' => null]);
6969
sort($keys);
7070

71-
if (self::FILE_KEYS == $keys) {
72-
if (\UPLOAD_ERR_NO_FILE == $file['error']) {
71+
if (self::FILE_KEYS === $keys) {
72+
if (\UPLOAD_ERR_NO_FILE === $file['error']) {
7373
$file = null;
7474
} else {
75-
$file = new UploadedFile($file['tmp_name'], $file['name'], $file['type'], $file['error'], false);
75+
$file = new UploadedFile($file['tmp_name'], $file['full_path'] ?? $file['name'], $file['type'], $file['error'], false);
7676
}
7777
} else {
7878
$file = array_map(fn ($v) => $v instanceof UploadedFile || \is_array($v) ? $this->convertFileInformation($v) : $v, $file);
79-
if (array_keys($keys) === $keys) {
79+
if (array_is_list($file)) {
8080
$file = array_filter($file);
8181
}
8282
}
@@ -98,12 +98,10 @@ protected function convertFileInformation(array|UploadedFile $file): array|Uploa
9898
*/
9999
protected function fixPhpFilesArray(array $data): array
100100
{
101-
// Remove extra key added by PHP 8.1.
102-
unset($data['full_path']);
103-
$keys = array_keys($data);
101+
$keys = array_keys($data + ['full_path' => null]);
104102
sort($keys);
105103

106-
if (self::FILE_KEYS != $keys || !isset($data['name']) || !\is_array($data['name'])) {
104+
if (self::FILE_KEYS !== $keys || !isset($data['name']) || !\is_array($data['name'])) {
107105
return $data;
108106
}
109107

@@ -119,7 +117,9 @@ protected function fixPhpFilesArray(array $data): array
119117
'type' => $data['type'][$key],
120118
'tmp_name' => $data['tmp_name'][$key],
121119
'size' => $data['size'][$key],
122-
]);
120+
] + (isset($data['full_path'][$key]) ? [
121+
'full_path' => $data['full_path'][$key],
122+
] : []));
123123
}
124124

125125
return $files;

‎src/Symfony/Component/HttpFoundation/Tests/File/Fixtures/webkitdirectory/nested/test.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
nested webkitdirectory text

‎src/Symfony/Component/HttpFoundation/Tests/File/Fixtures/webkitdirectory/test.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
webkitdirectory text

‎src/Symfony/Component/HttpFoundation/Tests/File/UploadedFileTest.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,4 +322,26 @@ public function testGetMaxFilesize()
322322
$this->assertSame(\PHP_INT_MAX, $size);
323323
}
324324
}
325+
326+
public function testgetClientOriginalPath()
327+
{
328+
$file = new UploadedFile(
329+
__DIR__.'/Fixtures/test.gif',
330+
'test.gif',
331+
'image/gif'
332+
);
333+
334+
$this->assertEquals('test.gif', $file->getClientOriginalPath());
335+
}
336+
337+
public function testgetClientOriginalPathWebkitDirectory()
338+
{
339+
$file = new UploadedFile(
340+
__DIR__.'/Fixtures/webkitdirectory/test.txt',
341+
'webkitdirectory/test.txt',
342+
'text/plain',
343+
);
344+
345+
$this->assertEquals('webkitdirectory/test.txt', $file->getClientOriginalPath());
346+
}
325347
}

‎src/Symfony/Component/HttpFoundation/Tests/FileBagTest.php‎

Lines changed: 53 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -32,27 +32,12 @@ public function testFileMustBeAnArrayOrUploadedFile()
3232
public function testShouldConvertsUploadedFiles()
3333
{
3434
$tmpFile = $this->createTempFile();
35-
$file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain');
35+
$name = basename($tmpFile);
3636

37-
$bag = new FileBag(['file' => [
38-
'name' => basename($tmpFile),
39-
'type' => 'text/plain',
40-
'tmp_name' => $tmpFile,
41-
'error' => 0,
42-
'size' => null,
43-
]]);
44-
45-
$this->assertEquals($file, $bag->get('file'));
46-
}
47-
48-
public function testShouldConvertsUploadedFilesPhp81()
49-
{
50-
$tmpFile = $this->createTempFile();
51-
$file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain');
37+
$file = new UploadedFile($tmpFile, $name, 'text/plain');
5238

5339
$bag = new FileBag(['file' => [
54-
'name' => basename($tmpFile),
55-
'full_path' => basename($tmpFile),
40+
'name' => $name,
5641
'type' => 'text/plain',
5742
'tmp_name' => $tmpFile,
5843
'error' => 0,
@@ -104,12 +89,13 @@ public function testShouldNotRemoveEmptyUploadedFilesForAssociativeArray()
10489
public function testShouldConvertUploadedFilesWithPhpBug()
10590
{
10691
$tmpFile = $this->createTempFile();
107-
$file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain');
92+
$name = basename($tmpFile);
93+
$file = new UploadedFile($tmpFile, $name, 'text/plain');
10894

10995
$bag = new FileBag([
11096
'child' => [
11197
'name' => [
112-
'file' => basename($tmpFile),
98+
'file' => $name,
11399
],
114100
'type' => [
115101
'file' => 'text/plain',
@@ -133,12 +119,13 @@ public function testShouldConvertUploadedFilesWithPhpBug()
133119
public function testShouldConvertNestedUploadedFilesWithPhpBug()
134120
{
135121
$tmpFile = $this->createTempFile();
136-
$file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain');
122+
$name = basename($tmpFile);
123+
$file = new UploadedFile($tmpFile, $name, 'text/plain');
137124

138125
$bag = new FileBag([
139126
'child' => [
140127
'name' => [
141-
'sub' => ['file' => basename($tmpFile)],
128+
'sub' => ['file' => $name],
142129
],
143130
'type' => [
144131
'sub' => ['file' => 'text/plain'],
@@ -162,13 +149,56 @@ public function testShouldConvertNestedUploadedFilesWithPhpBug()
162149
public function testShouldNotConvertNestedUploadedFiles()
163150
{
164151
$tmpFile = $this->createTempFile();
165-
$file = new UploadedFile($tmpFile, basename($tmpFile), 'text/plain');
152+
$name = basename($tmpFile);
153+
$file = new UploadedFile($tmpFile, $name, 'text/plain');
166154
$bag = new FileBag(['image' => ['file' => $file]]);
167155

168156
$files = $bag->all();
169157
$this->assertEquals($file, $files['image']['file']);
170158
}
171159

160+
public function testWebkitDirectoryUpload()
161+
{
162+
$file1 = __DIR__.'/File/Fixtures/webkitdirectory/test.txt';
163+
$file2 = __DIR__.'/File/Fixtures/webkitdirectory/nested/test.txt';
164+
165+
$bag = new FileBag([
166+
'child' => [
167+
'name' => [
168+
'test.txt',
169+
'test.txt',
170+
],
171+
'full_path' => [
172+
'webkitdirectory/test.txt',
173+
'webkitdirectory/nested/test.txt',
174+
],
175+
'type' => [
176+
'text/plain',
177+
'text/plain',
178+
],
179+
'tmp_name' => [
180+
$file1,
181+
$file2,
182+
],
183+
'error' => [
184+
0, 0,
185+
],
186+
'size' => [
187+
null, null,
188+
],
189+
],
190+
]);
191+
192+
/** @var UploadedFile[] */
193+
$files = $bag->get('child');
194+
195+
$this->assertEquals('test.txt', $files[0]->getClientOriginalName());
196+
$this->assertEquals('test.txt', $files[1]->getClientOriginalName());
197+
198+
$this->assertEquals('webkitdirectory/test.txt', $files[0]->getClientOriginalPath());
199+
$this->assertEquals('webkitdirectory/nested/test.txt', $files[1]->getClientOriginalPath());
200+
}
201+
172202
protected function createTempFile()
173203
{
174204
$tempFile = tempnam(sys_get_temp_dir().'/form_test', 'FormTest');

0 commit comments

Comments
 (0)