From b9058d036e762a760f29c0586cfd6ee816fd48ce Mon Sep 17 00:00:00 2001 From: Paul Tarjan Date: Wed, 29 May 2013 22:19:05 -0700 Subject: [PATCH] add SessionHandler In PHP 5.4 they added this class that encased the callback functions. The difficulty came with needing to fallback to the previously registered session handler. Closes #792 --- hphp/runtime/ext/ext_session.cpp | 96 ++++++++---- hphp/runtime/ext/ext_session.h | 29 +++- hphp/system/idl/session.idl.json | 141 +++++++++++++---- hphp/system/php.txt | 3 + .../php/sessions/SessionHandlerInterface.php | 145 ++++++++++++++++++ .../php/sessions/session_set_save_handler.php | 72 +++++++++ .../ext_session/EncryptedSessionHandler.php | 29 ++++ .../EncryptedSessionHandler.php.expect | 3 + .../slow/ext_session/array_as_callback.php | 14 ++ .../ext_session/array_as_callback.php.expect | 4 + .../ext-session/sessionhandler_open_001.php | 0 .../sessionhandler_open_001.php.expectf | 0 12 files changed, 467 insertions(+), 69 deletions(-) create mode 100644 hphp/system/php/sessions/SessionHandlerInterface.php create mode 100644 hphp/system/php/sessions/session_set_save_handler.php create mode 100644 hphp/test/slow/ext_session/EncryptedSessionHandler.php create mode 100644 hphp/test/slow/ext_session/EncryptedSessionHandler.php.expect create mode 100644 hphp/test/slow/ext_session/array_as_callback.php create mode 100644 hphp/test/slow/ext_session/array_as_callback.php.expect rename hphp/test/zend/{bad => good}/ext-session/sessionhandler_open_001.php (100%) rename hphp/test/zend/{bad => good}/ext-session/sessionhandler_open_001.php.expectf (100%) diff --git a/hphp/runtime/ext/ext_session.cpp b/hphp/runtime/ext/ext_session.cpp index d70931a4c..39203efde 100644 --- a/hphp/runtime/ext/ext_session.cpp +++ b/hphp/runtime/ext/ext_session.cpp @@ -78,12 +78,7 @@ public: int m_module_number; int64_t m_cache_expire; - std::string m_ps_open; - std::string m_ps_close; - std::string m_ps_read; - std::string m_ps_write; - std::string m_ps_destroy; - std::string m_ps_gc; + Object m_ps_session_handler; SessionSerializer *m_serializer; @@ -717,21 +712,24 @@ public: UserSessionModule() : SessionModule("user") {} virtual bool open(const char *save_path, const char *session_name) { - return vm_call_user_func - (String(PS(ps_open)), + return vm_call_user_func( + CREATE_VECTOR2(PS(ps_session_handler), + String("open")), CREATE_VECTOR2(String(save_path, CopyString), String(session_name, CopyString))); } virtual bool close() { - return vm_call_user_func - (String(PS(ps_close)), + return vm_call_user_func( + CREATE_VECTOR2(PS(ps_session_handler), + String("close")), Array::Create()); } virtual bool read(const char *key, String &value) { - Variant ret = vm_call_user_func - (String(PS(ps_read)), + Variant ret = vm_call_user_func( + CREATE_VECTOR2(PS(ps_session_handler), + String("read")), CREATE_VECTOR1(String(key, CopyString))); if (ret.isString()) { value = ret.toString(); @@ -741,20 +739,23 @@ public: } virtual bool write(const char *key, CStrRef value) { - return vm_call_user_func - (String(PS(ps_write)), + return vm_call_user_func( + CREATE_VECTOR2(PS(ps_session_handler), + String("write")), CREATE_VECTOR2(String(key, CopyString), value)); } virtual bool destroy(const char *key) { - return vm_call_user_func - (String(PS(ps_destroy)), + return vm_call_user_func( + CREATE_VECTOR2(PS(ps_session_handler), + String("destroy")), CREATE_VECTOR1(String(key, CopyString))); } virtual bool gc(int maxlifetime, int *nrdels) { - return vm_call_user_func - (String(PS(ps_gc)), + return vm_call_user_func( + CREATE_VECTOR2(PS(ps_session_handler), + String("gc")), CREATE_VECTOR1((int64_t)maxlifetime)); } }; @@ -1360,27 +1361,17 @@ Variant f_session_module_name(CStrRef newname /* = null_string */) { return oldname; } -static bool check_handler(const char *name, std::string &member) { - if (!f_is_callable(name)) { - raise_warning("Argument '%s' is not a valid callback", name); - return false; - } - member = name; - return true; -} +bool f_hphp_session_set_save_handler(CObjRef sessionhandler, + bool register_shutdown /* = true */) { -bool f_session_set_save_handler(CStrRef open, CStrRef close, CStrRef read, - CStrRef write, CStrRef destroy, CStrRef gc) { if (PS(session_status) != Session::None) { return false; } - if (!check_handler(open.data(), PS(ps_open))) return false; - if (!check_handler(close.data(), PS(ps_close))) return false; - if (!check_handler(read.data(), PS(ps_read))) return false; - if (!check_handler(write.data(), PS(ps_write))) return false; - if (!check_handler(destroy.data(), PS(ps_destroy))) return false; - if (!check_handler(gc.data(), PS(ps_gc))) return false; + PS(ps_session_handler) = sessionhandler; + if (register_shutdown) { + f_register_shutdown_function(1, String("session_write_close")); + } IniSetting::Set("session.save_handler", "user"); return true; @@ -1648,5 +1639,42 @@ bool f_session_is_registered(CStrRef varname) { "Relying on this feature is highly discouraged."); } +void c_SessionHandler::t___construct() { } +c_SessionHandler::c_SessionHandler(Class* cb) : ExtObjectData(cb) { + m_mod = PS(mod); +} +c_SessionHandler::~c_SessionHandler() { } + +bool c_SessionHandler::t_open(CStrRef save_path, CStrRef session_id) { + return m_mod->open(save_path->data(), session_id->data()); +} + +bool c_SessionHandler::t_close() { + return m_mod->close(); +} + +String c_SessionHandler::t_read(CStrRef session_id) { + String value; + if (m_mod->read(PS(id).data(), value)) { + php_session_decode(value); + return value; + } + return uninit_null(); +} + +bool c_SessionHandler::t_write(CStrRef session_id, CStrRef session_data) { + return m_mod->write(session_id->data(), session_data->data()); +} + +bool c_SessionHandler::t_destroy(CStrRef session_id) { + return m_mod->destroy(session_id->data()); +} + +bool c_SessionHandler::t_gc(int maxlifetime) { + int nrdels = -1; + return m_mod->gc(maxlifetime, &nrdels); +} + + /////////////////////////////////////////////////////////////////////////////// } diff --git a/hphp/runtime/ext/ext_session.h b/hphp/runtime/ext/ext_session.h index 817693cbb..2f7572029 100644 --- a/hphp/runtime/ext/ext_session.h +++ b/hphp/runtime/ext/ext_session.h @@ -29,7 +29,7 @@ void f_session_set_cookie_params(int64_t lifetime, CStrRef path = null_string, C Array f_session_get_cookie_params(); String f_session_name(CStrRef newname = null_string); Variant f_session_module_name(CStrRef newname = null_string); -bool f_session_set_save_handler(CStrRef open, CStrRef close, CStrRef read, CStrRef write, CStrRef destroy, CStrRef gc); +bool f_session_set_save_handler(CObjRef sessionhandler, bool register_shutdown = true); String f_session_save_path(CStrRef newname = null_string); String f_session_id(CStrRef newid = null_string); bool f_session_regenerate_id(bool delete_old_session = false); @@ -46,6 +46,33 @@ bool f_session_register(int _argc, CVarRef var_names, CArrRef _argv = null_array bool f_session_unregister(CStrRef varname); bool f_session_is_registered(CStrRef varname); +/////////////////////////////////////////////////////////////////////////////// +// class SessionHandler + +class SessionModule; + +FORWARD_DECLARE_CLASS_BUILTIN(SessionHandler); +class c_SessionHandler : public ExtObjectData { + public: + DECLARE_CLASS(SessionHandler, SessionHandler, ObjectData) + + // need to implement + public: c_SessionHandler(Class* cls = c_SessionHandler::s_cls); + public: ~c_SessionHandler(); + public: void t___construct(); + public: bool t_open(CStrRef save_path, CStrRef session_id); + public: bool t_close(); + public: String t_read(CStrRef session_id); + public: bool t_write(CStrRef session_id, CStrRef session_data); + public: bool t_destroy(CStrRef session_id); + public: bool t_gc(int maxlifetime); + + // implemented by HPHP + public: c_SessionHandler *create(); + + private: SessionModule* m_mod; +}; + /////////////////////////////////////////////////////////////////////////////// } diff --git a/hphp/system/idl/session.idl.json b/hphp/system/idl/session.idl.json index 1944e1ecf..879721ab6 100644 --- a/hphp/system/idl/session.idl.json +++ b/hphp/system/idl/session.idl.json @@ -97,45 +97,19 @@ ] }, { - "name": "session_set_save_handler", - "desc": "session_set_save_handler() sets the user-level session storage functions which are used for storing and retrieving data associated with a session. This is most useful when a storage method other than those supplied by PHP sessions is preferred. i.e. Storing the session data in a local database.", - "flags": [ - "HasDocComment" - ], + "name": "hphp_session_set_save_handler", "return": { - "type": "Boolean", - "desc": "Returns TRUE on success or FALSE on failure." + "type": "Boolean" }, "args": [ { - "name": "open", - "type": "String", - "desc": "Open function, this works like a constructor in classes and is executed when the session is being opened. The open function expects two parameters, where the first is the save path and the second is the session name." + "name": "sessionhandler", + "type": "Object" }, { - "name": "close", - "type": "String", - "desc": "Close function, this works like a destructor in classes and is executed when the session operation is done." - }, - { - "name": "read", - "type": "String", - "desc": "Read function must return string value always to make save handler work as expected. Return empty string if there is no data to read. Return values from other handlers are converted to boolean expression. TRUE for success, FALSE for failure." - }, - { - "name": "write", - "type": "String", - "desc": "Write function that is called when session data is to be saved. This function expects two parameters: an identifier and the data associated with it.\n\nThe \"write\" handler is not executed until after the output stream is closed. Thus, output from debugging statements in the \"write\" handler will never be seen in the browser. If debugging output is necessary, it is suggested that the debug output be written to a file instead." - }, - { - "name": "destroy", - "type": "String", - "desc": "The destroy handler, this is executed when a session is destroyed with session_destroy() and takes the session id as its only parameter." - }, - { - "name": "gc", - "type": "String", - "desc": "The garbage collector, this is executed when the session garbage collector is executed and takes the max session lifetime as its only parameter." + "name": "register_shutdown", + "type": "Boolean", + "value": "true" } ] }, @@ -384,5 +358,104 @@ } ], "classes": [ + { + "name": "SessionHandler", + "desc": "", + "ifaces": [ + "SessionHandlerInterface" + ], + "flags": [ + "HasDocComment" + ], + "funcs": [ + { + "name": "__construct", + "flags": [ + "HasDocComment" + ], + "return": { + "type": null + }, + "args": [ + ] + }, + { + "name": "open", + "return": { + "type": "Boolean" + }, + "args": [ + { + "name": "save_path", + "type": "String" + }, + { + "name": "session_id", + "type": "String" + } + ] + }, + { + "name": "close", + "return": { + "type": "Boolean" + }, + "args": [ + ] + }, + { + "name": "read", + "return": { + "type": "String" + }, + "args": [ + { + "name": "session_id", + "type": "String" + } + ] + }, + { + "name": "write", + "return": { + "type": "Boolean" + }, + "args": [ + { + "name": "session_id", + "type": "String" + }, + { + "name": "session_data", + "type": "String" + } + ] + }, + { + "name": "destroy", + "return": { + "type": "Boolean" + }, + "args": [ + { + "name": "session_id", + "type": "String" + } + ] + }, + { + "name": "gc", + "return": { + "type": "Boolean" + }, + "args": [ + { + "name": "maxlifetime", + "type": "Int32" + } + ] + } + ] + } ] -} \ No newline at end of file +} diff --git a/hphp/system/php.txt b/hphp/system/php.txt index 30e61fe6c..545a4053d 100644 --- a/hphp/system/php.txt +++ b/hphp/system/php.txt @@ -41,6 +41,9 @@ hphp/system/php/filter/filter_input.php hphp/system/php/filter/filter_var_array.php hphp/system/php/filter/filter_input_array.php +hphp/system/php/sessions/SessionHandlerInterface.php +hphp/system/php/sessions/session_set_save_handler.php + # If you have no inheritance relationship, go here in alphabetical order hphp/system/php/DebuggerCommand.php hphp/system/php/XhprofFrame.php diff --git a/hphp/system/php/sessions/SessionHandlerInterface.php b/hphp/system/php/sessions/SessionHandlerInterface.php new file mode 100644 index 000000000..ea7222833 --- /dev/null +++ b/hphp/system/php/sessions/SessionHandlerInterface.php @@ -0,0 +1,145 @@ +open = $this->validate($open, 1); + $this->close = $this->validate($close, 2); + $this->read = $this->validate($read, 3); + $this->write = $this->validate($write, 4); + $this->destroy = $this->validate($destroy, 5); + $this->gc = $this->validate($gc, 6); + } catch (Exception $e) { + trigger_error($e->getMessage(), E_USER_WARNING); + return false; + } + } + + public function open($save_path, $session_id) { + if ($this->open) { + return call_user_func($this->open, $save_path, $session_id); + } + } + public function close() { + if ($this->close) { + return call_user_func($this->close); + } + } + public function read($session_id) { + if ($this->read) { + return call_user_func($this->read, $session_id); + } + } + public function write($session_id, $session_data) { + if ($this->write) { + return call_user_func($this->write, $session_id, $session_data); + } + } + public function destroy($session_id) { + if ($this->destroy) { + return call_user_func($this->destroy, $session_id); + } + } + public function gc($maxlifetime) { + if ($this->gc) { + return call_user_func($this->gc, $maxlifetime); + } + } + private function validate($func, $num) { + if (!is_callable($func)) { + throw new Exception("Argument $num is not a valid callback"); + } + return $func; + } +} + +function session_set_save_handler( + $open, + $close = null, $read = null, $write = null, $destroy = null, $gc = null) { + if ($open instanceof SessionHandlerInterface) { + return hphp_session_set_save_handler($open, $close); + } + return hphp_session_set_save_handler( + new _SessionForwardingHandler($open, $close, $read, $write, $destroy, $gc) + ); +} diff --git a/hphp/test/slow/ext_session/EncryptedSessionHandler.php b/hphp/test/slow/ext_session/EncryptedSessionHandler.php new file mode 100644 index 000000000..003a1c409 --- /dev/null +++ b/hphp/test/slow/ext_session/EncryptedSessionHandler.php @@ -0,0 +1,29 @@ +key = $key; + } + + public function read($id) { + $data = parent::read($id); + var_dump($data); + return mcrypt_decrypt(MCRYPT_3DES, $this->key, $data, MCRYPT_MODE_ECB); + } + + public function write($id, $data) { + $data = mcrypt_encrypt(MCRYPT_3DES, $this->key, $data, MCRYPT_MODE_ECB); + var_dump($data); + return parent::write($id, $data); + } +} + +ini_set('session.save_handler', 'files'); +$handler = new EncryptedSessionHandler('mykey'); +session_set_save_handler($handler, true); +session_start(); + +$_SESSION['a'] = 'A'; +var_dump($_SESSION['a']); diff --git a/hphp/test/slow/ext_session/EncryptedSessionHandler.php.expect b/hphp/test/slow/ext_session/EncryptedSessionHandler.php.expect new file mode 100644 index 000000000..3b4420c90 --- /dev/null +++ b/hphp/test/slow/ext_session/EncryptedSessionHandler.php.expect @@ -0,0 +1,3 @@ +string(0) "" +string(1) "A" +string(16) "¤zBN÷¼£§¤¼T8¬k·" diff --git a/hphp/test/slow/ext_session/array_as_callback.php b/hphp/test/slow/ext_session/array_as_callback.php new file mode 100644 index 000000000..0b8f6fba6 --- /dev/null +++ b/hphp/test/slow/ext_session/array_as_callback.php @@ -0,0 +1,14 @@ + + string(1) "A" +} diff --git a/hphp/test/zend/bad/ext-session/sessionhandler_open_001.php b/hphp/test/zend/good/ext-session/sessionhandler_open_001.php similarity index 100% rename from hphp/test/zend/bad/ext-session/sessionhandler_open_001.php rename to hphp/test/zend/good/ext-session/sessionhandler_open_001.php diff --git a/hphp/test/zend/bad/ext-session/sessionhandler_open_001.php.expectf b/hphp/test/zend/good/ext-session/sessionhandler_open_001.php.expectf similarity index 100% rename from hphp/test/zend/bad/ext-session/sessionhandler_open_001.php.expectf rename to hphp/test/zend/good/ext-session/sessionhandler_open_001.php.expectf