Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:73195
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain devilix.net designates 209.85.213.41 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAGa2bXaL99PgxuWr7eTC89Ltc914UfxCgjWNqHsnnSJZyTUyPQ@mail.gmail.com>
References: <CAPhkiZw5xWRkc=ofh-nB5ZkjfThZFkH_+pK5RZaQFLKhno7u7A@mail.gmail.com>
	<CAGa2bXaL99PgxuWr7eTC89Ltc914UfxCgjWNqHsnnSJZyTUyPQ@mail.gmail.com>
Date: Sun, 16 Mar 2014 18:47:57 +0200
Message-ID: <CAPhkiZzBPaoeAqLYpWNNpi_H05rBw2uvUmPX=CTv-sWYerifjA@mail.gmail.com>
To: Yasuo Ohgaki <yohgaki@ohgaki.net>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [RFC] Revert/extend/postpone original RFC about
 read_only, lazy_write sessions
From: narf@devilix.net (Andrey Andreev)

Hi,

On Sun, Mar 16, 2014 at 7:32 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
> // $_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.

Read the explanation in my previous mail:
http://marc.info/?l=php-internals&m=139498773308515

> 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.)

It's about making a conscious choice of when to use it, not if at all.
The name, and it's presence as an option instead of a separate
function is misleading.

> 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.)

It's not incorrect, the RFC doesn't mention this case and what it
happens in it. I admit, I haven't looked at the patch and the tests,
but if PS_UPDATE_FUNC() doesn't exist, how would you call it?

The session ID is something consumed by (or passed to) the session
handler, there's a big difference.

> "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.

Again, nothing explains how that compatibility is maintaned and this
is exactly the concern that I have. When something is unknown, it
usually produces side effects.

Cheers,
Andrey.