Merge remote-tracking branch '38270/fix-38269' into aprilcomprs

Author: engcom-Charlie

app/code/Magento/PageCache/Model/App/Request/Http/IdentifierForSave.php

@@ -46,14 +46,38 @@ public function getValue()
     {
         $pattern = $this->identifier->getMarketingParameterPatterns();
         $replace = array_fill(0, count($pattern), '');
+        $url = preg_replace($pattern, $replace, (string)$this->request->getUriString());
+        list($baseUrl, $query) = $this->reconstructUrl($url);
         $data = [
             $this->request->isSecure(),
-            preg_replace($pattern, $replace, (string)$this->request->getUriString()),
+            $baseUrl,
+            $query,
             $this->context->getVaryString()
         ];
 
         $data = $this->identifierStoreReader->getPageTagsWithStoreCacheTags($data);
-
         return sha1($this->serializer->serialize($data));
     }
+
+    /**
+     * Reconstruct url and sort query
+     *
+     * @param string $url
+     * @return array
+     */
+    private function reconstructUrl(string $url): array
+    {
+        if (empty($url)) {
+            return [$url, ''];
+        }
+        $baseUrl = strtok($url, '?');
+        $query = $this->request->getUri()->getQueryAsArray();
+        if (!empty($query)) {
+            ksort($query);
+            $query = http_build_query($query);
+        } else {
+            $query = '';
+        }
+        return [$baseUrl, $query];
+    }
 }

app/code/Magento/PageCache/Test/Unit/Model/App/Request/Http/IdentifierForSaveTest.php

@@ -7,6 +7,8 @@
 
 namespace Magento\PageCache\Test\Unit\Model\App\Request\Http;
 
+use Laminas\Stdlib\Parameters;
+use Laminas\Uri\Http as HttpUri;
 use Magento\Framework\App\Http\Context;
 use Magento\Framework\App\ObjectManager;
 use Magento\Framework\App\PageCache\Identifier;
@@ -49,6 +51,9 @@ class IdentifierForSaveTest extends TestCase
      */
     private $identifierStoreReader;
 
+    /** @var Parameters|MockObject */
+    private $fileParams;
+
     /**
      * @var Identifier
      */
@@ -76,6 +81,7 @@ function ($value) {
                     return json_encode($value);
                 }
             );
+        $this->fileParams = $this->createMock(Parameters::class);
 
         $this->identifierStoreReader = $this->getMockBuilder(IdentifierStoreReader::class)
             ->onlyMethods(['getPageTagsWithStoreCacheTags'])
@@ -120,10 +126,24 @@ public function testGetValue(): void
             ->method('getUriString')
             ->willReturn('http://example.com/path1/');
 
+        $this->requestMock->expects($this->any())
+            ->method('getQuery')
+            ->willReturn($this->fileParams);
+
+        $this->fileParams->expects($this->any())
+            ->method('toArray')
+            ->willReturn([]);
+
         $this->contextMock->expects($this->any())
             ->method('getVaryString')
             ->willReturn(self::VARY);
 
+        $uri = $this->createMock(HttpUri::class);
+        $uri->expects($this->any())->method('getQueryAsArray')->willReturn('');
+        $this->requestMock->expects($this->any())
+            ->method('getUri')
+            ->willReturn($uri);
+
         $this->identifierStoreReader->method('getPageTagsWithStoreCacheTags')->willReturnCallback(
             function ($value) {
                 return $value;
@@ -136,6 +156,67 @@ function ($value) {
                     [
                         true,
                         'http://example.com/path1/',
+                        '',
+                        self::VARY
+                    ]
+                )
+            ),
+            $this->model->getValue()
+        );
+    }
+
+    /**
+     * Test get identifier for save value with query parameters.
+     *
+     * @return void
+     */
+    public function testGetValueWithQuery(): void
+    {
+        $this->requestMock->expects($this->any())
+            ->method('isSecure')
+            ->willReturn(true);
+
+        $this->requestMock->expects($this->any())
+            ->method('getUriString')
+            ->willReturn('http://example.com/path1/?b=2&a=1');
+
+        $this->requestMock->expects($this->any())
+            ->method('getQuery')
+            ->willReturn($this->fileParams);
+
+        $this->fileParams->expects($this->any())
+            ->method('toArray')
+            ->willReturn([
+                'b' => 2,
+                'a' => 1,
+            ]);
+
+        $this->contextMock->expects($this->any())
+            ->method('getVaryString')
+            ->willReturn(self::VARY);
+
+        $uri = $this->createMock(HttpUri::class);
+        $uri->expects($this->any())->method('getQueryAsArray')->willReturn([
+            'b' => 2,
+            'a' => 1,
+        ]);
+        $this->requestMock->expects($this->any())
+            ->method('getUri')
+            ->willReturn($uri);
+
+        $this->identifierStoreReader->method('getPageTagsWithStoreCacheTags')->willReturnCallback(
+            function ($value) {
+                return $value;
+            }
+        );
+
+        $this->assertEquals(
+            sha1(
+                json_encode(
+                    [
+                        true,
+                        'http://example.com/path1/',
+                        'a=1&b=2',
                         self::VARY
                     ]
                 )
@@ -167,18 +248,24 @@ public function testGetValueWithMarketingParameters(): void
             ->method('getVaryString')
             ->willReturn(self::VARY);
 
+        $uri = $this->createMock(HttpUri::class);
+        $uri->expects($this->any())->method('getQueryAsArray')->willReturn(['abc' => '123']);
+        $this->requestMock->expects($this->any())
+            ->method('getUri')
+            ->willReturn($uri);
+
         $this->identifierStoreReader->method('getPageTagsWithStoreCacheTags')->willReturnCallback(
             function ($value) {
                 return $value;
             }
         );
-
         $this->assertEquals(
             sha1(
                 json_encode(
                     [
                         true,
-                        'http://example.com/path1/?abc=123',
+                        'http://example.com/path1/',
+                        'abc=123',
                         self::VARY
                     ]
                 )

lib/internal/Magento/Framework/App/PageCache/Identifier.php

@@ -52,13 +52,15 @@ public function getValue()
     {
         $pattern = $this->getMarketingParameterPatterns();
         $replace = array_fill(0, count($pattern), '');
+        $url = preg_replace($pattern, $replace, (string)$this->request->getUriString());
+        list($baseUrl, $query) = $this->reconstructUrl($url);
         $data = [
             $this->request->isSecure(),
-            preg_replace($pattern, $replace, (string)$this->request->getUriString()),
+            $baseUrl,
+            $query,
             $this->request->get(\Magento\Framework\App\Response\Http::COOKIE_VARY_STRING)
                 ?: $this->context->getVaryString()
         ];
-
         return sha1($this->serializer->serialize($data));
     }
 
@@ -92,4 +94,26 @@ public function getMarketingParameterPatterns(): array
             '/&?_bta_(.*?)\=[^&]+/',
         ];
     }
+
+    /**
+     * Reconstruct url and sort query
+     *
+     * @param string $url
+     * @return array
+     */
+    private function reconstructUrl(string $url): array
+    {
+        if (empty($url)) {
+            return [$url, ''];
+        }
+        $baseUrl = strtok($url, '?');
+        $query = $this->request->getUri()->getQueryAsArray();
+        if (!empty($query)) {
+            ksort($query);
+            $query = http_build_query($query);
+        } else {
+            $query = '';
+        }
+        return [$baseUrl, $query];
+    }
 }