Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:96390
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain ohgaki.net designates 180.42.98.130 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAGa2bXYq=656z9eq-b=agpqJmojDn+DpDt1zfLLc_QTcFcLt8g@mail.gmail.com>
References: <CAGa2bXYq=656z9eq-b=agpqJmojDn+DpDt1zfLLc_QTcFcLt8g@mail.gmail.com>
Date: Mon, 17 Oct 2016 10:18:43 +0900
Message-ID: <CAGa2bXaGg0PocgtJEQ2Xhi6Us4F=ncEN-6zhZKEjGE1UuSoQ5A@mail.gmail.com>
To: Davey Shafik <davey@php.net>, "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: Fixing insane session_start() behaviors
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

Hi Davey,

On Sun, Oct 16, 2016 at 6:08 PM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
>
> I was planning to fix session_start() behaviors by PHP 7.1, but I
> forgot to do this completely. Partial fix is merged currently.
>
> Following PR makes session_start() return FALSE when it cannot start
> session always.
>
> https://github.com/php/php-src/pull/2167
>
> In short, this patch fixes number of session_start()'s insane behaviors.
>
> Original session_start() is designed to continue execution as much as
> it can. This design caused a lot of issues including number of crash
> bugs. We've removed most issues caused by this design, but
> session_start() behaves insane way. e.g. Return TRUE and initializes
> $_SESSION array for useless session, improper error messages, memory
> leak, etc. (Please verify phpt changes how this patch makes
> session_start() behave sane way)
>
> This fix may change app behavior. However, it changes behavior only
> when there is useless session which is fatal anyway. Therefore, it
> could be applied to PHP 7.1. IMO.
>
> What do you think?

Since session_start() is made to set proper session status finally,
many bugs and inconsistencies can be fixed altogether.

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.

 - Disallow nonsense function usage.
   . Do not allow to change cookie parameters when it has no effects.
     session_set_cookie_params() session_cache_limiter()
   . Dn not allow to change INI parameters when it cannot be changed.
     session_name() session_module_name() session_save_path()
session_set_save_handler() session_cache_expire()
   . Do not allow parameter for void parameter functions.
     session_unset() session_write_close() session_commit()
session_abort() session_reset()
 - Return function status as it should.
    . session_abort()  session_flush() session_commit()
session_write_close() session_reset() session_destroy()
 - Raise proper errors.
    . Many functions.

Session module allows insane usage currently. This will fix most of them.
I need to address nonsense INI usage. There will be one more push at
least for this.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net