Skip to content

Commit bc50d91

Browse files
author
Hwashiang Yu
committed
Merge remote-tracking branch 'upstream/2.1.18-develop' into MAGETWO-98341
2 parents 69b67fb + e29f4c5 commit bc50d91

File tree

20 files changed

+883
-188
lines changed

20 files changed

+883
-188
lines changed

app/code/Magento/CatalogImportExport/Model/Import/Uploader.php

Lines changed: 61 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ class Uploader extends \Magento\MediaStorage\Model\File\Uploader
3131
*/
3232
protected $_tmpDir = '';
3333

34+
/**
35+
* Download directory for url-based resources.
36+
*
37+
* @var string
38+
*/
39+
private $downloadDir;
40+
3441
/**
3542
* Destination directory.
3643
*
@@ -99,6 +106,13 @@ class Uploader extends \Magento\MediaStorage\Model\File\Uploader
99106
*/
100107
private $directoryResolver;
101108

109+
/**
110+
* Instance of random data generator.
111+
*
112+
* @var \Magento\Framework\Math\Random
113+
*/
114+
private $random;
115+
102116
/**
103117
* @param \Magento\MediaStorage\Helper\File\Storage\Database $coreFileStorageDb
104118
* @param \Magento\MediaStorage\Helper\File\Storage $coreFileStorage
@@ -108,6 +122,8 @@ class Uploader extends \Magento\MediaStorage\Model\File\Uploader
108122
* @param \Magento\Framework\Filesystem\File\ReadFactory $readFactory
109123
* @param string|null $filePath
110124
* @param \Magento\Framework\App\Filesystem\DirectoryResolver|null $directoryResolver
125+
* @param \Magento\Framework\Math\Random|null $random
126+
* @throws \Magento\Framework\Exception\FileSystemException
111127
* @throws \Magento\Framework\Exception\LocalizedException
112128
*/
113129
public function __construct(
@@ -118,7 +134,8 @@ public function __construct(
118134
\Magento\Framework\Filesystem $filesystem,
119135
\Magento\Framework\Filesystem\File\ReadFactory $readFactory,
120136
$filePath = null,
121-
\Magento\Framework\App\Filesystem\DirectoryResolver $directoryResolver = null
137+
\Magento\Framework\App\Filesystem\DirectoryResolver $directoryResolver = null,
138+
\Magento\Framework\Math\Random $random = null
122139
) {
123140
if ($filePath !== null) {
124141
$this->_setUploadFile($filePath);
@@ -131,6 +148,9 @@ public function __construct(
131148
$this->_readFactory = $readFactory;
132149
$this->directoryResolver = $directoryResolver
133150
?: ObjectManager::getInstance()->get(\Magento\Framework\App\Filesystem\DirectoryResolver::class);
151+
$this->random = $random
152+
?: ObjectManager::getInstance()->get(\Magento\Framework\Math\Random::class);
153+
$this->downloadDir = DirectoryList::getDefaultConfig()[DirectoryList::TMP][DirectoryList::PATH];
134154
}
135155

136156
/**
@@ -159,42 +179,58 @@ public function init()
159179
*/
160180
public function move($fileName, $renameFileOff = false)
161181
{
162-
if ($renameFileOff) {
163-
$this->setAllowRenameFiles(false);
164-
}
165-
166-
if ($this->getTmpDir()) {
167-
$filePath = $this->getTmpDir() . '/';
168-
} else {
169-
$filePath = '';
170-
}
182+
$this->setAllowRenameFiles(!$renameFileOff);
171183

172184
if (preg_match('/\bhttps?:\/\//i', $fileName, $matches)) {
173185
$url = str_replace($matches[0], '', $fileName);
174-
$driver = $matches[0] === $this->httpScheme ? DriverPool::HTTP : DriverPool::HTTPS;
175-
$read = $this->_readFactory->create($url, $driver);
176-
177-
$fileExtension = pathinfo($fileName, PATHINFO_EXTENSION);
178-
if ($fileExtension && !$this->checkAllowedExtension($fileExtension)) {
179-
throw new \Magento\Framework\Exception\LocalizedException(__('Disallowed file type.'));
180-
}
181-
182-
$fileName = preg_replace('/[^a-z0-9\._-]+/i', '', $fileName);
183-
$this->_directory->writeFile(
184-
$this->_directory->getRelativePath($filePath . $fileName),
185-
$read->readAll()
186-
);
186+
$driver = ($matches[0] === $this->httpScheme) ? DriverPool::HTTP : DriverPool::HTTPS;
187+
$tmpFilePath = $this->downloadFileFromUrl($url, $driver);
188+
} else {
189+
$tmpDir = $this->getTmpDir() ? ($this->getTmpDir() . '/') : '';
190+
$tmpFilePath = $this->_directory->getRelativePath($tmpDir . $fileName);
187191
}
188192

189-
$filePath = $this->_directory->getRelativePath($filePath . $fileName);
190-
$this->_setUploadFile($filePath);
193+
$this->_setUploadFile($tmpFilePath);
191194
$destDir = $this->_directory->getAbsolutePath($this->getDestDir());
192195
$result = $this->save($destDir);
193196
unset($result['path']);
194197
$result['name'] = self::getCorrectFileName($result['name']);
198+
195199
return $result;
196200
}
197201

202+
/**
203+
* Writes a url-based file to the temp directory.
204+
*
205+
* @param string $url
206+
* @param string $driver
207+
* @return string
208+
* @throws \Magento\Framework\Exception\FileSystemException
209+
* @throws \Magento\Framework\Exception\LocalizedException
210+
*/
211+
private function downloadFileFromUrl($url, $driver)
212+
{
213+
$parsedUrlPath = parse_url($url, PHP_URL_PATH);
214+
if (!$parsedUrlPath) {
215+
throw new \Magento\Framework\Exception\LocalizedException(__('Could not parse resource url.'));
216+
}
217+
$urlPathValues = explode('/', $parsedUrlPath);
218+
$fileName = preg_replace('/[^a-z0-9\._-]+/i', '', end($urlPathValues));
219+
$fileExtension = pathinfo($fileName, PATHINFO_EXTENSION);
220+
if ($fileExtension && !$this->checkAllowedExtension($fileExtension)) {
221+
throw new \Magento\Framework\Exception\LocalizedException(__('Disallowed file type.'));
222+
}
223+
$tmpFileName = str_replace(".$fileExtension", '', $fileName);
224+
$tmpFileName .= '_' . $this->random->getRandomString(16);
225+
$tmpFileName .= $fileExtension ? ".$fileExtension" : '';
226+
$tmpFilePath = $this->_directory->getRelativePath($this->downloadDir . '/' . $tmpFileName);
227+
$this->_directory->writeFile(
228+
$tmpFilePath,
229+
$this->_readFactory->create($url, $driver)->readAll()
230+
);
231+
return $tmpFilePath;
232+
}
233+
198234
/**
199235
* Prepare information about the file for moving
200236
*
@@ -250,7 +286,6 @@ protected function _validateFile()
250286

251287
$fileExtension = pathinfo($filePath, PATHINFO_EXTENSION);
252288
if (!$this->checkAllowedExtension($fileExtension)) {
253-
$this->_directory->delete($filePath);
254289
throw new \Exception('Disallowed file type.');
255290
}
256291
//run validate callbacks

app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/UploaderTest.php

Lines changed: 113 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ class UploaderTest extends \PHPUnit_Framework_TestCase
4848
*/
4949
private $directoryResolver;
5050

51+
/**
52+
* @var \Magento\Framework\Math\Random|\PHPUnit_Framework_MockObject_MockObject
53+
*/
54+
private $random;
55+
5156
/**
5257
* @var \Magento\CatalogImportExport\Model\Import\Uploader|\PHPUnit_Framework_MockObject_MockObject
5358
*/
@@ -94,6 +99,11 @@ protected function setUp()
9499
->setMethods(['validatePath'])
95100
->getMock();
96101

102+
$this->random = $this->getMockBuilder(\Magento\Framework\Math\Random::class)
103+
->disableOriginalConstructor()
104+
->setMethods(['getRandomString'])
105+
->getMock();
106+
97107
$this->uploader = $this->getMockBuilder(\Magento\CatalogImportExport\Model\Import\Uploader::class)
98108
->setConstructorArgs([
99109
$this->coreFileStorageDb,
@@ -104,49 +114,75 @@ protected function setUp()
104114
$this->readFactory,
105115
null,
106116
$this->directoryResolver,
117+
$this->random
107118
])
108119
->setMethods(['_setUploadFile', 'save', 'getTmpDir', 'checkAllowedExtension'])
109120
->getMock();
110121
}
111122

112123
/**
113124
* @dataProvider moveFileUrlDataProvider
125+
* @param $fileUrl
126+
* @param $expectedHost
127+
* @param $expectedFileName
128+
* @param $checkAllowedExtension
129+
* @throws \Magento\Framework\Exception\LocalizedException
114130
*/
115131
public function testMoveFileUrl($fileUrl, $expectedHost, $expectedFileName, $checkAllowedExtension)
116132
{
133+
$tmpDir = 'var/tmp';
117134
$destDir = 'var/dest/dir';
118-
$expectedRelativeFilePath = $expectedFileName;
119-
$this->directoryMock->expects($this->once())->method('isWritable')->with($destDir)->willReturn(true);
120-
$this->directoryMock->expects($this->any())->method('getRelativePath')->with($expectedRelativeFilePath);
121-
$this->directoryMock->expects($this->once())->method('getAbsolutePath')->with($destDir)
122-
->willReturn($destDir . '/' . $expectedFileName);
123-
// Check writeFile() method invoking.
124-
$this->directoryMock->expects($this->any())->method('writeFile')->will($this->returnValue($expectedFileName));
135+
136+
// Expected invocation to validate file extension
137+
$this->uploader->expects($this->exactly($checkAllowedExtension))->method('checkAllowedExtension')
138+
->willReturn(true);
139+
140+
// Expected invocation to generate random string for file name postfix
141+
$this->random->expects($this->once())->method('getRandomString')
142+
->with(16)
143+
->willReturn('38GcEmPFKXXR8NMj');
144+
145+
// Expected invocation to build the temp file path with the correct directory and filename
146+
$this->directoryMock->expects($this->any())->method('getRelativePath')
147+
->with($tmpDir . '/' . $expectedFileName);
125148

126149
// Create adjusted reader which does not validate path.
127150
$readMock = $this->getMockBuilder(\Magento\Framework\Filesystem\File\Read::class)
128151
->disableOriginalConstructor()
129152
->setMethods(['readAll'])
130153
->getMock();
131-
// Check readAll() method invoking.
132-
$readMock->expects($this->once())->method('readAll')->will($this->returnValue(null));
133154

134-
// Check create() method invoking with expected argument.
135-
$this->readFactory->expects($this->once())
136-
->method('create')
137-
->will($this->returnValue($readMock))->with($expectedHost);
138-
//Check invoking of getTmpDir(), _setUploadFile(), save() methods.
139-
$this->uploader->expects($this->any())->method('getTmpDir')->will($this->returnValue(''));
140-
$this->uploader->expects($this->once())->method('_setUploadFile')->will($this->returnSelf());
141-
$this->uploader->expects($this->once())->method('save')->with($destDir . '/' . $expectedFileName)
142-
->willReturn(['name' => $expectedFileName, 'path' => 'absPath']);
143-
$this->uploader->expects($this->exactly($checkAllowedExtension))
144-
->method('checkAllowedExtension')
155+
// Expected invocations to create reader and read contents from url
156+
$this->readFactory->expects($this->once())->method('create')
157+
->with($expectedHost)
158+
->will($this->returnValue($readMock));
159+
$readMock->expects($this->once())->method('readAll')
160+
->will($this->returnValue(null));
161+
162+
// Expected invocation to write the temp file
163+
$this->directoryMock->expects($this->any())->method('writeFile')
164+
->will($this->returnValue($expectedFileName));
165+
166+
// Expected invocations to move the temp file to the destination directory
167+
$this->directoryMock->expects($this->once())->method('isWritable')
168+
->with($destDir)
145169
->willReturn(true);
170+
$this->directoryMock->expects($this->once())->method('getAbsolutePath')
171+
->with($destDir)
172+
->willReturn($destDir . '/' . $expectedFileName);
173+
$this->uploader->expects($this->once())->method('_setUploadFile')
174+
->willReturnSelf();
175+
$this->uploader->expects($this->once())->method('save')
176+
->with($destDir . '/' . $expectedFileName)
177+
->willReturn(['name' => $expectedFileName, 'path' => 'absPath']);
178+
179+
// Do not use configured temp directory
180+
$this->uploader->expects($this->never())->method('getTmpDir');
146181

147182
$this->uploader->setDestDir($destDir);
148183
$result = $this->uploader->move($fileUrl);
149184
$this->assertEquals(['name' => $expectedFileName], $result);
185+
150186
$this->assertArrayNotHasKey('path', $result);
151187
}
152188

@@ -175,18 +211,66 @@ public function testMoveFileName()
175211
public function moveFileUrlDataProvider()
176212
{
177213
return [
178-
[
179-
'$fileUrl' => 'http://test_uploader_file',
214+
'https_no_file_ext' => [
215+
'$fileUrl' => 'https://test_uploader_file',
180216
'$expectedHost' => 'test_uploader_file',
181-
'$expectedFileName' => 'httptest_uploader_file',
182-
'$checkAllowedExtension' => 0,
217+
'$expectedFileName' => 'test_uploader_file_38GcEmPFKXXR8NMj',
218+
'$checkAllowedExtension' => 0
219+
],
220+
'https_invalid_chars' => [
221+
'$fileUrl' => 'https://www.google.com/!:^&`;image.jpg',
222+
'$expectedHost' => 'www.google.com/!:^&`;image.jpg',
223+
'$expectedFileName' => 'image_38GcEmPFKXXR8NMj.jpg',
224+
'$checkAllowedExtension' => 1
225+
],
226+
'https_invalid_chars_no_file_ext' => [
227+
'$fileUrl' => 'https://!:^&`;image',
228+
'$expectedHost' => '!:^&`;image',
229+
'$expectedFileName' => 'image_38GcEmPFKXXR8NMj',
230+
'$checkAllowedExtension' => 0
231+
],
232+
'http_jpg' => [
233+
'$fileUrl' => 'http://www.google.com/image.jpg',
234+
'$expectedHost' => 'www.google.com/image.jpg',
235+
'$expectedFileName' => 'image_38GcEmPFKXXR8NMj.jpg',
236+
'$checkAllowedExtension' => 1
237+
],
238+
'https_jpg' => [
239+
'$fileUrl' => 'https://www.google.com/image.jpg',
240+
'$expectedHost' => 'www.google.com/image.jpg',
241+
'$expectedFileName' => 'image_38GcEmPFKXXR8NMj.jpg',
242+
'$checkAllowedExtension' => 1
243+
],
244+
'https_jpeg' => [
245+
'$fileUrl' => 'https://www.google.com/image.jpeg',
246+
'$expectedHost' => 'www.google.com/image.jpeg',
247+
'$expectedFileName' => 'image_38GcEmPFKXXR8NMj.jpeg',
248+
'$checkAllowedExtension' => 1
249+
],
250+
'https_png' => [
251+
'$fileUrl' => 'https://www.google.com/image.png',
252+
'$expectedHost' => 'www.google.com/image.png',
253+
'$expectedFileName' => 'image_38GcEmPFKXXR8NMj.png',
254+
'$checkAllowedExtension' => 1
255+
],
256+
'https_gif' => [
257+
'$fileUrl' => 'https://www.google.com/image.gif',
258+
'$expectedHost' => 'www.google.com/image.gif',
259+
'$expectedFileName' => 'image_38GcEmPFKXXR8NMj.gif',
260+
'$checkAllowedExtension' => 1
183261
],
184-
[
185-
'$fileUrl' => 'https://!:^&`;file',
186-
'$expectedHost' => '!:^&`;file',
187-
'$expectedFileName' => 'httpsfile',
188-
'$checkAllowedExtension' => 0,
262+
'https_one_query_param' => [
263+
'$fileUrl' => 'https://www.google.com/image.jpg?param=1',
264+
'$expectedHost' => 'www.google.com/image.jpg?param=1',
265+
'$expectedFileName' => 'image_38GcEmPFKXXR8NMj.jpg',
266+
'$checkAllowedExtension' => 1
189267
],
268+
'https_two_query_params' => [
269+
'$fileUrl' => 'https://www.google.com/image.jpg?param=1&param=2',
270+
'$expectedHost' => 'www.google.com/image.jpg?param=1&param=2',
271+
'$expectedFileName' => 'image_38GcEmPFKXXR8NMj.jpg',
272+
'$checkAllowedExtension' => 1
273+
]
190274
];
191275
}
192276

app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog/Save.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ public function execute()
7878
unset($data['rule']);
7979
}
8080

81+
unset($data['conditions_serialized']);
82+
unset($data['actions_serialized']);
83+
8184
$model->loadPost($data);
8285

8386
$this->_objectManager->get('Magento\Backend\Model\Session')->setPageData($data);

0 commit comments

Comments
 (0)