MAGETWO-67321: Importing Products with images from external URL

Author: oshmyheliuk

app/code/Magento/CatalogImportExport/Model/Import/Product/Validator/Media.php

@@ -6,15 +6,36 @@
 namespace Magento\CatalogImportExport\Model\Import\Product\Validator;
 
 use Magento\CatalogImportExport\Model\Import\Product\RowValidatorInterface;
+use Magento\Framework\App\ObjectManager;
+use Magento\Framework\Url\Validator;
 
 class Media extends AbstractImportValidator implements RowValidatorInterface
 {
+    /**
+     * @deprecated As this regexp doesn't give guarantee of correct url validation
+     * @see \Magento\Framework\Url\Validator::isValid()
+     */
     const URL_REGEXP = '|^http(s)?://[a-z0-9-]+(.[a-z0-9-]+)*(:[0-9]+)?(/.*)?$|i';
 
     const PATH_REGEXP = '#^(?!.*[\\/]\.{2}[\\/])(?!\.{2}[\\/])[-\w.\\/]+$#';
 
     const ADDITIONAL_IMAGES = 'additional_images';
 
+    /**
+     * The url validator. Checks if given url is valid.
+     *
+     * @var Validator
+     */
+    private $validator;
+
+    /**
+     * @param Validator $validator The url validator
+     */
+    public function __construct(Validator $validator = null)
+    {
+        $this->validator = $validator ?: ObjectManager::getInstance()->get(Validator::class);
+    }
+
     /**
      * @deprecated
      * @see \Magento\CatalogImportExport\Model\Import\Product::getMultipleValueSeparator()
@@ -27,6 +48,8 @@ class Media extends AbstractImportValidator implements RowValidatorInterface
     /**
      * @param string $string
      * @return bool
+     * @deprecated As this method doesn't give guarantee of correct url validation.
+     * @see \Magento\Framework\Url\Validator::isValid() It provides better url validation.
      */
     protected function checkValidUrl($string)
     {
@@ -64,7 +87,7 @@ public function isValid($value)
         $valid = true;
         foreach ($this->mediaAttributes as $attribute) {
             if (isset($value[$attribute]) && strlen($value[$attribute])) {
-                if (!$this->checkPath($value[$attribute]) && !$this->checkValidUrl($value[$attribute])) {
+                if (!$this->checkPath($value[$attribute]) && !$this->validator->isValid($value[$attribute])) {
                     $this->_addMessages(
                         [
                             sprintf(
@@ -79,7 +102,7 @@ public function isValid($value)
         }
         if (isset($value[self::ADDITIONAL_IMAGES]) && strlen($value[self::ADDITIONAL_IMAGES])) {
             foreach (explode($this->context->getMultipleValueSeparator(), $value[self::ADDITIONAL_IMAGES]) as $image) {
-                if (!$this->checkPath($image) && !$this->checkValidUrl($image)) {
+                if (!$this->checkPath($image) && !$this->validator->isValid($image)) {
                     $this->_addMessages(
                         [
                             sprintf(

app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/Validator/MediaTest.php

@@ -10,6 +10,8 @@
 use Magento\CatalogImportExport\Model\Import\Product\Validator\Media;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
 use Magento\ImportExport\Model\Import;
+use Magento\Framework\Url\Validator;
+use PHPUnit_Framework_MockObject_MockObject as MockObject;
 
 class MediaTest extends \PHPUnit_Framework_TestCase
 {
@@ -19,14 +21,22 @@ class MediaTest extends \PHPUnit_Framework_TestCase
     /** @var ObjectManagerHelper */
     protected $objectManagerHelper;
 
+    /**
+     * @var Validator|MockObject
+     */
+    private $validatorMock;
+
     protected function setUp()
     {
-        
+        $this->validatorMock = $this->getMockBuilder(Validator::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
         $this->objectManagerHelper = new ObjectManagerHelper($this);
         $this->media = $this->objectManagerHelper->getObject(
             Media::class,
             [
-                
+                'validator' => $this->validatorMock
             ]
         );
     }
@@ -40,10 +50,15 @@ public function testInit()
     /**
      * @param array $data
      * @param array $expected
+     * @param \Closure|null $validatorCallback
      * @dataProvider isMediaValidDataProvider
      */
-    public function testIsValid($data, $expected)
+    public function testIsValid($data, $expected, \Closure $validatorCallback = null)
     {
+        if ($validatorCallback !== null) {
+            $validatorCallback($this->validatorMock);
+        }
+
         $contextMock = $this->getMockBuilder(Product::class)
             ->disableOriginalConstructor()
             ->getMock();
@@ -101,7 +116,27 @@ public function isMediaValidDataProvider()
             'additional_images_fail' => [
                 ['additional_images' => 'image1.png|image2.jpg|image3.gif'],
                 ['result' => false, 'messages' => [0 => 'additional_images']]
-            ]
+            ],
+            'additional_images_wrong_domain' => [
+                ['additional_images' => 'https://example/images/some-name.jpg'],
+                ['result' => false, 'messages' => [0 => 'additional_images']],
+                function ($validatorMock) {
+                    $validatorMock->expects($this->once())
+                        ->method('isValid')
+                        ->with('https://example/images/some-name.jpg')
+                        ->willReturn(false);
+                }
+            ],
+            'additional_images_url_multiple_underscores' => [
+                ['additional_images' => 'https://example.com/images/some-name__with___multiple____underscores.jpg'],
+                ['result' => true, 'messages' => []],
+                function ($validatorMock) {
+                    $validatorMock->expects($this->once())
+                        ->method('isValid')
+                        ->with('https://example.com/images/some-name__with___multiple____underscores.jpg')
+                        ->willReturn(true);
+                }
+            ],
         ];
     }
 }