Skip to content

Commit

Permalink
Directory and filename must be no more than 255 characters in length
Browse files Browse the repository at this point in the history
  • Loading branch information
lfluvisotto committed Sep 23, 2020
1 parent b9c13fd commit 52f39a4
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 10 deletions.
12 changes: 12 additions & 0 deletions app/code/Magento/CatalogImportExport/Model/Import/Uploader.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ class Uploader extends \Magento\MediaStorage\Model\File\Uploader
*/
private $fileSystem;

/**
* Directory and filename must be no more than 255 characters in length
*/
private $maxFilenameLength = 255;

/**
* @param \Magento\MediaStorage\Helper\File\Storage\Database $coreFileStorageDb
* @param \Magento\MediaStorage\Helper\File\Storage $coreFileStorage
Expand Down Expand Up @@ -188,6 +193,13 @@ public function move($fileName, $renameFileOff = false)
unset($result['path']);
$result['name'] = self::getCorrectFileName($result['name']);

// Directory and filename must be no more than 255 characters in length
if (strlen($result['file']) > $this->maxFilenameLength) {
throw new \LengthException(
__('Filename is too long; must be %1 characters or less', $this->maxFilenameLength)
);
}

return $result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Magento\CatalogImportExport\Model\Import\Uploader;
use Magento\Framework\Exception\LocalizedException;
use Magento\Framework\Filesystem;
use Magento\Framework\Filesystem\Directory\Write;
use Magento\Framework\Filesystem\Driver\Http;
use Magento\Framework\Filesystem\Driver\Https;
use Magento\Framework\Filesystem\DriverPool;
Expand Down Expand Up @@ -58,7 +59,7 @@ class UploaderTest extends TestCase
protected $readFactory;

/**
* @var \Magento\Framework\Filesystem\Directory\Writer|MockObject
* @var WriteInterface|MockObject
*/
protected $directoryMock;

Expand Down Expand Up @@ -96,7 +97,7 @@ protected function setUp(): void
->setMethods(['create'])
->getMock();

$this->directoryMock = $this->getMockBuilder(\Magento\Framework\Filesystem\Directory\Writer::class)
$this->directoryMock = $this->getMockBuilder(Write::class)
->setMethods(['writeFile', 'getRelativePath', 'isWritable', 'getAbsolutePath'])
->disableOriginalConstructor()
->getMock();
Expand Down Expand Up @@ -186,14 +187,21 @@ public function testMoveFileUrl($fileUrl, $expectedHost, $expectedFileName, $che
->willReturn($destDir . '/' . $expectedFileName);
$this->uploader->expects($this->once())->method('_setUploadFile')
->willReturnSelf();

$returnFile = $destDir . DIRECTORY_SEPARATOR . $expectedFileName;

$this->uploader->expects($this->once())->method('save')
->with($destDir . '/' . $expectedFileName)
->willReturn(['name' => $expectedFileName, 'path' => 'absPath']);
->willReturn([
'name' => $expectedFileName,
'path' => 'absPath',
'file' => $returnFile
]);

$this->uploader->setDestDir($destDir);
$result = $this->uploader->move($fileUrl);

$this->assertEquals(['name' => $expectedFileName], $result);
$this->assertEquals(['name' => $expectedFileName, 'file' => $returnFile], $result);
$this->assertArrayNotHasKey('path', $result);
}

Expand All @@ -209,11 +217,50 @@ public function testMoveFileName()
//Check invoking of getTmpDir(), _setUploadFile(), save() methods.
$this->uploader->expects($this->once())->method('getTmpDir')->willReturn('');
$this->uploader->expects($this->once())->method('_setUploadFile')->willReturnSelf();

$returnFile = $destDir . DIRECTORY_SEPARATOR . $fileName;

$this->uploader->expects($this->once())->method('save')->with($destDir . '/' . $fileName)
->willReturn(['name' => $fileName]);
->willReturn(['name' => $fileName, 'file' => $returnFile]);

$this->uploader->setDestDir($destDir);
$this->assertEquals(['name' => $fileName, 'file' => $returnFile], $this->uploader->move($fileName));
}

public function testFilenameLength()
{
$destDir = 'var/tmp/' . str_repeat('testFilenameLength', 13); // 242 characters

$fileName = \uniqid(); // 13 characters

$this->directoryMock->expects($this->once())
->method('isWritable')
->with($destDir)
->willReturn(true);

$this->directoryMock->expects($this->once())
->method('getRelativePath')
->with($fileName)
->willReturn(null);

$this->directoryMock->expects($this->once())
->method('getAbsolutePath')
->with($destDir)
->willReturn($destDir);

$this->uploader->expects($this->once())
->method('save')
->with($destDir)
->willReturn([
'name' => $fileName,
'file' => $destDir . DIRECTORY_SEPARATOR . $fileName // 256 characters
]);

$this->uploader->setDestDir($destDir);
$this->assertEquals(['name' => $fileName], $this->uploader->move($fileName));

$this->expectException(\LengthException::class);

$this->uploader->move($fileName);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public function testGetCorrectFileName($fileName, $expectedCorrectedFileName)
$isExceptionExpected = $expectedCorrectedFileName === true;

if ($isExceptionExpected) {
$this->expectException(\InvalidArgumentException::class);
$this->expectException(\LengthException::class);
}

$this->assertEquals(
Expand Down Expand Up @@ -62,7 +62,7 @@ public function getCorrectFileNameProvider()
'a.' . str_repeat('b', 88)
],
[
'a.' . str_repeat('b', 89),
'a.' . str_repeat('b', 199), // 201 characters
true
]
];
Expand Down
8 changes: 6 additions & 2 deletions lib/internal/Magento/Framework/File/Uploader.php
Original file line number Diff line number Diff line change
Expand Up @@ -407,8 +407,12 @@ public static function getCorrectFileName($fileName)
$fileInfo['extension'] = $fileInfo['extension'] ?? '';

// account for excessively long filenames that cannot be stored completely in database
if (strlen($fileInfo['basename']) > 90) {
throw new \InvalidArgumentException('Filename is too long; must be 90 characters or less');
$maxFilenameLength = 200;

if (strlen($fileInfo['basename']) > $maxFilenameLength) {
throw new \LengthException(
__('Filename is too long; must be %1 characters or less', $maxFilenameLength)
);
}

if (preg_match('/^_+$/', $fileInfo['filename'])) {
Expand Down

0 comments on commit 52f39a4

Please sign in to comment.