Skip to content

Commit a21ab6a

Browse files
committed
MAGETWO-95644: Import error
1 parent 991d785 commit a21ab6a

File tree

6 files changed

+106
-33
lines changed

6 files changed

+106
-33
lines changed

app/code/Magento/ImportExport/Controller/Adminhtml/Import/Validate.php

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,7 @@ public function execute()
4242
/** @var $import \Magento\ImportExport\Model\Import */
4343
$import = $this->getImport()->setData($data);
4444
try {
45-
$source = ImportAdapter::findAdapterFor(
46-
$import->uploadSource(),
47-
$this->_objectManager->create(\Magento\Framework\Filesystem::class)
48-
->getDirectoryWrite(DirectoryList::ROOT),
49-
$data[$import::FIELD_FIELD_SEPARATOR]
50-
);
45+
$source = $import->uploadFileAndGetSource();
5146
$this->processValidationResult($import->validateSource($source), $resultBlock);
5247
} catch (\Magento\Framework\Exception\LocalizedException $e) {
5348
$resultBlock->addError($e->getMessage());

app/code/Magento/ImportExport/Model/Import.php

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
use Magento\Framework\App\Filesystem\DirectoryList;
1212
use Magento\Framework\HTTP\Adapter\FileTransferFactory;
13+
use Magento\Framework\Stdlib\DateTime\DateTime;
1314
use Magento\ImportExport\Model\Import\ErrorProcessing\ProcessingError;
1415
use Magento\ImportExport\Model\Import\ErrorProcessing\ProcessingErrorAggregatorInterface;
1516

@@ -79,6 +80,11 @@ class Import extends \Magento\ImportExport\Model\AbstractModel
7980
*/
8081
const FIELD_FIELD_MULTIPLE_VALUE_SEPARATOR = '_import_multiple_value_separator';
8182

83+
/**
84+
* Import empty attribute value constant.
85+
*/
86+
const FIELD_EMPTY_ATTRIBUTE_VALUE_CONSTANT = '_import_empty_attribute_value_constant';
87+
8288
/**
8389
* Allow multiple values wrapping in double quotes for additional attributes.
8490
*/
@@ -91,6 +97,11 @@ class Import extends \Magento\ImportExport\Model\AbstractModel
9197
*/
9298
const DEFAULT_GLOBAL_MULTI_VALUE_SEPARATOR = ',';
9399

100+
/**
101+
* default empty attribute value constant
102+
*/
103+
const DEFAULT_EMPTY_ATTRIBUTE_VALUE_CONSTANT = '__EMPTY__VALUE__';
104+
94105
/**#@+
95106
* Import constants
96107
*/
@@ -159,6 +170,16 @@ class Import extends \Magento\ImportExport\Model\AbstractModel
159170
*/
160171
protected $_filesystem;
161172

173+
/**
174+
* @var History
175+
*/
176+
private $importHistoryModel;
177+
178+
/**
179+
* @var DateTime
180+
*/
181+
private $localeDate;
182+
162183
/**
163184
* @param \Psr\Log\LoggerInterface $logger
164185
* @param \Magento\Framework\Filesystem $filesystem
@@ -173,7 +194,7 @@ class Import extends \Magento\ImportExport\Model\AbstractModel
173194
* @param Source\Import\Behavior\Factory $behaviorFactory
174195
* @param \Magento\Framework\Indexer\IndexerRegistry $indexerRegistry
175196
* @param History $importHistoryModel
176-
* @param \Magento\Framework\Stdlib\DateTime\DateTime
197+
* @param DateTime $localeDate
177198
* @param array $data
178199
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
179200
*/
@@ -191,7 +212,7 @@ public function __construct(
191212
\Magento\ImportExport\Model\Source\Import\Behavior\Factory $behaviorFactory,
192213
\Magento\Framework\Indexer\IndexerRegistry $indexerRegistry,
193214
\Magento\ImportExport\Model\History $importHistoryModel,
194-
\Magento\Framework\Stdlib\DateTime\DateTime $localeDate,
215+
DateTime $localeDate,
195216
array $data = []
196217
) {
197218
$this->_importExportData = $importExportData;
@@ -234,7 +255,9 @@ protected function _getEntityAdapter()
234255
) {
235256
throw new \Magento\Framework\Exception\LocalizedException(
236257
__(
237-
'The entity adapter object must be an instance of %1 or %2.', \Magento\ImportExport\Model\Import\Entity\AbstractEntity::class, \Magento\ImportExport\Model\Import\AbstractEntity::class
258+
'The entity adapter object must be an instance of %1 or %2.',
259+
\Magento\ImportExport\Model\Import\Entity\AbstractEntity::class,
260+
\Magento\ImportExport\Model\Import\AbstractEntity::class
238261
)
239262
);
240263
}
@@ -513,14 +536,27 @@ public function uploadSource()
513536
}
514537
$this->_removeBom($sourceFile);
515538
$this->createHistoryReport($sourceFileRelative, $entity, $extension, $result);
516-
// trying to create source adapter for file and catch possible exception to be convinced in its adequacy
539+
540+
return $sourceFile;
541+
}
542+
543+
/**
544+
* Move uploaded file and provide source instance.
545+
*
546+
* @return Import\AbstractSource
547+
* @throws \Magento\Framework\Exception\LocalizedException
548+
*/
549+
public function uploadFileAndGetSource()
550+
{
551+
$sourceFile = $this->uploadSource();
517552
try {
518-
$this->_getSourceAdapter($sourceFile);
553+
$source = $this->_getSourceAdapter($sourceFile);
519554
} catch (\Exception $e) {
520-
$this->_varDirectory->delete($sourceFileRelative);
555+
$this->_varDirectory->delete($this->_varDirectory->getRelativePath($sourceFile));
521556
throw new \Magento\Framework\Exception\LocalizedException(__($e->getMessage()));
522557
}
523-
return $sourceFile;
558+
559+
return $source;
524560
}
525561

526562
/**
@@ -684,7 +720,9 @@ public function isReportEntityType($entity = null)
684720
try {
685721
$result = $this->_getEntityAdapter()->isNeedToLogInHistory();
686722
} catch (\Exception $e) {
687-
throw new \Magento\Framework\Exception\LocalizedException(__('Please enter a correct entity model'));
723+
throw new \Magento\Framework\Exception\LocalizedException(
724+
__('Please enter a correct entity model')
725+
);
688726
}
689727
} else {
690728
throw new \Magento\Framework\Exception\LocalizedException(__('Please enter a correct entity model'));
@@ -696,11 +734,11 @@ public function isReportEntityType($entity = null)
696734
}
697735

698736
/**
699-
* Create history report
737+
* Create history report.
700738
*
739+
* @param string $sourceFileRelative
701740
* @param string $entity
702741
* @param string $extension
703-
* @param string $sourceFileRelative
704742
* @param array $result
705743
* @return $this
706744
* @throws \Magento\Framework\Exception\LocalizedException
@@ -713,7 +751,7 @@ protected function createHistoryReport($sourceFileRelative, $entity, $extension
713751
$sourceFileRelative = $this->_varDirectory->getRelativePath(self::IMPORT_DIR . $fileName);
714752
} elseif (isset($result['name'])) {
715753
$fileName = $result['name'];
716-
} elseif (!is_null($extension)) {
754+
} elseif ($extension !== null) {
717755
$fileName = $entity . $extension;
718756
} else {
719757
$fileName = basename($sourceFileRelative);

app/code/Magento/ImportExport/Model/Import/Source/Zip.php

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
*/
66
namespace Magento\ImportExport\Model\Import\Source;
77

8+
use Magento\Framework\Exception\ValidatorException;
9+
810
/**
911
* Zip import adapter.
1012
*/
@@ -14,17 +16,22 @@ class Zip extends Csv
1416
* @param string $file
1517
* @param \Magento\Framework\Filesystem\Directory\Write $directory
1618
* @param string $options
19+
* @throws \Magento\Framework\Exception\ValidatorException
1720
*/
1821
public function __construct(
1922
$file,
2023
\Magento\Framework\Filesystem\Directory\Write $directory,
2124
$options
2225
) {
2326
$zip = new \Magento\Framework\Archive\Zip();
24-
$file = $zip->unpack(
25-
$directory->getRelativePath($file),
26-
$directory->getRelativePath(preg_replace('/\.zip$/i', '.csv', $file))
27+
$csvFile = $zip->unpack(
28+
$file,
29+
preg_replace('/\.zip$/i', '.csv', $file)
2730
);
28-
parent::__construct($file, $directory, $options);
31+
if (!$csvFile) {
32+
throw new ValidatorException(__('Sorry, but the data is invalid or the file is not uploaded.'));
33+
}
34+
$directory->delete($directory->getRelativePath($file));
35+
parent::__construct($csvFile, $directory, $options);
2936
}
3037
}

dev/tests/integration/testsuite/Magento/ImportExport/Controller/Adminhtml/Import/ValidateTest.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,14 @@ class ValidateTest extends \Magento\TestFramework\TestCase\AbstractBackendContro
1717
/**
1818
* @dataProvider validationDataProvider
1919
* @param string $fileName
20+
* @param string $mimeType
2021
* @param string $message
2122
* @param string $delimiter
2223
* @backupGlobals enabled
2324
* @magentoDbIsolation enabled
25+
* @SuppressWarnings(PHPMD.Superglobals)
2426
*/
25-
public function testValidationReturn($fileName, $message, $delimiter)
27+
public function testValidationReturn(string $fileName, string $mimeType, string $message, string $delimiter)
2628
{
2729
$validationStrategy = ProcessingErrorAggregatorInterface::VALIDATION_STRATEGY_STOP_ON_ERROR;
2830

@@ -50,7 +52,7 @@ public function testValidationReturn($fileName, $message, $delimiter)
5052
$_FILES = [
5153
'import_file' => [
5254
'name' => $fileName,
53-
'type' => 'text/csv',
55+
'type' => $mimeType,
5456
'tmp_name' => $target,
5557
'error' => 0,
5658
'size' => filesize($target)
@@ -84,24 +86,34 @@ public function validationDataProvider()
8486
return [
8587
[
8688
'file_name' => 'catalog_product.csv',
89+
'mime-type' => 'text/csv',
8790
'message' => 'File is valid',
8891
'delimiter' => ',',
8992
],
9093
[
9194
'file_name' => 'test.txt',
95+
'mime-type' => 'text/csv',
9296
'message' => '\'txt\' file extension is not supported',
9397
'delimiter' => ',',
9498
],
9599
[
96100
'file_name' => 'incorrect_catalog_product_comma.csv',
101+
'mime-type' => 'text/csv',
97102
'message' => 'Download full report',
98103
'delimiter' => ',',
99104
],
100105
[
101106
'file_name' => 'incorrect_catalog_product_semicolon.csv',
107+
'mime-type' => 'text/csv',
102108
'message' => 'Download full report',
103109
'delimiter' => ';',
104110
],
111+
[
112+
'file_name' => 'catalog_product.zip',
113+
'mime-type' => 'application/zip',
114+
'message' => 'File is valid',
115+
'delimiter' => ',',
116+
],
105117
];
106118
}
107119
}

lib/internal/Magento/Framework/Archive/Zip.php

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,11 @@
44
* See COPYING.txt for license details.
55
*/
66

7-
/**
8-
* Class to work with zip archives
9-
*
10-
* @author Magento Core Team <core@magentocommerce.com>
11-
*/
127
namespace Magento\Framework\Archive;
138

9+
/**
10+
* Zip compressed file archive.
11+
*/
1412
class Zip extends AbstractArchive implements ArchiveInterface
1513
{
1614
/**
@@ -54,11 +52,34 @@ public function pack($source, $destination)
5452
public function unpack($source, $destination)
5553
{
5654
$zip = new \ZipArchive();
57-
$zip->open($source);
58-
$filename = $zip->getNameIndex(0);
59-
$zip->extractTo(dirname($destination), $filename);
60-
rename(dirname($destination).'/'.$filename, $destination);
61-
$zip->close();
55+
if ($zip->open($source) === true) {
56+
$filename = $this->filterRelativePaths($zip->getNameIndex(0) ?: '');
57+
if ($filename) {
58+
$zip->extractTo(dirname($destination), $filename);
59+
rename(dirname($destination).'/'.$filename, $destination);
60+
} else {
61+
$destination = '';
62+
}
63+
$zip->close();
64+
} else {
65+
$destination = '';
66+
}
67+
6268
return $destination;
6369
}
70+
71+
/**
72+
* Filter file names with relative paths.
73+
*
74+
* @param string $path
75+
* @return string
76+
*/
77+
private function filterRelativePaths(string $path)
78+
{
79+
if ($path && preg_match('#^\s*(../)|(/../)#i', $path)) {
80+
$path = '';
81+
}
82+
83+
return $path;
84+
}
6485
}

0 commit comments

Comments
 (0)