Skip to content

Commit 084e48a

Browse files
authored
Fix directories named "0" being dropped in storage URI resolution (#1534)
1 parent caa3abb commit 084e48a

7 files changed

Lines changed: 111 additions & 9 deletions

File tree

src/Storage/AbstractStorage.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ public function resolveUri(object|array $obj, ?string $fieldName = null, ?string
108108
}
109109

110110
$dir = $mapping->getUploadDir($obj);
111-
$path = !empty($dir) ? $dir.'/'.$filename : $filename;
111+
$path = (\is_string($dir) && '' !== $dir) ? $dir.'/'.$filename : $filename;
112112

113113
return $mapping->getUriPrefix().'/'.$path;
114114
}

src/Storage/FileSystemStorage.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ protected function doResolvePath(
5454
string $name,
5555
?bool $relative = false
5656
): string {
57-
$path = !empty($dir) ? $dir.\DIRECTORY_SEPARATOR.$name : $name;
57+
$path = (\is_string($dir) && '' !== $dir) ? $dir.\DIRECTORY_SEPARATOR.$name : $name;
5858

5959
if ($relative) {
6060
return $path;
@@ -72,7 +72,7 @@ public function resolveUri(object|array $obj, ?string $fieldName = null, ?string
7272
}
7373

7474
$uploadDir = $this->convertWindowsDirectorySeparator($mapping->getUploadDir($obj));
75-
$uploadDir = empty($uploadDir) ? '' : $uploadDir.'/';
75+
$uploadDir = (\is_string($uploadDir) && '' !== $uploadDir) ? $uploadDir.'/' : '';
7676

7777
return \sprintf('%s/%s', $mapping->getUriPrefix(), $uploadDir.$name);
7878
}

src/Storage/FlysystemStorage.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public function __construct(PropertyMappingFactory $factory, mixed $registry, bo
4646
protected function doUpload(PropertyMapping $mapping, File $file, ?string $dir, string $name): void
4747
{
4848
$fs = $this->getFilesystem($mapping);
49-
$path = !empty($dir) ? $dir.'/'.$name : $name;
49+
$path = (\is_string($dir) && '' !== $dir) ? $dir.'/'.$name : $name;
5050

5151
$stream = \fopen($file->getRealPath(), 'rb');
5252
try {
@@ -61,7 +61,7 @@ protected function doUpload(PropertyMapping $mapping, File $file, ?string $dir,
6161
protected function doRemove(PropertyMapping $mapping, ?string $dir, string $name): ?bool
6262
{
6363
$fs = $this->getFilesystem($mapping);
64-
$path = !empty($dir) ? $dir.'/'.$name : $name;
64+
$path = (\is_string($dir) && '' !== $dir) ? $dir.'/'.$name : $name;
6565

6666
$fs->delete($path);
6767

@@ -70,7 +70,7 @@ protected function doRemove(PropertyMapping $mapping, ?string $dir, string $name
7070

7171
protected function doResolvePath(PropertyMapping $mapping, ?string $dir, string $name, ?bool $relative = false): string
7272
{
73-
$path = !empty($dir) ? $dir.'/'.$name : $name;
73+
$path = (\is_string($dir) && '' !== $dir) ? $dir.'/'.$name : $name;
7474

7575
if ($relative) {
7676
return $path;

src/Storage/GaufretteStorage.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public function __construct(PropertyMappingFactory $factory, protected Filesyste
3131
protected function doUpload(PropertyMapping $mapping, File $file, ?string $dir, string $name): void
3232
{
3333
$filesystem = $this->getFilesystem($mapping);
34-
$path = !empty($dir) ? $dir.'/'.$name : $name;
34+
$path = (\is_string($dir) && '' !== $dir) ? $dir.'/'.$name : $name;
3535

3636
$filesystem->write($path, \file_get_contents($file->getPathname()), true);
3737

@@ -43,14 +43,14 @@ protected function doUpload(PropertyMapping $mapping, File $file, ?string $dir,
4343
protected function doRemove(PropertyMapping $mapping, ?string $dir, string $name): ?bool
4444
{
4545
$filesystem = $this->getFilesystem($mapping);
46-
$path = !empty($dir) ? $dir.'/'.$name : $name;
46+
$path = (\is_string($dir) && '' !== $dir) ? $dir.'/'.$name : $name;
4747

4848
return $filesystem->delete($path);
4949
}
5050

5151
protected function doResolvePath(PropertyMapping $mapping, ?string $dir, string $name, ?bool $relative = false): string
5252
{
53-
$path = !empty($dir) ? $dir.'/'.$name : $name;
53+
$path = (\is_string($dir) && '' !== $dir) ? $dir.'/'.$name : $name;
5454

5555
if ($relative) {
5656
return $path;

tests/Storage/FileSystemStorageTest.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,10 @@ public static function resolveUriDataProvider(): array
252252
'dir\\sub-dir',
253253
'/uploads/dir/sub-dir/file.txt',
254254
],
255+
[
256+
'0',
257+
'/uploads/0/file.txt',
258+
],
255259
];
256260
}
257261

tests/Storage/Flysystem/AbstractFlysystemStorageTestCase.php

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,8 @@ public static function pathProvider(): array
154154
[null, 'file.txt', false],
155155
['foo', 'foo/file.txt', true],
156156
['foo', 'foo/file.txt', false],
157+
['0', '0/file.txt', true],
158+
['0', '0/file.txt', false],
157159
];
158160
}
159161

@@ -180,6 +182,34 @@ public function testResolveUri(): void
180182
self::assertEquals('/uploads/file.txt', $path);
181183
}
182184

185+
public function testResolveUriWithZeroDirectory(): void
186+
{
187+
$this->mapping
188+
->expects(self::once())
189+
->method('getUriPrefix')
190+
->willReturn('/uploads');
191+
192+
$this->mapping
193+
->expects(self::once())
194+
->method('getUploadDir')
195+
->willReturn('0');
196+
197+
$this->mapping
198+
->expects(self::once())
199+
->method('getFileName')
200+
->willReturn('file.txt');
201+
202+
$this->factory
203+
->expects(self::once())
204+
->method('fromField')
205+
->with($this->object, 'file_field')
206+
->willReturn($this->mapping);
207+
208+
$path = $this->getStorage()->resolveUri($this->object, 'file_field');
209+
210+
self::assertEquals('/uploads/0/file.txt', $path);
211+
}
212+
183213
#[RequiresMethod(Filesystem::class, 'publicUrl')]
184214
public function testResolveUriThroughFlysystem(): void
185215
{
@@ -212,6 +242,43 @@ public function testResolveUriThroughFlysystem(): void
212242
self::assertEquals('example.com/file.txt', $path);
213243
}
214244

245+
#[RequiresMethod(Filesystem::class, 'publicUrl')]
246+
public function testResolveUriThroughFlysystemWithZeroDirectory(): void
247+
{
248+
$this->useFlysystemToResolveUri = true;
249+
250+
$this->filesystem
251+
->expects(self::once())
252+
->method('publicUrl')
253+
->with('0/file.txt', [
254+
'object' => $this->object,
255+
'fieldName' => 'file_field',
256+
'className' => null,
257+
'mapping' => $this->mapping,
258+
])
259+
->willReturn('example.com/0/file.txt');
260+
261+
$this->mapping
262+
->expects(self::once())
263+
->method('getUploadDir')
264+
->willReturn('0');
265+
266+
$this->mapping
267+
->expects(self::once())
268+
->method('getFileName')
269+
->willReturn('file.txt');
270+
271+
$this->factory
272+
->expects(self::exactly(2))
273+
->method('fromField')
274+
->with($this->object, 'file_field')
275+
->willReturn($this->mapping);
276+
277+
$path = $this->getStorage()->resolveUri($this->object, 'file_field');
278+
279+
self::assertEquals('example.com/0/file.txt', $path);
280+
}
281+
215282
#[RequiresMethod(Filesystem::class, 'publicUrl')]
216283
public function testResolveUriHandlesUndefinedMethodError(): void
217284
{

tests/Storage/GaufretteStorageTest.php

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,15 +121,46 @@ public function testResolveUriFileNull(): void
121121
self::assertNull($path);
122122
}
123123

124+
public function testResolveUriWithZeroDirectory(): void
125+
{
126+
$this->mapping
127+
->expects(self::once())
128+
->method('getUriPrefix')
129+
->willReturn('/uploads');
130+
131+
$this->mapping
132+
->expects(self::once())
133+
->method('getUploadDir')
134+
->willReturn('0');
135+
136+
$this->mapping
137+
->expects(self::once())
138+
->method('getFileName')
139+
->willReturn('file.txt');
140+
141+
$this->factory
142+
->expects(self::once())
143+
->method('fromField')
144+
->with($this->object, 'file_field')
145+
->willReturn($this->mapping);
146+
147+
$this->storage = new GaufretteStorage($this->factory, $this->filesystemMap, 'gaufrette');
148+
$path = $this->storage->resolveUri($this->object, 'file_field');
149+
150+
self::assertEquals('/uploads/0/file.txt', $path);
151+
}
152+
124153
public static function pathProvider(): array
125154
{
126155
return [
127156
// protocol, fs identifier, upload dir, full path, relative
128157
['gaufrette', 'filesystemKey', null, 'gaufrette://filesystemKey/file.txt', false],
129158
['data', 'filesystemKey', null, 'data://filesystemKey/file.txt', false],
130159
['gaufrette', 'filesystemKey', 'foo', 'gaufrette://filesystemKey/foo/file.txt', false],
160+
['gaufrette', 'filesystemKey', '0', 'gaufrette://filesystemKey/0/file.txt', false],
131161
['gaufrette', 'filesystemKey', null, 'file.txt', true],
132162
['gaufrette', 'filesystemKey', 'foo', 'foo/file.txt', true],
163+
['gaufrette', 'filesystemKey', '0', '0/file.txt', true],
133164
];
134165
}
135166

0 commit comments

Comments
 (0)