Use proper error message when session write fails #20807

symfony · digilist · Jan 26, 2017 · Feb 18, 2017 · Jan 26, 2017 · 13434b1a18e76cf9c138101bf129dfd63ed55aec
commit 13434b1a18e76cf9c138101bf129dfd63ed55aec
diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php b/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php
@@ -223,7 +223,29 @@ public function regenerate($destroy = false, $lifetime = null)
     */
    public function save()
    {
-        session_write_close();
+        try {
+            session_write_close();
+        } catch (\ErrorException $e) {
+            // The default PHP error message is not very helpful, as it does not give any information on the curently
+            // used save handler.
+            // Therefore, we catch this exception and throw a new exception is a proper exception message
+            $handler = $this->getSaveHandler();
+            if ($handler instanceof SessionHandlerProxy) {
+                $handler = $handler->getHandler();
+            }
+
+            throw new \ErrorException(
+                sprintf(
+                    'session_write_close(): Failed to write session data with %s handler',
+                    get_class($handler)
+                ),
+                $e->getCode(),
+                $e->getSeverity(),
+                $e->getFile(),
+                $e->getLine(),
+                $e
+            );
+        }

        if (!$this->saveHandler->isWrapper() && !$this->saveHandler->isSessionHandlerInterface()) {
            // This condition matches only PHP 5.3 with internal save handlers

diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxy.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxy.php
@@ -35,6 +35,14 @@ public function __construct(\SessionHandlerInterface $handler)
        $this->saveHandlerName = $this->wrapper ? ini_get('session.save_handler') : 'user';
    }

+    /**
+     * @return \SessionHandlerInterface
+     */
+    public function getHandler(): \SessionHandlerInterface
+    {
+        return $this->handler;
+    }
+
    // \SessionHandlerInterface

    /**
-Original file line number
+Diff line change
@@ -223,7 +223,29 @@ public function regenerate($destroy = false, $lifetime = null)
          */
         public function save()
         {
-            session_write_close();
+            try {
+                session_write_close();
+            } catch (\ErrorException $e) {
             Copy link

  
      
    
  

  
      

  
  Member


      

  

  
    
      

      
            stof
  

      

      

      


        Jan 26, 2017


      
    

  


        
      
  
  
  
    

    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.
    

    
      
      


  


  
    
      this won't work in prod, as notices and warnings are not turned into ErrorException exceptions there.
Code must not be written by assuming that the error handler has this behavior
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

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


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    



  
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
    Contributor


      

  Author


  

  
    
      

      
            digilist
  

      

      

      


        Feb 7, 2017


      
    

  


        
      
  
  
  
    

    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, currently the session write fails silently in production.
I updated the code to check the last error message and throw a E_USER_WARNING with the corrected message. However, the warning is not logged by default.
Alternatively, we can throw an Excpetion if the write fails, as it is an error anyway. However, it does break BC.
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

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


  
  


          
      
  
    
    
      
        
            
    All reactions
+                // The default PHP error message is not very helpful, as it does not give any information on the curently
+                // used save handler.
+                // Therefore, we catch this exception and throw a new exception is a proper exception message
+                $handler = $this->getSaveHandler();
+                if ($handler instanceof SessionHandlerProxy) {
+                    $handler = $handler->getHandler();
+                }
+                throw new \ErrorException(
+                    sprintf(
+                        'session_write_close(): Failed to write session data with %s handler',
+                        get_class($handler)
+                    ),
+                    $e->getCode(),
+                    $e->getSeverity(),
+                    $e->getFile(),
+                    $e->getLine(),
+                    $e
+                );
+            }
             if (!$this->saveHandler->isWrapper() && !$this->saveHandler->isSessionHandlerInterface()) {
                 // This condition matches only PHP 5.3 with internal save handlers
-          Expand Down