Hi all,

If you have comments, please feedback before I apply
this patch.

Attached patch fixes many problems in session module.
It also breaks some of script which barely worked before.

This patch fixes:
  - Crashes are caused by invlaid save_path, invalid
    session id name, return value from user defined session function.
    There may be other crashes observed that I don't know.
  - Broken mm save handler.
  - Sticky session module name
  - Change error level for failure to open session.
        (E_ERROR -> E_WARNING)
  - Dead lock with files handler
  - Prevent calling session_name() and session_set_cookie_parameter()
    once session is started. It doesn't work after session is started,
    anyways.
  - Prevent to set module name after session is started. (This causes
    dead lock with files hadler. It may do something bad for other save
    handlers also. It should be prevented, IMO.)

The last fix will break some scripts.

-- 
Yasuo Ohgaki

? ext/session/tmp.diff
? ext/session/bak
Index: ext/session/mod_files.c
===================================================================
RCS file: /repository/php4/ext/session/mod_files.c,v
retrieving revision 1.67
diff -u -r1.67 mod_files.c
--- ext/session/mod_files.c     3 Feb 2002 05:40:19 -0000       1.67
+++ ext/session/mod_files.c     3 Feb 2002 09:06:56 -0000
@@ -16,7 +16,7 @@
    +----------------------------------------------------------------------+
  */
 
-/* $Id: mod_files.c,v 1.67 2002/02/03 05:40:19 yohgaki Exp $ */
+/* $Id: mod_files.c,v 1.66 2002/02/03 03:17:35 yohgaki Exp $ */
 
 #include "php.h"
 
@@ -123,7 +123,7 @@
        }
 }
 
-static void ps_files_open(ps_files *data, const char *key)
+static int ps_files_open(ps_files *data, const char *key)
 {
        char buf[MAXPATHLEN];
        TSRMLS_FETCH();
@@ -138,7 +138,7 @@
                
                if (!ps_files_valid_key(key) || 
                                !ps_files_path_create(buf, sizeof(buf), data, key))
-                       return;
+                       return FAILURE;
                
                data->lastkey = estrdup(key);
                
@@ -153,10 +153,13 @@
                if (data->fd != -1) 
                        flock(data->fd, LOCK_EX);
 
-               if (data->fd == -1)
+               if (data->fd == -1) {
                        php_error(E_WARNING, "open(%s, O_RDWR) failed: %s (%d)", buf, 
                                        strerror(errno), errno);
+                       return FAILURE;
+               }
        }
+       return SUCCESS;
 }
 
 static int ps_files_cleanup_dir(const char *dirname, int maxlifetime)
@@ -254,7 +257,9 @@
        struct stat sbuf;
        PS_FILES_DATA;
 
-       ps_files_open(data, key);
+       if (ps_files_open(data, key) == FAILURE)
+               return FAILURE;
+       
        if (data->fd < 0)
                return FAILURE;
        
@@ -283,7 +288,9 @@
        long n;
        PS_FILES_DATA;
 
-       ps_files_open(data, key);
+       if (ps_files_open(data, key) == FAILURE)
+               return FAILURE;
+
        if (data->fd < 0)
                return FAILURE;
 
Index: ext/session/mod_mm.c
===================================================================
RCS file: /repository/php4/ext/session/mod_mm.c,v
retrieving revision 1.31
diff -u -r1.31 mod_mm.c
--- ext/session/mod_mm.c        25 Jan 2002 20:59:24 -0000      1.31
+++ ext/session/mod_mm.c        3 Feb 2002 09:06:56 -0000
@@ -319,7 +319,6 @@
 {
        PS_MM_DATA;
        ps_sd *sd;
-       int ret = FAILURE;
 
        mm_lock(data->mm, MM_LOCK_RD);
        
@@ -329,12 +328,14 @@
                *val = emalloc(sd->datalen + 1);
                memcpy(*val, sd->data, sd->datalen);
                (*val)[sd->datalen] = '\0';
-               ret = SUCCESS;
        }
-
+       else {
+               *val = NULL;
+       }
+       
        mm_unlock(data->mm);
        
-       return ret;
+       return SUCCESS;
 }
 
 PS_WRITE_FUNC(mm)
Index: ext/session/php_session.h
===================================================================
RCS file: /repository/php4/ext/session/php_session.h,v
retrieving revision 1.67
diff -u -r1.67 php_session.h
--- ext/session/php_session.h   16 Jan 2002 04:56:35 -0000      1.67
+++ ext/session/php_session.h   3 Feb 2002 09:06:56 -0000
@@ -79,6 +79,7 @@
        char *cookie_path;
        char *cookie_domain;
        zend_bool  cookie_secure;
+       long rinit_mod;
        ps_module *mod;
        void *mod_data;
        HashTable vars;
Index: ext/session/session.c
===================================================================
RCS file: /repository/php4/ext/session/session.c,v
retrieving revision 1.273
diff -u -r1.273 session.c
--- ext/session/session.c       3 Feb 2002 05:40:19 -0000       1.273
+++ ext/session/session.c       3 Feb 2002 09:06:57 -0000
@@ -17,7 +17,7 @@
    +----------------------------------------------------------------------+
  */
 
-/* $Id: session.c,v 1.273 2002/02/03 05:40:19 yohgaki Exp $ */
+/* $Id: session.c,v 1.272 2002/02/03 03:17:35 yohgaki Exp $ */
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -549,13 +549,15 @@
        int vallen;
        
        if (PS(mod)->open(&PS(mod_data), PS(save_path), PS(session_name)) == FAILURE) {
-               php_error(E_ERROR, "Failed to initialize session module");
+               php_error(E_WARNING, "Failed to initialize session module");
                return;
        }
-       if (PS(mod)->read(&PS(mod_data), PS(id), &val, &vallen) == SUCCESS) {
-               php_session_decode(val, vallen TSRMLS_CC);
-               efree(val);
+       if (PS(mod)->read(&PS(mod_data), PS(id), &val, &vallen) == FAILURE) {
+               php_error(E_WARNING, "Failed to read session data");
        }
+       php_session_decode(val, vallen TSRMLS_CC);
+       if (val)
+               efree(val);
 }
 
 
@@ -950,7 +952,7 @@
 
        if (PS(mod_data) && PS(gc_probability) > 0) {
                int nrdels = -1;
-
+               
                nrand = (int) (100.0*php_combined_lcg(TSRMLS_C));
                if (nrand < PS(gc_probability)) {
                        PS(mod)->gc(&PS(mod_data), PS(gc_maxlifetime), &nrdels);
@@ -962,6 +964,7 @@
        }
 }
 
+
 static zend_bool php_session_destroy(TSRMLS_D)
 {
        zend_bool retval = SUCCESS;
@@ -1000,18 +1003,26 @@
        PS(cookie_lifetime) = Z_LVAL_PP(lifetime);
 
        if (ZEND_NUM_ARGS() > 1) {
-               convert_to_string_ex(path);
-               zend_alter_ini_entry("session.cookie_path", 
sizeof("session.cookie_path"), Z_STRVAL_PP(path), Z_STRLEN_PP(path), PHP_INI_USER, 
PHP_INI_STAGE_RUNTIME);
-
-               if (ZEND_NUM_ARGS() > 2) {
-                       convert_to_string_ex(domain);
-                       zend_alter_ini_entry("session.cookie_domain", 
sizeof("session.cookie_domain"), Z_STRVAL_PP(domain), Z_STRLEN_PP(domain), 
PHP_INI_USER, PHP_INI_STAGE_RUNTIME);
-                       if (ZEND_NUM_ARGS() > 3) {
-                               convert_to_long_ex(secure);
-                               zend_alter_ini_entry("session.cookie_secure", 
sizeof("session.cookie_secure"), Z_BVAL_PP(secure)?"1":"0", 1, PHP_INI_USER, 
PHP_INI_STAGE_RUNTIME);
+               if (PS(session_status) == php_session_none || PS(session_status) == 
+php_session_disabled) {
+                       convert_to_string_ex(path);
+                       zend_alter_ini_entry("session.cookie_path", 
+sizeof("session.cookie_path"), Z_STRVAL_PP(path), Z_STRLEN_PP(path), PHP_INI_USER, 
+PHP_INI_STAGE_RUNTIME);
+                       
+                       if (ZEND_NUM_ARGS() > 2) {
+                               convert_to_string_ex(domain);
+                               zend_alter_ini_entry("session.cookie_domain", 
+sizeof("session.cookie_domain"), Z_STRVAL_PP(domain), Z_STRLEN_PP(domain), 
+PHP_INI_USER, PHP_INI_STAGE_RUNTIME);
+                               if (ZEND_NUM_ARGS() > 3) {
+                                       convert_to_long_ex(secure);
+                                       zend_alter_ini_entry("session.cookie_secure", 
+sizeof("session.cookie_secure"), Z_BVAL_PP(secure)?"1":"0", 1, PHP_INI_USER, 
+PHP_INI_STAGE_RUNTIME);
+                               }
                        }
                }
+               else {
+                       php_error(E_WARNING,"%s() cannot set cookie parameter once 
+session is started.",
+                                         get_active_function_name(TSRMLS_C));
+                       RETURN_FALSE;
+               }
        }
+       
 }
 /* }}} */
 
@@ -1049,8 +1060,15 @@
                WRONG_PARAM_COUNT;
 
        if (ac == 1) {
-               convert_to_string_ex(p_name);
-               zend_alter_ini_entry("session.name", sizeof("session.name"), 
Z_STRVAL_PP(p_name), Z_STRLEN_PP(p_name), PHP_INI_USER, PHP_INI_STAGE_RUNTIME);
+               if (PS(session_status) == php_session_none || PS(session_status) == 
+php_session_disabled) {
+                       convert_to_string_ex(p_name);
+                       zend_alter_ini_entry("session.name", sizeof("session.name"), 
+Z_STRVAL_PP(p_name), Z_STRLEN_PP(p_name), PHP_INI_USER, PHP_INI_STAGE_RUNTIME);
+               }
+               else {
+                       php_error(E_WARNING, "%s() cannot set session name once 
+session is started.",
+                                         get_active_function_name(TSRMLS_C));
+                       RETURN_FALSE;
+               }
        }
        
        RETVAL_STRING(old, 0);
@@ -1071,19 +1089,30 @@
                WRONG_PARAM_COUNT;
 
        if (ac == 1) {
-               ps_module *tempmod;
-
-               convert_to_string_ex(p_name);
-               tempmod = _php_find_ps_module(Z_STRVAL_PP(p_name) TSRMLS_CC);
-               if (tempmod) {
-                       if (PS(mod_data))
-                               PS(mod)->close(&PS(mod_data));
-                       PS(mod) = tempmod;
-                       PS(mod_data) = NULL;
-               } else {
+               if (PS(session_status) == php_session_none || PS(session_status) == 
+php_session_disabled) {
+                       ps_module *tempmod;
+                       
+                       convert_to_string_ex(p_name);
+                       tempmod = _php_find_ps_module(Z_STRVAL_PP(p_name) TSRMLS_CC);
+                       if (tempmod) {
+                               if (PS(mod_data))
+                                       PS(mod)->close(&PS(mod_data));
+                               PS(mod) = tempmod;
+                               PS(mod_data) = NULL;
+                               PS(rinit_mod) = 1;
+                       } else {
+                               efree(old);
+                               php_error(E_ERROR, "Cannot find named PHP session 
+module (%s)",
+                                                 Z_STRVAL_PP(p_name));
+                               RETURN_FALSE;
+                       }
+               }
+               else {
                        efree(old);
-                       php_error(E_ERROR, "Cannot find named PHP session module (%s)",
-                                       Z_STRVAL_PP(p_name));
+                       php_error(E_WARNING, "%s() cannot set session module name once 
+session is started. "
+                                         "Current session save handler (%s)",
+                                         get_active_function_name(TSRMLS_C),
+                                         (PS(mod)->name ? PS(mod)->name : "none"));
                        RETURN_FALSE;
                }
        }
@@ -1181,8 +1210,15 @@
                WRONG_PARAM_COUNT;
 
        if (ac == 1) {
-               convert_to_string_ex(p_cache_limiter);
-               zend_alter_ini_entry("session.cache_limiter", 
sizeof("session.cache_limiter"), Z_STRVAL_PP(p_cache_limiter), 
Z_STRLEN_PP(p_cache_limiter), PHP_INI_USER, PHP_INI_STAGE_RUNTIME);
+               if (PS(session_status) == php_session_none || PS(session_status) == 
+php_session_disabled) {
+                       convert_to_string_ex(p_cache_limiter);
+                       zend_alter_ini_entry("session.cache_limiter", 
+sizeof("session.cache_limiter"), Z_STRVAL_PP(p_cache_limiter), 
+Z_STRLEN_PP(p_cache_limiter), PHP_INI_USER, PHP_INI_STAGE_RUNTIME);
+               }
+               else {
+                       php_error(E_WARNING, "%s() cannot set session module name once 
+session is started.",
+                                         get_active_function_name(TSRMLS_C));
+                       RETURN_FALSE;
+               }
        }
        
        RETVAL_STRING(old, 0);
@@ -1424,8 +1460,9 @@
 {
        php_rinit_session_globals(TSRMLS_C);
 
-       if (PS(mod) == NULL) {
+       if (PS(mod) == NULL || PS(rinit_mod)) {
                char *value;
+               PS(rinit_mod) = 0;
 
                value = zend_ini_string("session.save_handler", 
sizeof("session.save_handler"), 0);
                if (value) {

-- 
PHP Development Mailing List <http://www.php.net/>
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to