Merge branch '2.3' into 2.4

* 2.3:
  [Bridge][Twig] Replace deprecated features
  [HttpFoundation] fix switch statement
  [Doctrine Bridge] fix DBAL session handler according to PdoSessionHandler
  fixed previous merge
  Added phpdoc for Cache-Control directives methods
  Remove undefined variable $e
  bumped Symfony version to 2.3.17
  Fix a parameter name in a test
  updated VERSION for 2.3.16
  update CONTRIBUTORS for 2.3.16
  updated CHANGELOG for 2.3.16
  [HttpFoundation] use different approach for duplicate keys in postgres, fix merge for sqlsrv and oracle

Conflicts:
	src/Symfony/Component/DependencyInjection/ContainerBuilder.php
	src/Symfony/Component/HttpKernel/Kernel.php

Author: fabpot

HeaderBag.php

@@ -240,23 +240,48 @@ public function getDate($key, \DateTime $default = null)
         return $date;
     }
 
+    /**
+     * Adds a custom Cache-Control directive.
+     *
+     * @param string $key   The Cache-Control directive name
+     * @param mixed  $value The Cache-Control directive value
+     */
     public function addCacheControlDirective($key, $value = true)
     {
         $this->cacheControl[$key] = $value;
 
         $this->set('Cache-Control', $this->getCacheControlHeader());
     }
 
+    /**
+     * Returns true if the Cache-Control directive is defined.
+     *
+     * @param string $key The Cache-Control directive
+     *
+     * @return bool true if the directive exists, false otherwise
+     */
     public function hasCacheControlDirective($key)
     {
         return array_key_exists($key, $this->cacheControl);
     }
 
+    /**
+     * Returns a Cache-Control directive value by name.
+     *
+     * @param string $key The directive name
+     *
+     * @return mixed|null The directive value if defined, null otherwise
+     */
     public function getCacheControlDirective($key)
     {
         return array_key_exists($key, $this->cacheControl) ? $this->cacheControl[$key] : null;
     }
 
+    /**
+     * Removes a Cache-Control directive.
+     *
+     * @param string $key The Cache-Control directive
+     */
     public function removeCacheControlDirective($key)
     {
         unset($this->cacheControl[$key]);

Session/Storage/Handler/PdoSessionHandler.php

@@ -12,7 +12,13 @@
 namespace Symfony\Component\HttpFoundation\Session\Storage\Handler;
 
 /**
- * PdoSessionHandler.
+ * Session handler using a PDO connection to read and write data.
+ *
+ * Session data is a binary string that can contain non-printable characters like the null byte.
+ * For this reason this handler base64 encodes the data to be able to save it in a character column.
+ *
+ * This version of the PdoSessionHandler does NOT implement locking. So concurrent requests to the
+ * same session can result in data loss due to race conditions.
  *
  * @author Fabien Potencier <fabien@symfony.com>
  * @author Michael Williams <michael.williams@funsational.com>
@@ -164,13 +170,10 @@ public function read($sessionId)
      */
     public function write($sessionId, $data)
     {
-        // Session data can contain non binary safe characters so we need to encode it.
         $encoded = base64_encode($data);
 
-        // We use a MERGE SQL query when supported by the database.
-        // Otherwise we have to use a transactional DELETE followed by INSERT to prevent duplicate entries under high concurrency.
-
         try {
+            // We use a single MERGE SQL query when supported by the database.
             $mergeSql = $this->getMergeSql();
 
             if (null !== $mergeSql) {
@@ -183,28 +186,36 @@ public function write($sessionId, $data)
                 return true;
             }
 
-            $this->pdo->beginTransaction();
-
-            try {
-                $deleteStmt = $this->pdo->prepare(
-                    "DELETE FROM $this->table WHERE $this->idCol = :id"
-                );
-                $deleteStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
-                $deleteStmt->execute();
-
-                $insertStmt = $this->pdo->prepare(
-                    "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)"
-                );
-                $insertStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
-                $insertStmt->bindParam(':data', $encoded, \PDO::PARAM_STR);
-                $insertStmt->bindValue(':time', time(), \PDO::PARAM_INT);
-                $insertStmt->execute();
-
-                $this->pdo->commit();
-            } catch (\PDOException $e) {
-                $this->pdo->rollback();
-
-                throw $e;
+            $updateStmt = $this->pdo->prepare(
+                "UPDATE $this->table SET $this->dataCol = :data, $this->timeCol = :time WHERE $this->idCol = :id"
+            );
+            $updateStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
+            $updateStmt->bindParam(':data', $encoded, \PDO::PARAM_STR);
+            $updateStmt->bindValue(':time', time(), \PDO::PARAM_INT);
+            $updateStmt->execute();
+
+            // When MERGE is not supported, like in Postgres, we have to use this approach that can result in
+            // duplicate key errors when the same session is written simultaneously. We can just catch such an
+            // error and re-execute the update. This is similar to a serializable transaction with retry logic
+            // on serialization failures but without the overhead and without possible false positives due to
+            // longer gap locking.
+            if (!$updateStmt->rowCount()) {
+                try {
+                    $insertStmt = $this->pdo->prepare(
+                        "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)"
+                    );
+                    $insertStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
+                    $insertStmt->bindParam(':data', $encoded, \PDO::PARAM_STR);
+                    $insertStmt->bindValue(':time', time(), \PDO::PARAM_INT);
+                    $insertStmt->execute();
+                } catch (\PDOException $e) {
+                    // Handle integrity violation SQLSTATE 23000 (or a subclass like 23505 in Postgres) for duplicate keys
+                    if (0 === strpos($e->getCode(), '23')) {
+                        $updateStmt->execute();
+                    } else {
+                        throw $e;
+                    }
+                }
             }
         } catch (\PDOException $e) {
             throw new \RuntimeException(sprintf('PDOException was thrown when trying to write the session data: %s', $e->getMessage()), 0, $e);
@@ -230,12 +241,13 @@ private function getMergeSql()
                 // DUAL is Oracle specific dummy table
                 return "MERGE INTO $this->table USING DUAL ON ($this->idCol = :id) " .
                     "WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " .
-                    "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data";
-            case 'sqlsrv':
-                // MS SQL Server requires MERGE be terminated by semicolon
-                return "MERGE INTO $this->table USING (SELECT 'x' AS dummy) AS src ON ($this->idCol = :id) " .
+                    "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data, $this->timeCol = :time";
+            case 'sqlsrv' === $driver && version_compare($this->pdo->getAttribute(\PDO::ATTR_SERVER_VERSION), '10', '>='):
+                // MERGE is only available since SQL Server 2008 and must be terminated by semicolon
+                // It also requires HOLDLOCK according to http://weblogs.sqlteam.com/dang/archive/2009/01/31/UPSERT-Race-Condition-With-MERGE.aspx
+                return "MERGE INTO $this->table WITH (HOLDLOCK) USING (SELECT 1 AS dummy) AS src ON ($this->idCol = :id) " .
                     "WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " .
-                    "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data;";
+                    "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data, $this->timeCol = :time;";
             case 'sqlite':
                 return "INSERT OR REPLACE INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)";
         }

Tests/Session/Storage/Handler/PdoSessionHandlerTest.php

@@ -23,9 +23,9 @@ protected function setUp()
             $this->markTestSkipped('This test requires SQLite support in your environment');
         }
 
-        $this->pdo = new \PDO("sqlite::memory:");
+        $this->pdo = new \PDO('sqlite::memory:');
         $this->pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);
-        $sql = "CREATE TABLE sessions (sess_id VARCHAR(255) PRIMARY KEY, sess_data TEXT, sess_time INTEGER)";
+        $sql = 'CREATE TABLE sessions (sess_id VARCHAR(128) PRIMARY KEY, sess_data TEXT, sess_time INTEGER)';
         $this->pdo->exec($sql);
     }
 
@@ -37,9 +37,9 @@ public function testIncompleteOptions()
 
     public function testWrongPdoErrMode()
     {
-        $pdo = new \PDO("sqlite::memory:");
+        $pdo = new \PDO('sqlite::memory:');
         $pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_SILENT);
-        $pdo->exec("CREATE TABLE sessions (sess_id VARCHAR(255) PRIMARY KEY, sess_data TEXT, sess_time INTEGER)");
+        $pdo->exec('CREATE TABLE sessions (sess_id VARCHAR(128) PRIMARY KEY, sess_data TEXT, sess_time INTEGER)');
 
         $this->setExpectedException('InvalidArgumentException');
         $storage = new PdoSessionHandler($pdo, array('db_table' => 'sessions'));