Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:73190
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.217.171 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <CAPhkiZw5xWRkc=ofh-nB5ZkjfThZFkH_+pK5RZaQFLKhno7u7A@mail.gmail.com>
References: <CAPhkiZw5xWRkc=ofh-nB5ZkjfThZFkH_+pK5RZaQFLKhno7u7A@mail.gmail.com>
Date: Sun, 16 Mar 2014 14:32:54 +0900
Message-ID: <CAGa2bXaL99PgxuWr7eTC89Ltc914UfxCgjWNqHsnnSJZyTUyPQ@mail.gmail.com>
To: Andrey Andreev <narf@devilix.net>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a11c2b3f0e2182c04f4b2a4e3
Subject: Re: [PHP-DEV] [RFC] Revert/extend/postpone original RFC about
 read_only, lazy_write sessions
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

--001a11c2b3f0e2182c04f4b2a4e3
Content-Type: text/plain; charset=UTF-8

Hi Andrey,

I've finished modifying patch, so I read your RFC.

On Sun, Mar 16, 2014 at 3:24 AM, Andrey Andreev <narf@devilix.net> wrote:

> I'm announcing the following RFC for discussion, with the hope that it
> can get through before the PHP 5.6 release:
> https://wiki.php.net/rfc/session-read_only-lazy_write
>
> As noted in it, I don't feel like
> https://wiki.php.net/rfc/session-lock-ini was handled properly. Lack
> of attention to it alone is demonstrated by the fact that a total of
> only 10 people have voted. I hope that this follow-up receives more
> attention, so that we can avoid a potential mess.
>

// $_SESSION['logged_in'] has been set in a previous request
Request 1: session_start(['read_only' => TRUE]);
Request 2: session_start(); unset($_SESSION['logged_in']);
session_write_close();
Request 1: if ($_SESSION['logged_in']) { /* logic */ } // evaluates to TRUE


Your example ends up with the same result regardless of "read_only".
If Request 1 locks session data, it finishes execution as logged_in==TRUE.

There may be race condition like this. However, it does not matter much if
legitimate users' requests are processed as authenticated or not under such
race condition. It's _legitimate_ users' request anyway.

If apps must not allow such race condition, any of
session_write_close/commit(), read_only
should not be used. (Request serialization is not perfect still, though.
Please read
session_regenerate_id() thread for details.)


2. The RFC describes that with 'lazy_write' set to TRUE, if no change was
made to session data, then PS_UPDATE_FUNC() will be called instead of
PS_WRITE_FUNC(). However, it only talks about internal implementation and
fails to address (or at least mention) that a custom session handler should
now also have an update() method.


This is not correct.
User defined save handler does not have to implement update() method.
Session module is designed to allow omitting new method/function just
like undocumented create_sid() function/method. There are test cases
for it.  (BTW, I noticed that I missed to compare dummy function address.
It's fixed in my repository already.)

"lazy_write" could be INI option and default to false, but it makes little
sense
because it is compatible with current behavior. I cannot think of good
reason to
disable it, but it may be disabled if you need.

I don't see good reason to postpone this change still.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

--001a11c2b3f0e2182c04f4b2a4e3--