Hi all,
Current session module has a few limitations due to "register_globals"
support which is now obsolete. One of them is numeric key indexed session
data. i.e. $_SESSION[1] = $var is not allowed now and it raises error at
R_SHUTDOWN with useless message for debugging.
https://bugs.php.net/bug.php?id=65359
Following patch (diff against 5.5 branch) add php_serialize session
serialize handler and removes the limitation. To test patch, set
"session.serialize_handler = php_serialize" in php.ini file or set it via
ini_set().
https://gist.github.com/yohgaki/6171628
(All tests and my new test for new serializer passed with Valgrind)
It's possible to remove unneeded limitation from current serializers, but
it causes BC. (e.g. AFAIK, perl and python have serializer/unserializer for
PHP session data. If we simply remove limitation, session data decode may
fail.) Therefore, adding new serializer would be better.
Since it uses plain serialize(), users may simply unserialize() to inspect
session data if it is needed. Users may setup initial $_SESSION array from
serialize()ed data. i.e. session_decode($serialized_data) These would be
useful for some users.
I would like to add this new serializer to 5.5(optional) and
master(default).
Any comments?
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi!
Current session module has a few limitations due to "register_globals"
support which is now obsolete. One of them is numeric key indexed session
data. i.e. $_SESSION[1] = $var is not allowed now and it raises error at
R_SHUTDOWN with useless message for debugging.
Why is it useful to do this? I don't see how using numerical indexes in
global namespace (and SESSION is one of global namespaces in PHP) is a
good idea. Could you explain the use case here?
This seems to be to be of a very limited use and introducing new
serializer format with all resulting BC problems and confusion doesn't
seem to be worth it.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi Stas,
On Thu, Aug 8, 2013 at 5:34 AM, Stas Malyshev smalyshev@sugarcrm.comwrote:
Why is it useful to do this? I don't see how using numerical indexes in
global namespace (and SESSION is one of global namespaces in PHP) is a
good idea. Could you explain the use case here?
Personally, I would not use numeric index.
However, users are expecting it to work and there is no way to raise
informative error message where is wrong. IIRC, there were description
about "register_globals" originated limitations in our documents, but it
seems it was gone already.
This seems to be to be of a very limited use and introducing new
serializer format with all resulting BC problems and confusion doesn't
seem to be worth it.
The limitation is come from register_globals and it has nasty problem.
i.e. Error occurred at unknown script line 0.
This kind of behavior is better to be removed and it's worth it. IMHO.
Most users don't care what serializer does and users who may have
BC issue can use old serializers anytime.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi!
Personally, I would not use numeric index.
However, users are expecting it to work and there is no way to raise
I don't think it is reasonable to expect it to work, it never worked, it
never was documented to work and there's no real use case for it.
The limitation is come from register_globals and it has nasty problem.
I don't see how is it a nasty problem if there's actually no good reason
to do this. If there is a good reason to do it, please tell what it is.
Most users don't care what serializer does and users who may have
BC issue can use old serializers anytime.
Using old serializer is work which we would make users do for
improvement that practically nobody needs and that does not provide any
actual improvement that can not easily be achieved otherwise. Breaking
BC just because somebody somewhere needs an exotic feature that can
easily be worked around doesn't look like a good idea to me.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi Stas
On Thu, Aug 8, 2013 at 10:03 AM, Stas Malyshev smalyshev@sugarcrm.comwrote:
Personally, I would not use numeric index.
However, users are expecting it to work and there is no way to raiseI don't think it is reasonable to expect it to work, it never worked, it
never was documented to work and there's no real use case for it.
I also think this limitation was documented, but it seems it gone (or
I missed)
The limitation is come from register_globals and it has nasty problem.
I don't see how is it a nasty problem if there's actually no good reason
to do this. If there is a good reason to do it, please tell what it is.
PHP Notice: Unknown: Skipping numeric key 1 in Unknown on line 0
https://bugs.php.net/bug.php?id=65359
This error occurs at shutdown. Removing unneeded limitation is good reason
IMHO. If there is
$_SESSION[$key] = $var;
It's very hard to find what is and where is wrong.
Most users don't care what serializer does and users who may have
BC issue can use old serializers anytime.Using old serializer is work which we would make users do for
improvement that practically nobody needs and that does not provide any
actual improvement that can not easily be achieved otherwise. Breaking
BC just because somebody somewhere needs an exotic feature that can
easily be worked around doesn't look like a good idea to me.
I should have mentioned that new serializer not only allow numeric index,
but
also allow special characters due to the implementation. i.e. ! and | may
be
used with the new serializer.
Removing unneeded limitations, rather than forcing them to users, is user
friendly and the way to go. IMHO.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi!
Removing unneeded limitations, rather than forcing them to users, is user
friendly and the way to go. IMHO.
If we wrote it from scratch, sure. But if we already have existing and
working one, having people to deal with migrating data and
incompatibilities that arise IMHO is not worth the additional benefit of
having session variable named "!", without which one can live quite well.
So I'm not against having option for new serializer that does it if
somebody needs it, but I think changing the default for this and the
disruption potentially caused by it is not a very good idea.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi Stas,
On Thu, Aug 8, 2013 at 11:03 AM, Stas Malyshev smalyshev@sugarcrm.comwrote:
Removing unneeded limitations, rather than forcing them to users, is user
friendly and the way to go. IMHO.If we wrote it from scratch, sure. But if we already have existing and
working one, having people to deal with migrating data and
incompatibilities that arise IMHO is not worth the additional benefit of
having session variable named "!", without which one can live quite well.
So I'm not against having option for new serializer that does it if
somebody needs it, but I think changing the default for this and the
disruption potentially caused by it is not a very good idea.
I agree. Breaking existing apps w/o good reason must be avoided. Therefore,
I proposed that introduce new serializer for 5.5 as optional. Do you
agree to have new serializer for 5.6 as optional and make it default
for future releases?
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Stas,
On Thu, Aug 8, 2013 at 11:03 AM, Stas Malyshev smalyshev@sugarcrm.comwrote:
Removing unneeded limitations, rather than forcing them to users, is
user
friendly and the way to go. IMHO.If we wrote it from scratch, sure. But if we already have existing and
working one, having people to deal with migrating data and
incompatibilities that arise IMHO is not worth the additional benefit of
having session variable named "!", without which one can live quite well.
So I'm not against having option for new serializer that does it if
somebody needs it, but I think changing the default for this and the
disruption potentially caused by it is not a very good idea.I agree. Breaking existing apps w/o good reason must be avoided. Therefore,
I proposed that introduce new serializer for 5.5 as optional. Do you
agree to have new serializer for 5.6 as optional and make it default
for future releases?
Or make it optional for 5.5 and 5.6, write full documentation about
serializer
and make new serializer default for future releases.
It would work better. There would be enough time for users may have BC
issue. If they do, it would be simple task to adopt because it is plain
serialize().
What do you think?
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Yasuo Ohgaki wrote:
Or make it optional for 5.5 and 5.6, write full documentation about
serializer
and make new serializer default for future releases.It would work better. There would be enough time for users may have BC
issue. If they do, it would be simple task to adopt because it is plain
serialize().What do you think?
The one thing that PHP used to have was consistency. Applications that were
designed on PHP4 worked on PHP5, and those developed on PHP5 could be made
backwards compatible. This allowed sites to run and run without any major
reworking. Then E_STRICT came along, and while it can be 'switched off',
ignoring it creates many more problems. At what point will these fixes simply be
dropped altogether, and the remaining legacy code HAS to be converted? If one
was starting again today would things be done the same way? Obviously not, but
this continual picking away at core functionality is a further distraction from
keeping sites working? I'm currently battling with owncloud which works
perfectly on Apache 2.2, but crashes without ANY error messages on Apache 2.4.
I've asked for help on the Apache list and basically been told to 'piss off' as
I often am here! But the people who keep pushing all these little changes 'just
to make things better' need to start helping fix the breaks. phpdocs is still
not producing good documentation for many of the namespace changes and other
extras included in PHP5.3 let alone 5.4, and now other changes are being
proposed that would require further rework on phpdocs and Reflections? The
current builds of phpdocs do not actually yet produce output that matches the
previous version following a complete rework of that which now requires many
days of work simply to understand the new code base :(
We need to help get the rest of the infrastructure working with PHP5.4 before
starting work on 5.6 ... let alone 5.5 ...
--
Lester Caine - G8HFL
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
Hi Lester,
We need to help get the rest of the infrastructure working with PHP5.4
before starting work on 5.6 ... let alone 5.5 ...
Since 5.5 is just released, some fixes may be good to add.
However, I agree that released version is better to keep the
feature sets as it is released.
I'll just commit this to master branch and document it as 5.6
feature.
Any more comments?
--
Yasuo Ohgaki
yohgaki@ohgaki.net