Skip to content

Commit 13d8ccd

Browse files
joanhejoanhe
authored
Merge pull request #7196 from magento-arcticfoxes/B2B-2069
B2B-2069: Images from remote directory is throwing errors on lib/internal/Magento/Framework/Filesystem/Directory/Write.php
2 parents f95efe4 + b8d69bf commit 13d8ccd

File tree

9 files changed

+166
-103
lines changed

9 files changed

+166
-103
lines changed

‎app/code/Magento/AwsS3/Driver/AwsS3.php‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
* Driver for AWS S3 IO operations.
2626
*
2727
* @SuppressWarnings(PHPMD.ExcessiveClassComplexity)
28+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
2829
*/
2930
class AwsS3 implements RemoteDriverInterface
3031
{
@@ -258,6 +259,7 @@ public function filePutContents($path, $content, $mode = null): int
258259
$path = $this->normalizeRelativePath($path, true);
259260
$config = self::CONFIG;
260261

262+
// phpcs:ignore Generic.PHP.NoSilencedErrors.Discouraged
261263
if (false !== ($imageSize = @getimagesizefromstring($content))) {
262264
$config['Metadata'] = [
263265
'image-width' => $imageSize[0],
@@ -295,19 +297,20 @@ public function readDirectory($path): array
295297
*/
296298
public function getRealPathSafety($path)
297299
{
300+
//Removing redundant directory separators
301+
$path = preg_replace(
302+
'~(?<!:)\/\/+~',
303+
'/',
304+
$path
305+
);
306+
298307
if (strpos($path, '/.') === false) {
299308
return $path;
300309
}
301310

302311
$isAbsolute = strpos($path, $this->normalizeAbsolutePath('')) === 0;
303312
$path = $this->normalizeRelativePath($path);
304313

305-
//Removing redundant directory separators.
306-
$path = preg_replace(
307-
'/\\/\\/+/',
308-
'/',
309-
$path
310-
);
311314
$pathParts = explode('/', $path);
312315
if (end($pathParts) === '.') {
313316
$pathParts[count($pathParts) - 1] = '';
@@ -488,8 +491,7 @@ public function getRelativePath($basePath, $path = null): string
488491
$basePath = (string)$basePath;
489492
$path = (string)$path;
490493

491-
if (
492-
($basePath && $path)
494+
if ($basePath && $path
493495
&& ($basePath === $path . '/' || strpos($path, $basePath) === 0)
494496
) {
495497
$result = substr($path, strlen($basePath));
@@ -722,6 +724,7 @@ public function fileGetCsv($resource, $length = 0, $delimiter = ',', $enclosure
722724
*/
723725
public function fileTell($resource): int
724726
{
727+
// phpcs:ignore Magento2.Functions.DiscouragedFunction, Generic.PHP.NoSilencedErrors.Discouraged
725728
$result = @ftell($resource);
726729
if ($result === null) {
727730
throw new FileSystemException(
@@ -737,6 +740,7 @@ public function fileTell($resource): int
737740
*/
738741
public function fileSeek($resource, $offset, $whence = SEEK_SET): int
739742
{
743+
// phpcs:ignore Magento2.Functions.DiscouragedFunction, Generic.PHP.NoSilencedErrors.Discouraged
740744
$result = @fseek($resource, $offset, $whence);
741745
if ($result === -1) {
742746
throw new FileSystemException(
@@ -755,6 +759,7 @@ public function fileSeek($resource, $offset, $whence = SEEK_SET): int
755759
*/
756760
public function endOfFile($resource): bool
757761
{
762+
// phpcs:ignore Magento2.Functions.DiscouragedFunction.DiscouragedWithAlternative
758763
return feof($resource);
759764
}
760765

@@ -772,6 +777,7 @@ public function filePutCsv($resource, array $data, $delimiter = ',', $enclosure
772777
*/
773778
public function fileFlush($resource): bool
774779
{
780+
// phpcs:ignore Magento2.Functions.DiscouragedFunction, Generic.PHP.NoSilencedErrors.Discouraged
775781
$result = @fflush($resource);
776782
if (!$result) {
777783
throw new FileSystemException(
@@ -790,6 +796,7 @@ public function fileFlush($resource): bool
790796
*/
791797
public function fileLock($resource, $lockMode = LOCK_EX): bool
792798
{
799+
// phpcs:ignore Magento2.Functions.DiscouragedFunction, Generic.PHP.NoSilencedErrors.Discouraged
793800
$result = @flock($resource, $lockMode);
794801
if (!$result) {
795802
throw new FileSystemException(
@@ -808,6 +815,7 @@ public function fileLock($resource, $lockMode = LOCK_EX): bool
808815
*/
809816
public function fileUnlock($resource): bool
810817
{
818+
// phpcs:ignore Magento2.Functions.DiscouragedFunction, Generic.PHP.NoSilencedErrors.Discouraged
811819
$result = @flock($resource, LOCK_UN);
812820
if (!$result) {
813821
throw new FileSystemException(
@@ -851,13 +859,14 @@ public function fileClose($resource): bool
851859
//phpcs:enable
852860

853861
foreach ($this->streams as $path => $stream) {
854-
// phpcs:ignore Magento2.Functions.DiscouragedFunction
862+
// phpcs:ignore
855863
if (stream_get_meta_data($stream)['uri'] === $resourcePath) {
856864
$this->adapter->writeStream($path, $resource, new Config(self::CONFIG));
857865

858866
// Remove path from streams after
859867
unset($this->streams[$path]);
860868

869+
// phpcs:ignore Magento2.Functions.DiscouragedFunction.DiscouragedWithAlternative
861870
return fclose($stream);
862871
}
863872
}
@@ -931,6 +940,7 @@ private function readPath(string $path, $isRecursive = false): array
931940
if (!empty($path)
932941
&& $path !== $relativePath
933942
&& (!$relativePath || strpos($path, $relativePath) === 0)) {
943+
//phpcs:ignore Magento2.Functions.DiscouragedFunction
934944
$itemsList[] = $this->getAbsolutePath(dirname($path), $path);
935945
}
936946
}

‎app/code/Magento/AwsS3/Test/Unit/Driver/AwsS3Test.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ public function testGetAbsolutePath($basePath, $path, string $expected): void
6464

6565
/**
6666
* @return array
67+
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
6768
*/
6869
public function getAbsolutePathDataProvider(): array
6970
{
@@ -407,6 +408,18 @@ public function getRealPathSafetyDataProvider(): array
407408
[
408409
'test/test/../test.txt',
409410
'test/test.txt'
411+
],
412+
[
413+
'test//test/../test.txt',
414+
'test/test.txt'
415+
],
416+
[
417+
'test1///test2/..//test3//test.txt',
418+
'test1/test3/test.txt'
419+
],
420+
[
421+
self::URL . '/test1///test2/..//test3//test.txt',
422+
self::URL . 'test1/test3/test.txt'
410423
]
411424
];
412425
}

‎app/code/Magento/Customer/Model/Metadata/Form/Image.php‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use Magento\Framework\Exception\LocalizedException;
2323
use Magento\Framework\File\UploaderFactory;
2424
use Magento\Framework\Filesystem;
25+
use Magento\Framework\Filesystem\Directory\ReadInterface;
2526
use Magento\Framework\Filesystem\Directory\WriteFactory;
2627
use Magento\Framework\Filesystem\Directory\WriteInterface;
2728
use Magento\Framework\Filesystem\Io\File as IoFileSystem;
@@ -48,10 +49,15 @@ class Image extends File
4849
*/
4950
private $ioFileSystem;
5051

52+
/**
53+
* @var ReadInterface
54+
*/
55+
private $mediaEntityTmpReadDirectory;
56+
5157
/**
5258
* @var WriteInterface
5359
*/
54-
private $mediaEntityTmpDirectory;
60+
private $mediaWriteDirectory;
5561

5662
/**
5763
* @param TimezoneInterface $localeDate
@@ -71,6 +77,7 @@ class Image extends File
7177
* @param DirectoryList|null $directoryList
7278
* @param WriteFactory|null $writeFactory
7379
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
80+
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
7481
* @throws FileSystemException
7582
*/
7683
public function __construct(
@@ -109,12 +116,10 @@ public function __construct(
109116
->get(ImageContentInterfaceFactory::class);
110117
$this->ioFileSystem = $ioFileSystem ?: ObjectManager::getInstance()
111118
->get(IoFileSystem::class);
112-
$writeFactory = $writeFactory ?? ObjectManager::getInstance()->get(WriteFactory::class);
113-
$directoryList = $directoryList ?? ObjectManager::getInstance()->get(DirectoryList::class);
114-
$this->mediaEntityTmpDirectory = $writeFactory->create(
115-
$directoryList->getPath($directoryList::MEDIA)
116-
. '/' . $this->_entityTypeCode
117-
. '/' . FileProcessor::TMP_DIR
119+
$this->mediaWriteDirectory = $fileSystem->getDirectoryWrite(DirectoryList::MEDIA);
120+
$this->mediaEntityTmpReadDirectory = $fileSystem->getDirectoryReadByPath(
121+
$this->mediaWriteDirectory->getAbsolutePath() . $this->_entityTypeCode
122+
. DIRECTORY_SEPARATOR . FileProcessor::TMP_DIR . DIRECTORY_SEPARATOR
118123
);
119124
}
120125

@@ -135,6 +140,7 @@ protected function _validateByRules($value)
135140
$rules = $this->getAttribute()->getValidationRules();
136141

137142
try {
143+
// phpcs:ignore Magento2.Functions.DiscouragedFunction
138144
$imageProp = getimagesize($value['tmp_name']);
139145
} catch (\Throwable $e) {
140146
$imageProp = false;
@@ -224,18 +230,18 @@ protected function processUiComponentValue(array $value)
224230
*/
225231
protected function processCustomerAddressValue(array $value)
226232
{
227-
$fileName = $this->mediaEntityTmpDirectory
233+
$fileName = $this->mediaWriteDirectory
228234
->getDriver()
229235
->getRealPathSafety(
230-
$this->mediaEntityTmpDirectory->getAbsolutePath(
236+
$this->mediaEntityTmpReadDirectory->getAbsolutePath(
231237
ltrim(
232238
$value['file'],
233239
'/'
234240
)
235241
)
236242
);
237243
return $this->getFileProcessor()->moveTemporaryFile(
238-
$this->mediaEntityTmpDirectory->getRelativePath($fileName)
244+
$this->mediaEntityTmpReadDirectory->getRelativePath($fileName)
239245
);
240246
}
241247

@@ -249,7 +255,7 @@ protected function processCustomerAddressValue(array $value)
249255
protected function processCustomerValue(array $value)
250256
{
251257
$file = ltrim($value['file'], '/');
252-
if ($this->mediaEntityTmpDirectory->isExist($file)) {
258+
if ($this->mediaEntityTmpReadDirectory->isExist($file)) {
253259
$temporaryFile = FileProcessor::TMP_DIR . '/' . $file;
254260
$base64EncodedData = $this->getFileProcessor()->getBase64EncodedData($temporaryFile);
255261
/** @var ImageContentInterface $imageContentDataObject */

‎app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php‎

Lines changed: 22 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,12 @@
1515
use Magento\Customer\Model\Metadata\Form\Image;
1616
use Magento\Framework\Api\Data\ImageContentInterface;
1717
use Magento\Framework\Api\Data\ImageContentInterfaceFactory;
18-
use Magento\Framework\App\Filesystem\DirectoryList;
1918
use Magento\Framework\App\Request\Http;
2019
use Magento\Framework\Exception\FileSystemException;
2120
use Magento\Framework\Exception\LocalizedException;
2221
use Magento\Framework\File\UploaderFactory;
2322
use Magento\Framework\Filesystem;
24-
use Magento\Framework\Filesystem\Directory\WriteFactory;
23+
use Magento\Framework\Filesystem\Directory\ReadInterface;
2524
use Magento\Framework\Filesystem\Directory\Write;
2625
use Magento\Framework\Filesystem\Driver\File as Driver;
2726
use Magento\Framework\Filesystem\Io\File;
@@ -82,19 +81,14 @@ class ImageTest extends AbstractFormTestCase
8281
private $ioFileSystemMock;
8382

8483
/**
85-
* @var DirectoryList|\PHPUnit\Framework\MockObject\MockObject
84+
* @var ReadInterface|\PHPUnit\Framework\MockObject\MockObject
8685
*/
87-
private $directoryListMock;
88-
89-
/**
90-
* @var WriteFactory|\PHPUnit\Framework\MockObject\MockObject
91-
*/
92-
private $writeFactoryMock;
86+
private $readDirectoryMock;
9387

9488
/**
9589
* @var Write|\PHPUnit\Framework\MockObject\MockObject
9690
*/
97-
private $mediaEntityTmpDirectoryMock;
91+
private $mediaWriteDirectoryMock;
9892

9993
/**
10094
* @var Driver|\PHPUnit\Framework\MockObject\MockObject
@@ -140,23 +134,24 @@ protected function setUp(): void
140134
$this->ioFileSystemMock = $this->getMockBuilder(File::class)
141135
->disableOriginalConstructor()
142136
->getMock();
143-
$this->directoryListMock = $this->getMockBuilder(DirectoryList::class)
144-
->disableOriginalConstructor()
145-
->getMock();
146-
$this->writeFactoryMock = $this->getMockBuilder(WriteFactory::class)
147-
->setMethods(['create'])
148-
->disableOriginalConstructor()
149-
->getMock();
150-
$this->mediaEntityTmpDirectoryMock = $this->getMockBuilder(Write::class)
137+
138+
$this->readDirectoryMock = $this->getMockBuilder(ReadInterface::class)
139+
->getMockForAbstractClass();
140+
$this->fileSystemMock->expects($this->once())
141+
->method('getDirectoryReadByPath')
142+
->willReturn($this->readDirectoryMock);
143+
144+
$this->mediaWriteDirectoryMock = $this->getMockBuilder(Write::class)
151145
->disableOriginalConstructor()
152146
->getMock();
147+
$this->fileSystemMock->expects($this->once())
148+
->method('getDirectoryWrite')
149+
->willReturn($this->mediaWriteDirectoryMock);
150+
153151
$this->driverMock = $this->getMockBuilder(Driver::class)
154152
->disableOriginalConstructor()
155153
->getMock();
156-
$this->writeFactoryMock->expects($this->any())
157-
->method('create')
158-
->willReturn($this->mediaEntityTmpDirectoryMock);
159-
$this->mediaEntityTmpDirectoryMock->expects($this->any())
154+
$this->mediaWriteDirectoryMock->expects($this->any())
160155
->method('getDriver')
161156
->willReturn($this->driverMock);
162157
}
@@ -184,9 +179,7 @@ private function initialize(array $data): Image
184179
$this->uploaderFactoryMock,
185180
$this->fileProcessorFactoryMock,
186181
$this->imageContentFactory,
187-
$this->ioFileSystemMock,
188-
$this->directoryListMock,
189-
$this->writeFactoryMock
182+
$this->ioFileSystemMock
190183
);
191184
}
192185

@@ -470,10 +463,10 @@ public function testCompactValueUiComponentAddress()
470463
->method('getRealPathSafety')
471464
->with($value['file'])
472465
->willReturn($value['file']);
473-
$this->mediaEntityTmpDirectoryMock->expects($this->once())
466+
$this->readDirectoryMock->expects($this->once())
474467
->method('getAbsolutePath')
475468
->willReturn($value['file']);
476-
$this->mediaEntityTmpDirectoryMock->expects($this->once())
469+
$this->readDirectoryMock->expects($this->once())
477470
->method('getRelativePath')
478471
->willReturn($value['file']);
479472
$this->fileProcessorMock->expects($this->once())
@@ -505,7 +498,7 @@ public function testCompactValueUiComponentCustomer()
505498

506499
$base64EncodedData = 'encoded_data';
507500

508-
$this->mediaEntityTmpDirectoryMock->expects($this->once())
501+
$this->readDirectoryMock->expects($this->once())
509502
->method('isExist')
510503
->with($value['file'])
511504
->willReturn(true);
@@ -561,7 +554,7 @@ public function testCompactValueUiComponentCustomerNotExists()
561554
'type' => 'image',
562555
];
563556

564-
$this->mediaEntityTmpDirectoryMock->expects($this->once())
557+
$this->readDirectoryMock->expects($this->once())
565558
->method('isExist')
566559
->with($value['file'])
567560
->willReturn(false);

‎dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/Product/Save/ImagesTest.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,10 @@ private function assertSuccessfulImageSave(array $expectation): void
138138
$this->assertEquals($expectation['small_image'], $product->getData('small_image'));
139139
$this->assertEquals($expectation['thumbnail'], $product->getData('thumbnail'));
140140
$this->assertEquals($expectation['swatch_image'], $product->getData('swatch_image'));
141-
$this->assertFileExists(
142-
$this->mediaDirectory->getAbsolutePath($this->config->getBaseMediaPath() . $expectation['image'])
141+
$this->assertTrue(
142+
$this->mediaDirectory->isExist(
143+
$this->mediaDirectory->getAbsolutePath($this->config->getBaseMediaPath() . $expectation['image'])
144+
)
143145
);
144146
}
145147
}

‎dev/tests/integration/testsuite/Magento/ConfigurableProduct/Controller/Adminhtml/Product/Initialization/HelperTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -315,8 +315,8 @@ private function assertChildProductImages(array $expectedImages): void
315315
foreach ($roles as $role) {
316316
$this->assertEquals($image, $simpleProduct->getData($role));
317317
}
318-
$this->assertFileExists(
319-
$this->mediaDirectory->getAbsolutePath($this->config->getBaseMediaPath() . $image)
318+
$this->assertTrue(
319+
$this->mediaDirectory->isExist($this->config->getBaseMediaPath() . $image)
320320
);
321321
}
322322
}

0 commit comments

Comments
 (0)