fix handling of + in image names, added some functional tests

.github/workflows/.editorconfig

@@ -0,0 +1,2 @@
+[*.yml]
+indent_size = 2

.github/workflows/php-cs-fixer.yml

@@ -3,6 +3,9 @@ name: PHP-CS-Fixer
 on:
   pull_request:
   push:
+    branches:
+      - 1.x
+      - 2.x
 
 jobs:
   php-cs-fixer:

.github/workflows/phpunit.yml

@@ -3,6 +3,9 @@ name: PHPUnit
 on:
   pull_request:
   push:
+    branches:
+      - 1.x
+      - 2.x
 
 jobs:
   phpunit:

CHANGELOG.md

@@ -95,6 +95,7 @@ for a given releases. Unreleased, upcoming changes will be updated here periodic
 - Fix special characters encoding in URL path ([dbalabka](https://github.com/dbalabka))
 - Update imagine/imagine dependency to 1.1 ([maximgubar](https://github.com/maximgubar))
 - Only use actual path without any query parameters from the url ([maximgubar](https://github.com/maximgubar))
+  See also UPGRADE.md - if you used URLs with domains to get your images, you will need to adjust.
 - __\[Dependency Injection\]__ Add aliases for data and filter manager ([fpaterno](https://github.com/fpaterno))
 - Use Autorotate Filter from Imagine library ([franmomu](https://github.com/franmomu))
 - Fix Mime deprecations for Symfony 4 ([franmomu](https://github.com/franmomu))

Imagine/Cache/Helper/PathHelper.php

@@ -20,12 +20,14 @@ class PathHelper
 {
     public static function filePathToUrlPath(string $path): string
     {
+        // this is not exactly the same as what symfony does in UrlGenerator::doGenerate where it strtr a bunch of encodings back to unencoded
+        // however, as we need to rawurldecode anyways, it should not hurt
         return implode('/', array_map('rawurlencode', explode('/', $path)));
     }
 
     public static function urlPathToFilePath(string $url): string
     {
-        // used urldecode instead of rawurlencode for BC safety to support "+" in URL
-        return urldecode($url);
+        // revert the rawurlencode that the Symfony UrlGenerator applies to paths
+        return rawurldecode($url);
     }
 }

Tests/Functional/Controller/ImagineControllerTest.php

@@ -49,24 +49,38 @@ public function testCouldBeGetFromContainer(): void
         $this->assertInstanceOf(ImagineController::class, self::$kernel->getContainer()->get(ImagineController::class));
     }
 
-    public function testShouldResolvePopulatingCacheFirst(): void
+    public function provideImageNames(): iterable
+    {
+        yield 'regular' => ['image' => 'cats.jpeg', 'urlimage' => 'cats.jpeg'];
+        yield 'whitespace' => ['image' => 'white cat.jpeg', 'urlimage' => 'white%20cat.jpeg'];
+        yield 'plus' => ['image' => 'cat+plus.jpeg', 'urlimage' => 'cat%2Bplus.jpeg'];
+        yield 'questionmark' => ['image' => 'cat?question.jpeg', 'urlimage' => 'cat%3Fquestion.jpeg'];
+        yield 'hash' => ['image' => 'cat#hash.jpeg', 'urlimage' => 'cat%23hash.jpeg'];
+    }
+
+    /**
+     * @dataProvider provideImageNames
+     *
+     * @param string $image
+     */
+    public function testShouldResolvePopulatingCacheFirst($image, $urlimage): void
     {
         //guard
-        $this->assertFileDoesNotExist($this->cacheRoot.'/thumbnail_web_path/images/cats.jpeg');
+        $this->assertFileDoesNotExist($this->cacheRoot.'/thumbnail_web_path/images/'.$image);
 
-        $this->client->request('GET', '/media/cache/resolve/thumbnail_web_path/images/cats.jpeg');
+        $this->client->request('GET', '/media/cache/resolve/thumbnail_web_path/images/'.$urlimage);
 
         $response = $this->client->getResponse();
 
         $this->assertInstanceOf(RedirectResponse::class, $response);
         $this->assertSame(302, $response->getStatusCode());
-        $this->assertSame('http://localhost/media/cache/thumbnail_web_path/images/cats.jpeg', $response->getTargetUrl());
+        $this->assertSame('http://localhost/media/cache/thumbnail_web_path/images/'.$urlimage, $response->getTargetUrl());
 
-        $this->assertFileExists($this->cacheRoot.'/thumbnail_web_path/images/cats.jpeg');
+        $this->assertFileExists($this->cacheRoot.'/thumbnail_web_path/images/'.$image);
 
         // PHP compiled with WebP support
         if ($this->webp_generate) {
-            $this->assertFileExists($this->cacheRoot.'/thumbnail_web_path/images/cats.jpeg.webp');
+            $this->assertFileExists($this->cacheRoot.'/thumbnail_web_path/images/'.$image.'.webp');
         }
     }
 

Tests/Functional/Imagine/Cache/CacheManagerTest.php

@@ -0,0 +1,50 @@
+<?php
+
+/*
+ * This file is part of the `liip/LiipImagineBundle` project.
+ *
+ * (c) https://github.com/liip/LiipImagineBundle/graphs/contributors
+ *
+ * For the full copyright and license information, please view the LICENSE.md
+ * file that was distributed with this source code.
+ */
+
+namespace Liip\ImagineBundle\Tests\Functional\Imagine\Cache;
+
+use Liip\ImagineBundle\Imagine\Cache\CacheManager;
+use Liip\ImagineBundle\Tests\Functional\AbstractWebTestCase;
+
+/**
+ * @covers \Liip\ImagineBundle\Imagine\Cache\CacheManager
+ */
+class CacheManagerTest extends AbstractWebTestCase
+{
+    public function providePaths(): iterable
+    {
+        yield 'querystring' => [
+            'path' => 'path?v51',
+            'expectedUrl' => 'path%3Fv51',
+        ];
+
+        yield 'plus' => [
+            'path' => 'path+x.jpg',
+            'expectedUrl' => 'path+x.jpg',
+        ];
+    }
+
+    /**
+     * @dataProvider providePaths
+     */
+    public function testGetAsService($path, $expectedUrl): void
+    {
+        $this->createClient();
+
+        /** @var CacheManager $manager */
+        $manager = self::$kernel->getContainer()->get('liip_imagine.cache.manager');
+
+        $this->assertSame(
+            'http://localhost/media/cache/resolve/thumbnail_web_path/'.$expectedUrl,
+            $manager->generateUrl($path, 'thumbnail_web_path')
+        );
+    }
+}

UPGRADE.md

@@ -21,6 +21,10 @@ given release.
 ## [2.2.0](https://github.com/liip/LiipImagineBundle/blob/2.2.0/CHANGELOG.md#unreleased)
 
 *Released on* 2019-04-10 *and assigned* [`2.2.0`](https://github.com/liip/LiipImagineBundle/releases/tag/2.2.0) *tag \([view verbose changelog](https://github.com/liip/LiipImagineBundle/compare/2.0.0...2.2.0)\).*
+- Until this version, it was possible to pass any URL with domain to the ImagineController and, if the StreamLoader was
+  used, have your application download images from anywhere and store them locally. In 2.2.0, this security issue has
+  been fixed. If you relied on this, you need to download images to a location that can be accessed by your configured
+  loader. If everything comes from the same domain, you could use the StreamLoader with that domain configured as prefix.
 - __[Deprecated]__ Constructing `FileSystemLoader`, `FlysystemLoader`, `SimpleMimeTypeGuesser` and `DataManager` with 
 `\Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesserInterface` and 
 `\Symfony\Component\HttpFoundation\File\MimeType\ExtensionGuesserInterface` have been deprecated for Symfony 4.3+ in