Allow session locking to work with session_regenerate_id (see #1267)

redis_session.c

@@ -62,7 +62,7 @@
 #define IS_REDIS_OK(r, len) (r != NULL && len == 3 && !memcmp(r, "+OK", 3))
 
 ps_module ps_mod_redis = {
-    PS_MOD(redis)
+    PS_MOD_SID(redis)
 };
 ps_module ps_mod_redis_cluster = {
     PS_MOD(rediscluster)
@@ -581,6 +581,55 @@ redis_session_key(redis_pool_member *rpm, const char *key, int key_len, int *ses
     return session;
 }
 
+/* {{{ PS_CREATE_SID_FUNC
+ */
+PS_CREATE_SID_FUNC(redis)
+{
+    char *sid;
+    int retries = 3;
+
+    redis_pool *pool = PS_GET_MOD_DATA();
+
+    while (retries-- > 0) {
+#if (PHP_MAJOR_VERSION < 7)
+        sid = php_session_create_id((void **) &pool, newlen TSRMLS_CC);
+        redis_pool_member *rpm = redis_pool_get_sock(pool, sid TSRMLS_CC);
+#else
+        sid = php_session_create_id((void **) &pool TSRMLS_CC);
+        redis_pool_member *rpm = redis_pool_get_sock(pool, ZSTR_VAL(sid) TSRMLS_CC);
+#endif
+        RedisSock *redis_sock = rpm?rpm->redis_sock:NULL;
+
+        if (!rpm || !redis_sock) {
+            php_error_docref(NULL TSRMLS_CC, E_NOTICE,
+                "Redis not available while creating session_id");
+
+            efree(sid);
+#if (PHP_MAJOR_VERSION < 7)
+            return php_session_create_id(NULL, newlen TSRMLS_CC);
+#else
+            return php_session_create_id(NULL TSRMLS_CC);
+#endif
+        }
+
+        pool->lock_status->session_key = sid;
+        if (lock_acquire(redis_sock, pool->lock_status TSRMLS_CC) == SUCCESS) {
+            break;
+        }
+
+        efree(sid);
+        sid = NULL;
+    }
+
+    if (sid == NULL) {
+        php_error_docref(NULL TSRMLS_CC, E_NOTICE,
+            "Acquiring session lock failed while creating session_id");
+    }
+
+    return sid;
+}
+/* }}} */
+
 /* {{{ PS_READ_FUNC
  */
 PS_READ_FUNC(redis)

redis_session.h

@@ -9,6 +9,7 @@ PS_READ_FUNC(redis);
 PS_WRITE_FUNC(redis);
 PS_DESTROY_FUNC(redis);
 PS_GC_FUNC(redis);
+PS_CREATE_SID_FUNC(redis);
 
 PS_OPEN_FUNC(rediscluster);
 PS_CLOSE_FUNC(rediscluster);

tests/RedisTest.php

@@ -5288,6 +5288,46 @@ class Redis_Test extends TestSuite
         }
     }
 
+    public  function testSession_regenerateSessionId_noLock_noDestroy() {
+        $this->setSessionHandler();
+        $sessionId = $this->generateSessionId();
+        $writeSuccessful = $this->startSessionProcess($sessionId, 0, false, 300, true, null, -1, 1, 'bar');
+
+        $newSessionId = $this->regenerateSessionId($sessionId);
+	
+        $this->assertEquals('bar', $this->getSessionData($newSessionId));
+    }
+
+    public  function testSession_regenerateSessionId_noLock_withDestroy() {
+        $this->setSessionHandler();
+        $sessionId = $this->generateSessionId();
+        $writeSuccessful = $this->startSessionProcess($sessionId, 0, false, 300, true, null, -1, 1, 'bar');
+
+        $newSessionId = $this->regenerateSessionId($sessionId, false, true);
+	
+        $this->assertEquals('bar', $this->getSessionData($newSessionId));
+    }
+
+    public  function testSession_regenerateSessionId_withLock_noDestroy() {
+        $this->setSessionHandler();
+        $sessionId = $this->generateSessionId();
+        $writeSuccessful = $this->startSessionProcess($sessionId, 0, false, 300, true, null, -1, 1, 'bar');
+
+        $newSessionId = $this->regenerateSessionId($sessionId, true);
+	
+        $this->assertEquals('bar', $this->getSessionData($newSessionId));
+    }
+
+    public  function testSession_regenerateSessionId_withLock_withDestroy() {
+        $this->setSessionHandler();
+        $sessionId = $this->generateSessionId();
+        $writeSuccessful = $this->startSessionProcess($sessionId, 0, false, 300, true, null, -1, 1, 'bar');
+
+        $newSessionId = $this->regenerateSessionId($sessionId, true, true);
+
+        $this->assertEquals('bar', $this->getSessionData($newSessionId));
+    }
+
     private function setSessionHandler()
     {
         $host = $this->getHost() ?: 'localhost';
@@ -5358,5 +5398,23 @@ class Redis_Test extends TestSuite
 
         return $output[0];
     }
+
+    /**
+     * @param string $sessionId
+     *
+     * @return string
+     */
+    private function regenerateSessionId($sessionId, $locking = false, $destroyPrevious = false)
+    {
+	$args = array_map('escapeshellarg', array($sessionId, $locking, $destroyPrevious));
+
+        $command = 'php --no-php-ini --define extension=igbinary.so --define extension=' . __DIR__ . '/../modules/redis.so ' . __DIR__ . '/regenerateSessionId.php ' . escapeshellarg($this->getHost()) . ' ' . implode(' ', $args);
+
+        $command .= ' 2>&1';
+
+        exec($command, $output);
+
+        return $output[0];
+    }
 }
 ?>

tests/regenerateSessionId.php

@@ -0,0 +1,31 @@
+<?php
+error_reporting(E_ERROR | E_WARNING);
+
+$redisHost = $argv[1];
+$sessionId = $argv[2];
+$locking = !!$argv[3];
+$destroyPrevious = !!$argv[4];
+
+if (empty($redisHost)) {
+    $redisHost = 'localhost';
+}
+
+ini_set('session.save_handler', 'redis');
+ini_set('session.save_path', 'tcp://' . $redisHost . ':6379');
+
+if ($locking) {
+    ini_set('redis.session.locking_enabled', true);
+}
+
+session_id($sessionId);
+if (!session_start()) {
+    $result = "FAILED: session_start()";
+}
+elseif (!session_regenerate_id($destroyPrevious)) {
+    $result = "FAILED: session_regenerate_id()";
+}
+else {
+    $result = session_id();
+}
+echo $result;
+