Hi!
My main concern with this change is that it is binary incompatible with
existing session implementation, which means it would be hard to get it
into 5.3 and 5.4. While I understand sometimes adding handlers is
inevitable, in this case I'm not sure why it is required. So far the
only reason I understand was "If we do this, users cannot know if
ps_modules is new one that prevents adoption or not, especially for
third party ps_modules." but this is definitely a documentation issue
and even with new API you can not really know that - you just know
there's a handler but you can not know the module implements it properly
- maybe it just always returns OK.
Maybe we can make separate binary-compatible patch for 5.3/5.4 branches
and patch with new APIs for trunk? The user-space handlers for 5.3/5.4
would still have to do the checks (though stock modules will have the
checks built-in) but for trunk they could do it by defining validation
callbacks.
I also still not sure why default SID creation function was changed to 3
identical cloned functions. I do not think it's the right way - if some
macro is not allowing us to leave it as is, we can have another macro,
but I do not see how splitting one standard SID creation function into a
number of identical clones is beneficial. I think it needs to be changed.
For user module, with the patch if you add validation handler you also
have to add create_sid handler - which most users don't even want to
change. We should allow for using standard SID creation functions there
(maybe by passing NULL there? or by having some function that implements
php_session_create_id, maybe even better), and we shouldn't check
validate_sid when we mean create_sid - if we allow to set create_sid, we
should also support case where we set create_sid but not validate_sid.
Also, all other modules when SID creation fails set invalid_sid and
return NULL, while new user module SID creation returns "" and doesn't
set invalid_sid. Why is that?
We also probably need to deal then with the question what should be done
when create_sid fails (I'd say we can not do much but issue fatal error
but maybe there are any better ideas) if we allow it to fail. We don't
seem to check invalid_sid now after create_sid, so if we're setting it,
we must check it.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
