From 2b3ff84829c697017a9784e84e444c8e429aad9d Mon Sep 17 00:00:00 2001
From: Jeremy Benoist <jeremy.benoist@gmail.com>
Date: Thu, 3 Feb 2022 13:29:40 +0100
Subject: [PATCH] Avoid overlapping images when downloading them

---
 composer.json                                 |  5 ++++-
 .../CoreBundle/Helper/DownloadImages.php      |  9 ++++++---
 .../CoreBundle/Helper/DownloadImagesTest.php  | 20 ++++++++++++++++++-
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/composer.json b/composer.json
index 0f7bc07f8..36ae0bc2d 100644
--- a/composer.json
+++ b/composer.json
@@ -164,7 +164,10 @@
         "platform": {
             "php": "7.2.5"
         },
-        "sort-packages": true
+        "sort-packages": true,
+        "allow-plugins": {
+            "phpstan/extension-installer": true
+        }
     },
     "minimum-stability": "dev",
     "prefer-stable": true
diff --git a/src/Wallabag/CoreBundle/Helper/DownloadImages.php b/src/Wallabag/CoreBundle/Helper/DownloadImages.php
index 018bc2c6c..30f508829 100644
--- a/src/Wallabag/CoreBundle/Helper/DownloadImages.php
+++ b/src/Wallabag/CoreBundle/Helper/DownloadImages.php
@@ -72,13 +72,16 @@ class DownloadImages
     {
         $imagesUrls = self::extractImagesUrlsFromHtml($html);
 
+        // ensure images aren't overlapping
+        arsort($imagesUrls);
+
         $relativePath = $this->getRelativePath($entryId);
 
         // download and save the image to the folder
         foreach ($imagesUrls as $image) {
-            $imagePath = $this->processSingleImage($entryId, $image, $url, $relativePath);
+            $newImage = $this->processSingleImage($entryId, $image, $url, $relativePath);
 
-            if (false === $imagePath) {
+            if (false === $newImage) {
                 continue;
             }
 
@@ -87,7 +90,7 @@ class DownloadImages
                 $image = str_replace('&', '&amp;', $image);
             }
 
-            $html = str_replace($image, $imagePath, $html);
+            $html = str_replace($image, $newImage, $html);
         }
 
         return $html;
diff --git a/tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php b/tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php
index 63041254f..654246e10 100644
--- a/tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php
+++ b/tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php
@@ -31,7 +31,6 @@ class DownloadImagesTest extends TestCase
     public function testProcessHtml($html, $url)
     {
         $httpMockClient = new HttpMockClient();
-
         $httpMockClient->addResponse(new Response(200, ['content-type' => 'image/png'], file_get_contents(__DIR__ . '/../fixtures/unnamed.png')));
 
         $logHandler = new TestHandler();
@@ -201,4 +200,23 @@ class DownloadImagesTest extends TestCase
         );
         $this->assertFalse($res);
     }
+
+    public function testEnsureOnlyFirstOccurenceIsReplaced()
+    {
+        $httpMockClient = new HttpMockClient();
+        $httpMockClient->addResponse(new Response(200, ['content-type' => 'image/png'], file_get_contents(__DIR__ . '/../fixtures/unnamed.png')));
+        $httpMockClient->addResponse(new Response(200, ['content-type' => 'image/png'], file_get_contents(__DIR__ . '/../fixtures/unnamed.png')));
+
+        $logHandler = new TestHandler();
+        $logger = new Logger('test', [$logHandler]);
+
+        $download = new DownloadImages($httpMockClient, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger);
+
+        $html = '<img src="https://images.wsj.net/im-410981?width=860&height=573" srcset="https://images.wsj.net/im-410981?width=860&height=573&pixel_ratio=1.5 1290w" height="573" width="860" alt="" referrerpolicy="no-referrer">';
+        $url = 'https://www.wsj.com/articles/5-interior-design-tips-to-max-out-your-basement-space-11633435201';
+
+        $res = $download->processHtml(123, $html, $url);
+
+        $this->assertSame('<img src="http://wallabag.io/assets/images/9/b/9b0ead26/6bef06fe.png" srcset="http://wallabag.io/assets/images/9/b/9b0ead26/43cc0123.png 1290w" height="573" width="860" alt="" referrerpolicy="no-referrer">', $res);
+    }
 }