On October 2, 2002 05:13 am, Sascha Schumann wrote:
>     Could some kind soul with a thorough understanding of the
>     engine details please review the attached patch?
>
>     Changes:
>
>     - Fix two simple memory leaks.
>     - Fix a bug which did not initialize $_SESSION, if
>       is_array($HTTP_SESSION_VARS) was true.
>
>     - Sascha

The patch does solve the problems it sets out to fix, most important of which 
in my opinion is the memory leak. There is however another problem, which I 
believe should be addressed. The problem being that session_register() 
function does not work unless the user has register_globals enabled.
This particular problem causes problems for anyone using php software that was 
written before this limitation was added. In fact the sessions tests inside 
ext/session/tests fail for that very reason.
To this affect, I have made a few modifications to the patch written by Sascha 
that address this problem. After the patch is applied, register_session() 
works regardless of whether or not register_globals are enabled or disabled.

Ilia
Index: session.c
===================================================================
RCS file: /repository/php4/ext/session/session.c,v
retrieving revision 1.323
diff -u -3 -p -u -r1.323 session.c
--- session.c	1 Oct 2002 11:59:45 -0000	1.323
+++ session.c	2 Oct 2002 14:48:27 -0000
@@ -241,6 +241,7 @@ typedef struct {
 void php_add_session_var(char *name, size_t namelen TSRMLS_DC)
 {
 	zval **sym_track = NULL;
+	zval **sym_global = NULL;
 	
 	zend_hash_find(Z_ARRVAL_P(PS(http_session_vars)), name, namelen + 1, 
 			(void *) &sym_track);
@@ -249,29 +250,25 @@ void php_add_session_var(char *name, siz
 	 * Set up a proper reference between $_SESSION["x"] and $x.
 	 */
 
-	if (PG(register_globals)) {
-		zval **sym_global = NULL;
-		
-		zend_hash_find(&EG(symbol_table), name, namelen + 1, 
-				(void *) &sym_global);
+	zend_hash_find(&EG(symbol_table), name, namelen + 1, (void *) &sym_global);
 				
-		if (sym_global == NULL && sym_track == NULL) {
-			zval *empty_var;
+	if (sym_global == NULL && sym_track == NULL) {
+		zval *empty_var;
 
-			ALLOC_INIT_ZVAL(empty_var);
-			zend_set_hash_symbol(empty_var, name, namelen, 1, 2, Z_ARRVAL_P(PS(http_session_vars)), &EG(symbol_table));
-		} else if (sym_global == NULL) {
-			zend_set_hash_symbol(*sym_track, name, namelen, 1, 1, &EG(symbol_table));
-		} else if (sym_track == NULL) {
-			zend_set_hash_symbol(*sym_global, name, namelen, 1, 1, Z_ARRVAL_P(PS(http_session_vars)));
-		}
-	} else {
-		if (sym_track == NULL) {
-			zval *empty_var;
-	
-			ALLOC_INIT_ZVAL(empty_var);
-			zend_set_hash_symbol(empty_var, name, namelen, 0, 1, Z_ARRVAL_P(PS(http_session_vars)));
-		}
+		ALLOC_INIT_ZVAL(empty_var); /* this sets refcount to 1 */
+		ZVAL_DELREF(empty_var); /* our module does not maintain a ref */
+		/* The next call will increase refcount by NR_OF_SYM_TABLES==2 */
+		if (PG(register_globals)) {
+			zend_set_hash_symbol(empty_var, name, namelen, 1, 2,
+				Z_ARRVAL_P(PS(http_session_vars)), &EG(symbol_table));
+		} else {
+			zend_set_hash_symbol(empty_var, name, namelen, 1, 1,
+				Z_ARRVAL_P(PS(http_session_vars)));
+		}	
+	} else if (sym_global == NULL) {
+		zend_set_hash_symbol(*sym_track, name, namelen, 1, 1, &EG(symbol_table));
+	} else if (sym_track == NULL) {
+		zend_set_hash_symbol(*sym_global, name, namelen, 1, 1, Z_ARRVAL_P(PS(http_session_vars)));
 	}
 }
 
@@ -465,23 +462,16 @@ break_outer_loop:
 
 static void php_session_track_init(TSRMLS_D)
 {
-	zval **old_vars = NULL;
+	/* Unconditionally destroy existing arrays -- possible dirty data */
+	zend_hash_del(&EG(symbol_table), "HTTP_SESSION_VARS", 
+			sizeof("HTTP_SESSION_VARS"));
+	zend_hash_del(&EG(symbol_table), "_SESSION", sizeof("_SESSION"));
 
-	if (zend_hash_find(&EG(symbol_table), "HTTP_SESSION_VARS", sizeof("HTTP_SESSION_VARS"), (void **)&old_vars) == SUCCESS && Z_TYPE_PP(old_vars) == IS_ARRAY) {
-		PS(http_session_vars) = *old_vars;
-		zend_hash_clean(Z_ARRVAL_P(PS(http_session_vars)));
-	} else {
-		if(old_vars) {
-			zend_hash_del(&EG(symbol_table), "HTTP_SESSION_VARS", sizeof("HTTP_SESSION_VARS"));
-			zend_hash_del(&EG(symbol_table), "_SESSION", sizeof("_SESSION"));
-		}
-		MAKE_STD_ZVAL(PS(http_session_vars));
-		array_init(PS(http_session_vars));
-		PS(http_session_vars)->refcount = 2;
-		PS(http_session_vars)->is_ref = 1;
-		zend_hash_update(&EG(symbol_table), "HTTP_SESSION_VARS", sizeof("HTTP_SESSION_VARS"), &PS(http_session_vars), sizeof(zval *), NULL);
-		zend_hash_update(&EG(symbol_table), "_SESSION", sizeof("_SESSION"), &PS(http_session_vars), sizeof(zval *), NULL);
-	}
+	MAKE_STD_ZVAL(PS(http_session_vars));
+	array_init(PS(http_session_vars));
+		
+	ZEND_SET_GLOBAL_VAR("HTTP_SESSION_VARS", PS(http_session_vars));
+	ZEND_SET_GLOBAL_VAR("_SESSION", PS(http_session_vars));
 }
 
 static char *php_session_encode(int *newlen TSRMLS_DC)

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

Reply via email to