MAGETWO-80194: [2.2.x] - Handle lifetime value 0 for form_key cookie #10528

Author: omiroshnichenko

app/code/Magento/PageCache/Observer/RegisterFormKeyFromCookie.php

@@ -84,10 +84,7 @@ private function updateCookieFormKey($formKey)
             ->createPublicCookieMetadata();
         $cookieMetadata->setDomain($this->sessionConfig->getCookieDomain());
         $cookieMetadata->setPath($this->sessionConfig->getCookiePath());
-        $lifetime = $this->sessionConfig->getCookieLifetime();
-        if ($lifetime !== 0) {
-            $cookieMetadata->setDuration($lifetime);
-        }
+        $cookieMetadata->setDuration($this->sessionConfig->getCookieLifetime());
 
         $this->cookieFormKey->set(
             $formKey,

app/code/Magento/PageCache/Test/Unit/Observer/RegisterFormKeyFromCookieTest.php

@@ -166,74 +166,4 @@ public function testExecute()
 
         $this->observer->execute($this->observerMock);
     }
-
-    public function testExecuteWithZeroLifetime()
-    {
-        $formKey = 'form_key';
-        $escapedFormKey = 'escaped_form_key';
-        $cookieDomain = 'example.com';
-        $cookiePath = '/';
-        $cookieLifetime = 0;
-
-        $cookieMetadata = $this->getMockBuilder(
-            \Magento\Framework\Stdlib\Cookie\PublicCookieMetadata::class
-        )
-            ->disableOriginalConstructor()
-            ->getMock();
-
-        $this->cookieFormKey->expects(static::any())
-            ->method('get')
-            ->willReturn($formKey);
-        $this->cookieMetadataFactory->expects(static::once())
-            ->method('createPublicCookieMetadata')
-            ->willReturn(
-                $cookieMetadata
-            );
-
-        $this->sessionConfig->expects(static::once())
-            ->method('getCookieDomain')
-            ->willReturn(
-                $cookieDomain
-            );
-        $cookieMetadata->expects(static::once())
-            ->method('setDomain')
-            ->with(
-                $cookieDomain
-            );
-        $this->sessionConfig->expects(static::once())
-            ->method('getCookiePath')
-            ->willReturn(
-                $cookiePath
-            );
-        $cookieMetadata->expects(static::once())
-            ->method('setPath')
-            ->with(
-                $cookiePath
-            );
-        $this->sessionConfig->expects(static::once())
-            ->method('getCookieLifetime')
-            ->willReturn(
-                $cookieLifetime
-            );
-        $cookieMetadata->expects(static::never())
-            ->method('setDuration');
-
-        $this->cookieFormKey->expects(static::once())
-            ->method('set')
-            ->with(
-                $formKey,
-                $cookieMetadata
-            );
-
-        $this->escaper->expects(static::once())
-            ->method('escapeHtml')
-            ->with($formKey)
-            ->willReturn($escapedFormKey);
-
-        $this->sessionFormKey->expects(static::once())
-            ->method('set')
-            ->with($escapedFormKey);
-
-        $this->observer->execute($this->observerMock);
-    }
 }

lib/internal/Magento/Framework/Stdlib/Cookie/PhpCookieManager.php

@@ -237,7 +237,9 @@ private function computeExpirationTime(array $metadataArray)
         ) {
             $expireTime = $metadataArray[PhpCookieManager::KEY_EXPIRE_TIME];
         } else {
-            if (isset($metadataArray[CookieMetadata::KEY_DURATION])) {
+            if (isset($metadataArray[CookieMetadata::KEY_DURATION])
+                && $metadataArray[CookieMetadata::KEY_DURATION] !== PhpCookieManager::EXPIRE_AT_END_OF_SESSION_TIME
+            ) {
                 $expireTime = $metadataArray[CookieMetadata::KEY_DURATION] + time();
             } else {
                 $expireTime = PhpCookieManager::EXPIRE_AT_END_OF_SESSION_TIME;

lib/internal/Magento/Framework/Stdlib/Test/Unit/Cookie/PhpCookieManagerTest.php

@@ -36,6 +36,7 @@ class PhpCookieManagerTest extends \PHPUnit\Framework\TestCase
         const PUBLIC_COOKIE_NAME_DEFAULT_VALUES = 'public_cookie_name_default_values';
         const PUBLIC_COOKIE_NAME_SOME_FIELDS_SET = 'public_cookie_name_some_fields_set';
         const MAX_COOKIE_SIZE_TEST_NAME = 'max_cookie_size_test_name';
+        const PUBLIC_COOKIE_ZERO_DURATION = 'public_cookie_zero_duration';
         const MAX_NUM_COOKIE_TEST_NAME = 'max_num_cookie_test_name';
         const DELETE_COOKIE_NAME = 'delete_cookie_name';
         const DELETE_COOKIE_NAME_NO_METADATA = 'delete_cookie_name_no_metadata';
@@ -66,6 +67,7 @@ class PhpCookieManagerTest extends \PHPUnit\Framework\TestCase
             self::PUBLIC_COOKIE_NAME_DEFAULT_VALUES => 'self::assertPublicCookieWithDefaultValues',
             self::PUBLIC_COOKIE_NAME_SOME_FIELDS_SET => 'self::assertPublicCookieWithSomeFieldSet',
             self::MAX_COOKIE_SIZE_TEST_NAME => 'self::assertCookieSize',
+            self::PUBLIC_COOKIE_ZERO_DURATION => 'self::assertZeroDuration',
         ];
 
         /**
@@ -405,6 +407,38 @@ public function testSetPublicCookieDefaultValues()
             $this->assertTrue(self::$isSetCookieInvoked);
         }
 
+        public function testSetPublicCookieZeroDuration()
+        {
+            /** @var PublicCookieMetadata $publicCookieMetadata */
+            $publicCookieMetadata = $this->objectManager->getObject(
+                \Magento\Framework\Stdlib\Cookie\PublicCookieMetadata::class,
+                [
+                    'metadata' => [
+                        'domain' => null,
+                        'path' => null,
+                        'secure' => false,
+                        'http_only' => false,
+                        'duration' => 0,
+                    ],
+                ]
+            );
+
+            $this->scopeMock->expects($this->once())
+                ->method('getPublicCookieMetadata')
+                ->with($publicCookieMetadata)
+                ->will(
+                    $this->returnValue($publicCookieMetadata)
+                );
+
+            $this->cookieManager->setPublicCookie(
+                self::PUBLIC_COOKIE_ZERO_DURATION,
+                'cookie_value',
+                $publicCookieMetadata
+            );
+
+            $this->assertTrue(self::$isSetCookieInvoked);
+        }
+
         public function testSetPublicCookieSomeFieldsSet()
         {
             self::$isSetCookieInvoked = false;
@@ -839,6 +873,30 @@ private static function assertCookieSize(
             self::assertEquals('', $path);
         }
 
+        /**
+         * Assert cookie set with zero duration
+         *
+         * Suppressing UnusedPrivateMethod, since PHPMD doesn't detect callback method use.
+         * @SuppressWarnings(PHPMD.UnusedPrivateMethod)
+         */
+        private static function assertZeroDuration(
+            $name,
+            $value,
+            $expiry,
+            $path,
+            $domain,
+            $secure,
+            $httpOnly
+        ) {
+            self::assertEquals(self::PUBLIC_COOKIE_ZERO_DURATION, $name);
+            self::assertEquals(self::COOKIE_VALUE, $value);
+            self::assertEquals(self::COOKIE_EXPIRE_END_OF_SESSION, $expiry);
+            self::assertFalse($secure);
+            self::assertFalse($httpOnly);
+            self::assertEquals('', $domain);
+            self::assertEquals('', $path);
+        }
+
         protected function stubGetCookie($get, $default, $return)
         {
             $this->readerMock->expects($this->atLeastOnce())