Hi Stas,
On Wed, Oct 19, 2016 at 3:08 PM, Stanislav Malyshev <smalys...@gmail.com> wrote:
>> I pushed patch fixes number of nonsense/inconsistent session function
>> behaviors. The additional patch is pushed so that it's easy to cherry
>> pick minimum fixes. The last push is the additional fixes.
>
> These changes look like a reasonable cleanup. I'm not a big fan of
> zend_parse_parameters_none additions, since I don't see any useful
> function for them, but they don't hurt either. I left some notes on the
> patch though, please look there.
>
> I'm not sure about 7.1 - while I don't see anything that would break any
> reasonable code, except issues noted on the pull, the patch is pretty
> big and I might have missed something. I guess it's for RM to decide. No
> objection for 7.2.
>
> I notice that most test changes deal with scenarios that were covered
> before, are there tests that test the new things - the scenarios for
> which handling has changed? I couldn't find tests for some cases, like
> INI modification, would be nice to add them.
>
> Also, a reminder that you'd have to update all the documentation in the
> manual to reflect the change in the return values. And UPGRADING too.
Thank you.
I replied to your comments.
There is one debatable comment regarding session_cache_limiter().
We may delay error check at 'headers sent'. I suppose almost all usage
is misuse.
Only valid use case is
<?php
ob_start();
session_start();
session_set_cache_limiter('public'); // <== Call this between
session_start() and session_regenerate_id()
session_regenerate_id();
?>
Other than this, all calls after session_start() is invalid. Valid use
case is very limited and unlikely. If users have to change cache
limiter, they can session_commit(), then session_start(). So I would
like to keep as it is now.
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
