Edit report at https://bugs.php.net/bug.php?id=63379&edit=1

 ID:                 63379
 Comment by:         ar...@php.net
 Reported by:        avatar2004-php at yahoo dot fr
 Summary:            Warning when using session_regenerate_id(TRUE) with
                     a SessionHandler
 Status:             Closed
 Type:               Bug
 Package:            Session related
 Operating System:   Gentoo
 PHP Version:        5.4.8
 Assigned To:        arpad
 Block user comment: N
 Private report:     N

 New Comment:

Laruence you were correct, the reset wasn't necessary :)

avatar2004, the flag is only meant to keep the parent session handler in a sane 
state. If a user implementation calls the parent handler in close() but not in 
open(), we need to be able to prevent the close() call on the parent handler 
even though session_status tells us that the session is open.

The flag is a global because having more than one session handler instance 
active in the same request is nonsensical.


Previous Comments:
------------------------------------------------------------------------
[2012-12-14 00:07:34] ar...@php.net

Automatic comment on behalf of array...@gmail.com
Revision: 
http://git.php.net/?p=php-src.git;a=commit;h=6566ea61732a1ab42c1a57e60adc96788cb0feb2
Log: Fix #63379 - Don't reset mod_user_is_open in destroy

------------------------------------------------------------------------
[2012-10-29 04:21:47] avatar2004-php at yahoo dot fr

Looking at the code, it feels like the very semantics of the mod_user_is_open 
flag are not exactly consistent.

The flag is a global, yet it is manipulated by instance code. Meaning if the 
user space code uses several handlers interchangeably, the result can quickly 
become confusing.

I suggest a decision should be made as to whether the SessionHandler is a 
stateless, thin wrapper for the handler calls in which case it should default 
to the same checks as the procedural API. Otherwise, the flag should really be 
an instance variable used to track the proper invocation sequence of the 
different callbacks and make sure THIS handler is open before calling dependent 
routines.

Adding an instance variable to the (base) class though is probably not worth 
it. The session management semantics are already defined by the procedural API 
which uses a global session state flag (session_status). If required for a 
handler implementation, the flag can simply be implemented in user space.

Just my 2c.

------------------------------------------------------------------------
[2012-10-29 03:07:41] larue...@php.net

I mean, maybe only reset it in close handler?

------------------------------------------------------------------------
[2012-10-29 03:07:08] larue...@php.net

is the reseting of user_is_open necessary?

diff --git a/ext/session/mod_user_class.c b/ext/session/mod_user_class.c
index 70d2f40..4edac28 100644
--- a/ext/session/mod_user_class.c
+++ b/ext/session/mod_user_class.c
@@ -121,7 +121,6 @@ PHP_METHOD(SessionHandler, destroy)
                return;
        }
        
-       PS(mod_user_is_open) = 0;
        RETVAL_BOOL(SUCCESS == PS(default_mod)->s_destroy(&PS(mod_data), key 
TSRMLS_CC));
 }
 /* }}} */

------------------------------------------------------------------------
[2012-10-29 01:14:11] avatar2004-php at yahoo dot fr

If I understand correctly and "mod_user_is_open" is just a global state 
parameter used by the SessionHandler instance, I was wondering why the check 
wasn't being done on "session_status" instead to conform with the rest of the 
session_* API ?

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=63379


-- 
Edit this bug report at https://bugs.php.net/bug.php?id=63379&edit=1

Reply via email to