Re: [RFC] session_start(), read_only, lazy_write; Take 2

From: Date: Mon, 24 Mar 2014 19:49:43 +0000
Subject: Re: [RFC] session_start(), read_only, lazy_write; Take 2
References: 1 2  Groups: php.internals 
Request: Send a blank email to [email protected] to get a copy of this message
Hi,

On Mon, Mar 24, 2014 at 7:39 PM, Bill Salak <[email protected]> wrote:
> Hi Andrey,
> In your proposed example problematic use case your code shows that the following happens:
>
>  - Request 1, opens session, populates _SESSION superglobal, closes session (no writeback)
>  - Request 2, opens sessions, unsets $_SESSION['logged_in'], writes session
>  - Request 1, checks value of $_SESSION['logged_in'] and does not recognize that the
> value of $_SESSION['logged_in'] has changed by Request 2
>
> The net result for Request 1 is exactly the same as what we have today except that today the
> path to this same result is that Request 2 can't read or modify the data until Request 1
> finishes. Because this is not a new or unexpected result I don't see how this is
> "downright dangerous" unless the user misunderstood what the new functionality does and
> designed solutions around this misunderstanding that exposed problems in their new code. I think
> this is actually the core of your RFP on this point which I think just boils down to - is read_only
> the best name for this flag ? I personally think it's a descriptive and self-evident name for
> the option because the session in Request 1 *is* read only (can't be written to) and that what
> you think of as a read only is actually better called a "read lock". Ultimately I
> don't care what it's called so if that's all this is about then I have no more
> interest in this part of the discussion and am happy to let those who care determine the name of the
> option.

The issue is exactly that it would be easy to not understand what
'read_only' does. Nowhere have I said that the functionality itself is
dangerous or that it is not useful, exactly the opposite - I love the
idea, it's pretty neat. My problem is with how it is named, and that's
what the RFC talks about. :)
Unless I'm mistaken, you already mentioned in another thread that it
is you who suggested the 'read_only' name, so I guess it's natural
that you don't see a problem with it and that it seems descriptive and
self-evident to you. Somebody else already applied the same logic - it
only reads and it doesn't allow a write. However, as described in the
RFC, the _close immediately_ part is very unclear.
I don't want to get into lengthy arguments with you or somebody else
over what "read-only" means. I'm not just talking about my
understanding of it here ... the term already has a meaning that is
recognized everywhere (google it if you don't believe me) and that's
why I'm so strongly against keeping it as it is.

> On your point about some future implementation of a write blocking, read-shared, session lock -
> it's an interesting idea.

Glad you like it, but as I already told you previously - it's just for
reference, I'm not interested in adding that feature right now.

> On your point about, "Maybe, if session_start() didn't accept mode parameters, that
> would've been fine. However, session_start() also accepts all session.* INIs +
> 'lazy_write' and all of those are modes of operation and not additional actions per se. So
> that makes it not only strange, but also inconsistent", you've lost me -I don't see a
> problem. If I call session_start and I can pass in a bunch of options about how the session will act
> in this call stack that seems like the best and most pragmatic solution. The distinction between
> modes of operation and additional actions seem like a semantic nitpick that end-users wouldn't
> intuitively understand. In other words, it seems counter-intuitive to work some other way and
> wouldn't produce more easily read/written code to have it different.

Well, I certainly can't understand why you think that a separate
function would be counter-intuitive or that it won't produce
easily-read code. With what we currently have, chances are that the
following line would be seen quite often:

    session_start($options);

What do you understand from that line (regardless of whether
'read_only' is in $options or not)? I see "start a session with some
options". This is again where the closing part is lost, nothing
implies that anything but "start a session" would be performed, as an
action. While on the other hand:

    session_start_close($options);

I'm quite certain that everybody would have a better understanding of
what this line does, simply because it's explicit.
Yes, it is nitpicky and it's nothing but semantics, but semantics are
important. :)

Cheers,
Andrey.


Thread (29 messages)

« previous php.internals (#73392) next »