symfony · nicolas-grekas · Dec 14, 2022 · Dec 13, 2022 · derrabus · Dec 13, 2022
diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MemcachedSessionHandler.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MemcachedSessionHandler.php
@@ -77,7 +77,7 @@ protected function doRead(string $sessionId)
    #[\ReturnTypeWillChange]
    public function updateTimestamp($sessionId, $data)
    {
-        $this->memcached->touch($this->prefix.$sessionId, time() + (int) ($this->ttl ?? \ini_get('session.gc_maxlifetime')));
+        $this->memcached->touch($this->prefix.$sessionId, $this->getCompatibleTtl());

        return true;
    }
@@ -87,7 +87,20 @@ public function updateTimestamp($sessionId, $data)
     */
    protected function doWrite(string $sessionId, string $data)
    {
-        return $this->memcached->set($this->prefix.$sessionId, $data, time() + (int) ($this->ttl ?? \ini_get('session.gc_maxlifetime')));
+        return $this->memcached->set($this->prefix.$sessionId, $data, $this->getCompatibleTtl());
+    }
+
+    private function getCompatibleTtl(): int
+    {
+        $ttl = (int) ($this->ttl ?? \ini_get('session.gc_maxlifetime'));
+
+        // If the relative TTL that is used exceeds 30 days, memcached will treat the value as Unix time.
+        // We have to convert it to an absolute Unix time at this point, to make sure the TTL is correct.
+        if ($ttl > 60 * 60 * 24 * 30) {
+            $ttl += time();
+        }
+
+        return $ttl;
    }

    /**

diff --git a/...ny/Component/HttpFoundation/Tests/Session/Storage/Handler/MemcachedSessionHandlerTest.php b/...ny/Component/HttpFoundation/Tests/Session/Storage/Handler/MemcachedSessionHandlerTest.php
@@ -16,6 +16,7 @@

 /**
 * @requires extension memcached
+ *
 * @group time-sensitive
 */
 class MemcachedSessionHandlerTest extends TestCase
@@ -92,13 +93,30 @@ public function testWriteSession()
        $this->memcached
            ->expects($this->once())
            ->method('set')
-            ->with(self::PREFIX.'id', 'data', $this->equalTo(time() + self::TTL, 2))
+            ->with(self::PREFIX.'id', 'data', $this->equalTo(self::TTL, 2))
            ->willReturn(true)
        ;

        $this->assertTrue($this->storage->write('id', 'data'));
    }

+    public function testWriteSessionWithLargeTTL()
+    {
+        $this->memcached
+            ->expects($this->once())
+            ->method('set')
+            ->with(self::PREFIX.'id', 'data', $this->equalTo(time() + self::TTL + 60 * 60 * 24 * 30, 2))
+            ->willReturn(true)
+        ;
+
+        $storage = new MemcachedSessionHandler(
+            $this->memcached,
+            ['prefix' => self::PREFIX, 'expiretime' => self::TTL + 60 * 60 * 24 * 30]
+        );
+
+        $this->assertTrue($storage->write('id', 'data'));
+    }
+
    public function testDestroySession()
    {
        $this->memcached
-Original file line number
+Diff line change
@@ -77,7 +77,7 @@ protected function doRead(string $sessionId)
         #[\ReturnTypeWillChange]
         public function updateTimestamp($sessionId, $data)
         {
-            $this->memcached->touch($this->prefix.$sessionId, time() + (int) ($this->ttl ?? \ini_get('session.gc_maxlifetime')));
             Copy link

  
      
    
  

  
      

  
  Member


      

  

  
    
      

      
            derrabus
  

      

      

      


        Dec 13, 2022


      
  •

  
    
      
        
          edited
          
        
        
      
    
    
      
  
        
      Loading


  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  
    
  


    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      I've read up how PHP's memcached extension interprets the values passed here: https://www.php.net/manual/en/memcached.expiration.php
tl;dr: If the passed integer is < 30 days in seconds, the value is considered to be a relative TTL. Otherwise, the value is assumed to be a UNIX timestamp.
My guess is that we added time() to be sure our relative TTL is never mistaken as a timestamp.
If we go back to relative TTLs again, I suggest we make sure our TTL is below that threshold and continue to add time() otherwise.
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    



  
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
    Contributor


      

  Author


  

  
    
      

      
            tvlooy
  

      

      

      


        Dec 13, 2022


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      yes indeed https://github.com/memcached/memcached/blob/master/doc/protocol.txt#L139
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
+            $this->memcached->touch($this->prefix.$sessionId, $this->getCompatibleTtl());
             return true;
         }
@@ -87,7 +87,20 @@ public function updateTimestamp($sessionId, $data)
          */
         protected function doWrite(string $sessionId, string $data)
         {
-            return $this->memcached->set($this->prefix.$sessionId, $data, time() + (int) ($this->ttl ?? \ini_get('session.gc_maxlifetime')));
+            return $this->memcached->set($this->prefix.$sessionId, $data, $this->getCompatibleTtl());
+        }
+        private function getCompatibleTtl(): int
+        {
+            $ttl = (int) ($this->ttl ?? \ini_get('session.gc_maxlifetime'));
+            // If the relative TTL that is used exceeds 30 days, memcached will treat the value as Unix time.
+            // We have to convert it to an absolute Unix time at this point, to make sure the TTL is correct.
+            if ($ttl > 60 * 60 * 24 * 30) {
+                $ttl += time();
+            }
+            return $ttl;
         }
         /**
-          Expand Down