BC fix for PR 109 merge - create_sid() method in SessionHandler

Creates a new SessionIdInterface and moves create_sid() into it, so existing
handlers implementing SessionHandlerInterface don't require create_sid().
SessionHandler still includes the method so the default mod can be called, but
now implements both interfaces.

Also added several more tests for this feature.

9 files changed, 480 insertions, 16 deletions

diff --git a/ext/session/php_session.h b/ext/session/php_session.h
index 210043582d..1dd5b1a1a6 100644
--- a/ext/session/php_session.h
+++ b/ext/session/php_session.h
@@ -278,6 +278,9 @@ extern zend_class_entry *php_session_class_entry;
 #define PS_IFACE_NAME "SessionHandlerInterface"
 extern zend_class_entry *php_session_iface_entry;
 
+#define PS_SID_IFACE_NAME "SessionIdInterface"
+extern zend_class_entry *php_session_id_iface_entry;
+
 extern PHP_METHOD(SessionHandler, open);
 extern PHP_METHOD(SessionHandler, close);
 extern PHP_METHOD(SessionHandler, read);
diff --git a/ext/session/session.c b/ext/session/session.c
index d8de5769fc..78ecd8201c 100644
--- a/ext/session/session.c
+++ b/ext/session/session.c
@@ -70,6 +70,9 @@ zend_class_entry *php_session_class_entry;
 /* SessionHandlerInterface */
 zend_class_entry *php_session_iface_entry;
 
+/* SessionIdInterface */
+zend_class_entry *php_session_id_iface_entry;
+
 /* ***********
    * Helpers *
    *********** */
@@ -1603,11 +1606,11 @@ static PHP_FUNCTION(session_set_save_handler)
 			RETURN_FALSE;
 		}
 
-		/* Find implemented methods */
-		zend_hash_internal_pointer_reset_ex(&php_session_class_entry->function_table, &pos);
+		/* Find implemented methods - SessionHandlerInterface */
+		zend_hash_internal_pointer_reset_ex(&php_session_iface_entry->function_table, &pos);
 		i = 0;
-		while (zend_hash_get_current_data_ex(&php_session_class_entry->function_table, (void **) &default_mptr, &pos) == SUCCESS) {
-			zend_hash_get_current_key_ex(&php_session_class_entry->function_table, &func_name, &func_name_len, &func_index, 0, &pos);
+		while (zend_hash_get_current_data_ex(&php_session_iface_entry->function_table, (void **) &default_mptr, &pos) == SUCCESS) {
+			zend_hash_get_current_key_ex(&php_session_iface_entry->function_table, &func_name, &func_name_len, &func_index, 0, &pos);
 
 			if (zend_hash_find(&Z_OBJCE_P(obj)->function_table, func_name, func_name_len, (void **)&current_mptr) == SUCCESS) {
 				if (PS(mod_user_names).names[i] != NULL) {
@@ -1625,7 +1628,29 @@ static PHP_FUNCTION(session_set_save_handler)
 				RETURN_FALSE;
 			}
 
-			zend_hash_move_forward_ex(&php_session_class_entry->function_table, &pos);
+			zend_hash_move_forward_ex(&php_session_iface_entry->function_table, &pos);
+			++i;
+		}
+
+		/* Find implemented methods - SessionIdInterface (optional) */
+		zend_hash_internal_pointer_reset_ex(&php_session_id_iface_entry->function_table, &pos);
+		while (zend_hash_get_current_data_ex(&php_session_id_iface_entry->function_table, (void **) &default_mptr, &pos) == SUCCESS) {
+			zend_hash_get_current_key_ex(&php_session_id_iface_entry->function_table, &func_name, &func_name_len, &func_index, 0, &pos);
+
+			if (zend_hash_find(&Z_OBJCE_P(obj)->function_table, func_name, func_name_len, (void **)&current_mptr) == SUCCESS) {
+				if (PS(mod_user_names).names[i] != NULL) {
+					zval_ptr_dtor(&PS(mod_user_names).names[i]);
+				}
+
+				MAKE_STD_ZVAL(callback);
+				array_init_size(callback, 2);
+				Z_ADDREF_P(obj);
+				add_next_index_zval(callback, obj);
+				add_next_index_stringl(callback, func_name, func_name_len - 1, 1);
+				PS(mod_user_names).names[i] = callback;
+			}
+
+			zend_hash_move_forward_ex(&php_session_id_iface_entry->function_table, &pos);
 			++i;
 		}
 
@@ -1993,7 +2018,7 @@ ZEND_END_ARG_INFO()
 ZEND_BEGIN_ARG_INFO(arginfo_session_void, 0)
 ZEND_END_ARG_INFO()
 
-ZEND_BEGIN_ARG_INFO_EX(arginfo_session_set_save_handler, 0, 0, 7)
+ZEND_BEGIN_ARG_INFO_EX(arginfo_session_set_save_handler, 0, 0, 1)
 	ZEND_ARG_INFO(0, open)
 	ZEND_ARG_INFO(0, close)
 	ZEND_ARG_INFO(0, read)
@@ -2083,7 +2108,14 @@ static const zend_function_entry php_session_iface_functions[] = {
 	PHP_ABSTRACT_ME(SessionHandlerInterface, write, arginfo_session_class_write)
 	PHP_ABSTRACT_ME(SessionHandlerInterface, destroy, arginfo_session_class_destroy)
 	PHP_ABSTRACT_ME(SessionHandlerInterface, gc, arginfo_session_class_gc)
-	PHP_ABSTRACT_ME(SessionHandlerInterface, create_sid, arginfo_session_class_create_sid)
+	{ NULL, NULL, NULL }
+};
+/* }}} */
+
+/* {{{ SessionIdInterface functions[]
+*/
+static const zend_function_entry php_session_id_iface_functions[] = {
+	PHP_ABSTRACT_ME(SessionIdInterface, create_sid, arginfo_session_class_create_sid)
 	{ NULL, NULL, NULL }
 };
 /* }}} */
@@ -2206,15 +2238,20 @@ static PHP_MINIT_FUNCTION(session) /* {{{ */
 	php_session_rfc1867_orig_callback = php_rfc1867_callback;
 	php_rfc1867_callback = php_session_rfc1867_callback;
 
-	/* Register interface */
+	/* Register interfaces */
 	INIT_CLASS_ENTRY(ce, PS_IFACE_NAME, php_session_iface_functions);
 	php_session_iface_entry = zend_register_internal_class(&ce TSRMLS_CC);
 	php_session_iface_entry->ce_flags |= ZEND_ACC_INTERFACE;
 
+	INIT_CLASS_ENTRY(ce, PS_SID_IFACE_NAME, php_session_id_iface_functions);
+	php_session_id_iface_entry = zend_register_internal_class(&ce TSRMLS_CC);
+	php_session_id_iface_entry->ce_flags |= ZEND_ACC_INTERFACE;
+
 	/* Register base class */
 	INIT_CLASS_ENTRY(ce, PS_CLASS_NAME, php_session_class_functions);
 	php_session_class_entry = zend_register_internal_class(&ce TSRMLS_CC);
 	zend_class_implements(php_session_class_entry TSRMLS_CC, 1, php_session_iface_entry);
+	zend_class_implements(php_session_class_entry TSRMLS_CC, 1, php_session_id_iface_entry);
 
 	REGISTER_LONG_CONSTANT("PHP_SESSION_DISABLED", php_session_disabled, CONST_CS | CONST_PERSISTENT);
 	REGISTER_LONG_CONSTANT("PHP_SESSION_NONE", php_session_none, CONST_CS | CONST_PERSISTENT);
diff --git a/ext/session/tests/session_set_save_handler_class_002.phpt b/ext/session/tests/session_set_save_handler_class_002.phpt
index 4195a163a7..6fb831f695 100644
--- a/ext/session/tests/session_set_save_handler_class_002.phpt
+++ b/ext/session/tests/session_set_save_handler_class_002.phpt
@@ -53,10 +53,6 @@ class MySession2 extends SessionHandler {
 		}
 		return true;
 	}
-	
-	public function create_sid() {
-		return parent::create_sid();
-	}
 }
 
 $handler = new MySession2;
diff --git a/ext/session/tests/session_set_save_handler_class_016.phpt b/ext/session/tests/session_set_save_handler_class_016.phpt
new file mode 100644
index 0000000000..2de03c0682
--- /dev/null
+++ b/ext/session/tests/session_set_save_handler_class_016.phpt
@@ -0,0 +1,90 @@
+--TEST--
+Test session_set_save_handler() function: class with create_sid
+--INI--
+session.save_handler=files
+session.name=PHPSESSID
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--FILE--
+<?php
+
+ob_start();
+
+/* 
+ * Prototype : bool session_set_save_handler(SessionHandlerInterface $handler [, bool $register_shutdown_function = true])
+ * Description : Sets user-level session storage functions
+ * Source code : ext/session/session.c 
+ */
+
+echo "*** Testing session_set_save_handler() function: class with create_sid ***\n";
+
+class MySession2 extends SessionHandler {
+	public $path;
+
+	public function open($path, $name) {
+		if (!$path) {
+			$path = sys_get_temp_dir();
+		}
+		$this->path = $path . '/u_sess_' . $name;
+		return true;
+	}
+
+	public function close() {
+		return true;
+	}
+
+	public function read($id) {
+		return @file_get_contents($this->path . $id);
+	}
+
+	public function write($id, $data) {
+		return file_put_contents($this->path . $id, $data);
+	}
+
+	public function destroy($id) {
+		@unlink($this->path . $id);
+	}
+
+	public function gc($maxlifetime) {
+		foreach (glob($this->path . '*') as $filename) {
+			if (filemtime($filename) + $maxlifetime < time()) {
+				@unlink($filename);
+			}
+		}
+		return true;
+	}
+
+	public function create_sid() {
+		return parent::create_sid();
+	}
+}
+
+$handler = new MySession2;
+session_set_save_handler($handler);
+session_start();
+
+$_SESSION['foo'] = "hello";
+
+var_dump(session_id(), ini_get('session.save_handler'), $_SESSION);
+
+session_write_close();
+session_unset();
+
+session_start();
+var_dump($_SESSION);
+
+session_write_close();
+session_unset();
+
+--EXPECTF--
+*** Testing session_set_save_handler() function: class with create_sid ***
+string(%d) "%s"
+string(4) "user"
+array(1) {
+  ["foo"]=>
+  string(5) "hello"
+}
+array(1) {
+  ["foo"]=>
+  string(5) "hello"
+}
diff --git a/ext/session/tests/session_set_save_handler_class_017.phpt b/ext/session/tests/session_set_save_handler_class_017.phpt
new file mode 100644
index 0000000000..756dc55d03
--- /dev/null
+++ b/ext/session/tests/session_set_save_handler_class_017.phpt
@@ -0,0 +1,90 @@
+--TEST--
+Test session_set_save_handler() function: class with create_sid
+--INI--
+session.save_handler=files
+session.name=PHPSESSID
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--FILE--
+<?php
+
+ob_start();
+
+/* 
+ * Prototype : bool session_set_save_handler(SessionHandlerInterface $handler [, bool $register_shutdown_function = true])
+ * Description : Sets user-level session storage functions
+ * Source code : ext/session/session.c 
+ */
+
+echo "*** Testing session_set_save_handler() function: class with create_sid ***\n";
+
+class MySession2 extends SessionHandler {
+	public $path;
+
+	public function open($path, $name) {
+		if (!$path) {
+			$path = sys_get_temp_dir();
+		}
+		$this->path = $path . '/u_sess_' . $name;
+		return true;
+	}
+
+	public function close() {
+		return true;
+	}
+
+	public function read($id) {
+		return @file_get_contents($this->path . $id);
+	}
+
+	public function write($id, $data) {
+		return file_put_contents($this->path . $id, $data);
+	}
+
+	public function destroy($id) {
+		@unlink($this->path . $id);
+	}
+
+	public function gc($maxlifetime) {
+		foreach (glob($this->path . '*') as $filename) {
+			if (filemtime($filename) + $maxlifetime < time()) {
+				@unlink($filename);
+			}
+		}
+		return true;
+	}
+
+	public function create_sid() {
+		return 'my_sid';
+	}
+}
+
+$handler = new MySession2;
+session_set_save_handler($handler);
+session_start();
+
+$_SESSION['foo'] = "hello";
+
+var_dump(session_id(), ini_get('session.save_handler'), $_SESSION);
+
+session_write_close();
+session_unset();
+
+session_start();
+var_dump($_SESSION);
+
+session_write_close();
+session_unset();
+
+--EXPECTF--
+*** Testing session_set_save_handler() function: class with create_sid ***
+string(%d) "my_sid"
+string(4) "user"
+array(1) {
+  ["foo"]=>
+  string(5) "hello"
+}
+array(1) {
+  ["foo"]=>
+  string(5) "hello"
+}
diff --git a/ext/session/tests/session_set_save_handler_iface_001.phpt b/ext/session/tests/session_set_save_handler_iface_001.phpt
index 0576341a10..39a4b9975b 100644
--- a/ext/session/tests/session_set_save_handler_iface_001.phpt
+++ b/ext/session/tests/session_set_save_handler_iface_001.phpt
@@ -53,10 +53,6 @@ class MySession2 implements SessionHandlerInterface {
 		}
 		return true;
 	}
-	
-	public function create_sid() {
-		return md5(mt_rand());
-	}
 }
 
 $handler = new MySession2;
diff --git a/ext/session/tests/session_set_save_handler_iface_003.phpt b/ext/session/tests/session_set_save_handler_iface_003.phpt
new file mode 100644
index 0000000000..bd757dce63
--- /dev/null
+++ b/ext/session/tests/session_set_save_handler_iface_003.phpt
@@ -0,0 +1,90 @@
+--TEST--
+Test session_set_save_handler() function: id interface
+--INI--
+session.save_handler=files
+session.name=PHPSESSID
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--FILE--
+<?php
+
+ob_start();
+
+/* 
+ * Prototype : bool session_set_save_handler(SessionHandlerInterface $handler [, bool $register_shutdown_function = true])
+ * Description : Sets user-level session storage functions
+ * Source code : ext/session/session.c 
+ */
+
+echo "*** Testing session_set_save_handler() function: id interface ***\n";
+
+class MySession2 implements SessionHandlerInterface, SessionIdInterface {
+	public $path;
+
+	public function open($path, $name) {
+		if (!$path) {
+			$path = sys_get_temp_dir();
+		}
+		$this->path = $path . '/u_sess_' . $name;
+		return true;
+	}
+
+	public function close() {
+		return true;
+	}
+
+	public function read($id) {
+		return @file_get_contents($this->path . $id);
+	}
+
+	public function write($id, $data) {
+		return file_put_contents($this->path . $id, $data);
+	}
+
+	public function destroy($id) {
+		@unlink($this->path . $id);
+	}
+
+	public function gc($maxlifetime) {
+		foreach (glob($this->path . '*') as $filename) {
+			if (filemtime($filename) + $maxlifetime < time()) {
+				@unlink($filename);
+			}
+		}
+		return true;
+	}
+
+	public function create_sid() {
+		return 'my_sid';
+	}
+}
+
+$handler = new MySession2;
+session_set_save_handler($handler);
+session_start();
+
+$_SESSION['foo'] = "hello";
+
+var_dump(session_id(), ini_get('session.save_handler'), $_SESSION);
+
+session_write_close();
+session_unset();
+
+session_start();
+var_dump($_SESSION);
+
+session_write_close();
+session_unset();
+
+--EXPECTF--
+*** Testing session_set_save_handler() function: id interface ***
+string(%d) "my_sid"
+string(4) "user"
+array(1) {
+  ["foo"]=>
+  string(5) "hello"
+}
+array(1) {
+  ["foo"]=>
+  string(5) "hello"
+}
diff --git a/ext/session/tests/session_set_save_handler_sid_001.phpt b/ext/session/tests/session_set_save_handler_sid_001.phpt
new file mode 100644
index 0000000000..0dc4fc11cf
--- /dev/null
+++ b/ext/session/tests/session_set_save_handler_sid_001.phpt
@@ -0,0 +1,85 @@
+--TEST--
+Test session_set_save_handler() function: create_sid
+--INI--
+session.save_handler=files
+session.name=PHPSESSID
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--FILE--
+<?php
+
+ob_start();
+
+echo "*** Testing session_set_save_handler() function: create_sid ***\n";
+
+class MySession2 {
+	public $path;
+
+	public function open($path, $name) {
+		if (!$path) {
+			$path = sys_get_temp_dir();
+		}
+		$this->path = $path . '/u_sess_' . $name;
+		return true;
+	}
+
+	public function close() {
+		return true;
+	}
+
+	public function read($id) {
+		return @file_get_contents($this->path . $id);
+	}
+
+	public function write($id, $data) {
+		return file_put_contents($this->path . $id, $data);
+	}
+
+	public function destroy($id) {
+		@unlink($this->path . $id);
+	}
+
+	public function gc($maxlifetime) {
+		foreach (glob($this->path . '*') as $filename) {
+			if (filemtime($filename) + $maxlifetime < time()) {
+				@unlink($filename);
+			}
+		}
+		return true;
+	}
+
+	public function create_sid() {
+		return 'my_sid';
+	}
+}
+
+$handler = new MySession2;
+session_set_save_handler(array($handler, 'open'), array($handler, 'close'),
+	array($handler, 'read'), array($handler, 'write'), array($handler, 'destroy'), array($handler, 'gc'), array($handler, 'create_sid'));
+session_start();
+
+$_SESSION['foo'] = "hello";
+
+var_dump(session_id(), ini_get('session.save_handler'), $_SESSION);
+
+session_write_close();
+session_unset();
+
+session_start();
+var_dump($_SESSION);
+
+session_write_close();
+session_unset();
+
+--EXPECTF--
+*** Testing session_set_save_handler() function: create_sid ***
+string(%d) "my_sid"
+string(4) "user"
+array(1) {
+  ["foo"]=>
+  string(5) "hello"
+}
+array(1) {
+  ["foo"]=>
+  string(5) "hello"
+}
diff --git a/ext/session/tests/session_set_save_handler_sid_002.phpt b/ext/session/tests/session_set_save_handler_sid_002.phpt
new file mode 100644
index 0000000000..f9a72aebca
--- /dev/null
+++ b/ext/session/tests/session_set_save_handler_sid_002.phpt
@@ -0,0 +1,77 @@
+--TEST--
+Test session_set_save_handler() function: create_sid
+--INI--
+session.save_handler=files
+session.name=PHPSESSID
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--FILE--
+<?php
+
+ob_start();
+
+echo "*** Testing session_set_save_handler() function: create_sid ***\n";
+
+class MySession2 {
+	public $path;
+
+	public function open($path, $name) {
+		if (!$path) {
+			$path = sys_get_temp_dir();
+		}
+		$this->path = $path . '/u_sess_' . $name;
+		return true;
+	}
+
+	public function close() {
+		return true;
+	}
+
+	public function read($id) {
+		return @file_get_contents($this->path . $id);
+	}
+
+	public function write($id, $data) {
+		return file_put_contents($this->path . $id, $data);
+	}
+
+	public function destroy($id) {
+		@unlink($this->path . $id);
+	}
+
+	public function gc($maxlifetime) {
+		foreach (glob($this->path . '*') as $filename) {
+			if (filemtime($filename) + $maxlifetime < time()) {
+				@unlink($filename);
+			}
+		}
+		return true;
+	}
+
+	public function create_sid() {
+		return null;
+	}
+}
+
+$handler = new MySession2;
+session_set_save_handler(array($handler, 'open'), array($handler, 'close'),
+	array($handler, 'read'), array($handler, 'write'), array($handler, 'destroy'), array($handler, 'gc'), array($handler, 'create_sid'));
+session_start();
+
+$_SESSION['foo'] = "hello";
+
+var_dump(session_id(), ini_get('session.save_handler'), $_SESSION);
+
+session_write_close();
+session_unset();
+
+session_start();
+var_dump($_SESSION);
+
+session_write_close();
+session_unset();
+
+--EXPECTF--
+*** Testing session_set_save_handler() function: create_sid ***
+
+Fatal error: session_start(): Session id must be a string in %s on line %d