ENGCOM-8326: Fix #30296 - Wrong ip value in sendfriend_log table #30355

Author: sidolov

app/code/Magento/SendFriend/Model/ResourceModel/SendFriend.php

@@ -6,10 +6,6 @@
 namespace Magento\SendFriend\Model\ResourceModel;
 
 /**
- * SendFriend Log Resource Model
- *
- * @author      Magento Core Team <[email protected]>
- *
  * @api
  * @since 100.0.2
  */
@@ -32,6 +28,7 @@ protected function _construct()
      * @param int $ip
      * @param int $startTime
      * @param int $websiteId
+     *
      * @return int
      * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      */
@@ -46,7 +43,7 @@ public function getSendCount($object, $ip, $startTime, $websiteId = null)
                 AND  time>=:time
                 AND  website_id=:website_id'
         );
-        $bind = ['ip' => ip2long($ip), 'time' => $startTime, 'website_id' => (int)$websiteId];
+        $bind = ['ip' => $ip, 'time' => $startTime, 'website_id' => (int)$websiteId];
 
         $row = $connection->fetchRow($select, $bind);
         return $row['count'];
@@ -58,21 +55,24 @@ public function getSendCount($object, $ip, $startTime, $websiteId = null)
      * @param int $ip
      * @param int $startTime
      * @param int $websiteId
+     *
      * @return $this
      */
     public function addSendItem($ip, $startTime, $websiteId)
     {
         $this->getConnection()->insert(
             $this->getMainTable(),
-            ['ip' => ip2long($ip), 'time' => $startTime, 'website_id' => $websiteId]
+            ['ip' => $ip, 'time' => $startTime, 'website_id' => $websiteId]
         );
+
         return $this;
     }
 
     /**
      * Delete Old logs
      *
      * @param int $time
+     *
      * @return $this
      */
     public function deleteLogsBefore($time)

dev/tests/integration/testsuite/Magento/SendFriend/Model/SendFriendTest.php

@@ -13,7 +13,7 @@
 use Magento\SendFriend\Helper\Data as SendFriendHelper;
 use Magento\TestFramework\Helper\Bootstrap;
 use PHPUnit\Framework\TestCase;
-use Zend\Stdlib\Parameters;
+use Laminas\Stdlib\Parameters;
 
 /**
  * Class checks send friend model behavior
@@ -28,6 +28,9 @@ class SendFriendTest extends TestCase
     /** @var SendFriend */
     private $sendFriend;
 
+    /** @var ResourceModel\SendFriend */
+    private $sendFriendResource;
+
     /** @var CookieManagerInterface */
     private $cookieManager;
 
@@ -43,6 +46,7 @@ protected function setUp(): void
 
         $this->objectManager = Bootstrap::getObjectManager();
         $this->sendFriend = $this->objectManager->get(SendFriendFactory::class)->create();
+        $this->sendFriendResource = $this->objectManager->get(ResourceModel\SendFriend::class);
         $this->cookieManager = $this->objectManager->get(CookieManagerInterface::class);
         $this->request = $this->objectManager->get(RequestInterface::class);
     }
@@ -55,6 +59,7 @@ protected function setUp(): void
      * @param array $sender
      * @param array $recipients
      * @param string|bool $expectedResult
+     *
      * @return void
      */
     public function testValidate(array $sender, array $recipients, $expectedResult): void
@@ -185,22 +190,34 @@ public function testisExceedLimitByCookies(): void
      * @magentoDataFixture Magento/SendFriend/_files/sendfriend_log_record_half_hour_before.php
      *
      * @magentoDbIsolation disabled
+     *
      * @return void
      */
     public function testisExceedLimitByIp(): void
     {
-        $this->markTestSkipped('Blocked by MC-31968');
+        $remoteAddr = '127.0.0.1';
         $parameters = $this->objectManager->create(Parameters::class);
-        $parameters->set('REMOTE_ADDR', '127.0.0.1');
+        $parameters->set('REMOTE_ADDR', $remoteAddr);
         $this->request->setServer($parameters);
         $this->assertTrue($this->sendFriend->isExceedLimit());
+        // Verify that ip is saved correctly as integer value
+        $this->assertEquals(
+            1,
+            (int)$this->sendFriendResource->getSendCount(
+                null,
+                ip2long($remoteAddr),
+                time() - (60 * 60 * 24 * 365),
+                1
+            )
+        );
     }
 
     /**
-     * Check result
+     * Check test result
      *
      * @param array|bool $expectedResult
      * @param array|bool $result
+     *
      * @return void
      */
     private function checkResult($expectedResult, $result): void
@@ -217,6 +234,7 @@ private function checkResult($expectedResult, $result): void
      *
      * @param array $sender
      * @param array $recipients
+     *
      * @return void
      */
     private function prepareData(array $sender, array $recipients): void

lib/internal/Magento/Framework/HTTP/PhpEnvironment/RemoteAddress.php

@@ -120,7 +120,7 @@ function (string $ip) {
     public function getRemoteAddress(bool $ipToLong = false)
     {
         if ($this->remoteAddress !== null) {
-            return $this->remoteAddress;
+            return $ipToLong ? ip2long($this->remoteAddress) : $this->remoteAddress;
         }
 
         $remoteAddress = $this->readAddress();
@@ -135,11 +135,11 @@ public function getRemoteAddress(bool $ipToLong = false)
             $this->remoteAddress = false;
 
             return false;
-        } else {
-            $this->remoteAddress = $remoteAddress;
-
-            return $ipToLong ? ip2long($this->remoteAddress) : $this->remoteAddress;
         }
+
+        $this->remoteAddress = $remoteAddress;
+
+        return $ipToLong ? ip2long($this->remoteAddress) : $this->remoteAddress;
     }
 
     /**

lib/internal/Magento/Framework/HTTP/Test/Unit/PhpEnvironment/RemoteAddressTest.php

@@ -9,38 +9,28 @@
 
 use Magento\Framework\App\Request\Http as HttpRequest;
 use Magento\Framework\HTTP\PhpEnvironment\RemoteAddress;
-use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 
 /**
- * Test for
- *
  * @see RemoteAddress
  */
 class RemoteAddressTest extends TestCase
 {
     /**
      * @var MockObject|HttpRequest
      */
-    protected $_request;
-
-    /**
-     * @var ObjectManager
-     */
-    protected $_objectManager;
+    private $requestMock;
 
     /**
      * @inheritdoc
      */
     protected function setUp(): void
     {
-        $this->_request = $this->getMockBuilder(HttpRequest::class)
+        $this->requestMock = $this->getMockBuilder(HttpRequest::class)
             ->disableOriginalConstructor()
-            ->setMethods(['getServer'])
+            ->onlyMethods(['getServer'])
             ->getMock();
-
-        $this->_objectManager = new ObjectManager($this);
     }
 
     /**
@@ -49,6 +39,7 @@ protected function setUp(): void
      * @param string|bool $expected
      * @param bool $ipToLong
      * @param string[]|null $trustedProxies
+     *
      * @return void
      * @dataProvider getRemoteAddressProvider
      */
@@ -59,18 +50,16 @@ public function testGetRemoteAddress(
         bool $ipToLong,
         array $trustedProxies = null
     ): void {
-        $remoteAddress = $this->_objectManager->getObject(
-            RemoteAddress::class,
-            [
-                'httpRequest' => $this->_request,
-                'alternativeHeaders' => $alternativeHeaders,
-                'trustedProxies' => $trustedProxies,
-            ]
+        $remoteAddress = new RemoteAddress(
+            $this->requestMock,
+            $alternativeHeaders,
+            $trustedProxies
         );
-        $this->_request->expects($this->any())
-            ->method('getServer')
+        $this->requestMock->method('getServer')
             ->willReturnMap($serverValueMap);
 
+        // Check twice to verify if internal variable is cached correctly
+        $this->assertEquals($expected, $remoteAddress->getRemoteAddress($ipToLong));
         $this->assertEquals($expected, $remoteAddress->getRemoteAddress($ipToLong));
     }