Everything

php

undocumented session_name() change

php.internals

Lester Caine

8 years ago

Since Tony is blocked from this list he has posted a BC break on the
PHP-General list ... the main jist of which is that session_name() has
had it's DOCUMENTED functionality changed some time between 7.1.11 and
7.2.5

I can't see any discussion on session_name in the last two years or any
notification of the change, so when did it happen and why. More to the
point, why has the documentation not been amended to match the new
functionality?

--
Lester Caine - G8HFL
-----------------------------
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

Christoph Becker

8 years ago

On 24.05.2018 at 18:16, Lester Caine wrote:

> Since Tony is blocked from this list he has posted a BC break on the
> PHP-General list ... the main jist of which is that session_name() has
> had it's DOCUMENTED functionality changed some time between 7.1.11 and
> 7.2.5

In my opinion, the documentation is somewhat ambiguous regarding the
“current session name”.  What is the name of a session that will never
be started?  Is it ini_get('session.name'), or is there simply no
session name?

> I can't see any discussion on session_name in the last two years or any
> notification of the change, so when did it happen and why. More to the
> point, why has the documentation not been amended to match the new
> functionality?

The change was triggered by <https://bugs.php.net/71038> which required
some deeper changes, which apparently caused Yasuo to have a closer look
at the code recognizing some further strange behavior, so he submitted
<https://github.com/php/php-src/pull/2167>.  There was some discussion,
but obviously nobody objected to these changes for master, so the PR has
been merged.

Apparently, this well documented change (see UPGRADING) has been
overlooked for the migration guide.

Anyhow, it seems to me that Tony makes a mountain out of a molehill.
Apparently, very few code out there is affected by this change
(otherwise there certainly would have been more bug reports or
complains), and to cite Yasuo[1]:

| Therefore, proper codes will not be affected by this change. Only bad
| codes are detected.

I agree.  Consider the code Tony has posted on php-general@[2]:

if ($_GET['action'] == 'newsession') {
$session_name = getNewSessionName();  // user-defined function
session_name($session_name);
session_regenerate_id();
header('Location: ' ….);  // restart script to use new session name
and id
exit;
}

Why even call session_name($session_name) here?  To my knowledge, this
is a no-op in this case (assuming a session has already been started).

Finally, every minor or major PHP version has a pre-release phase of
roughly six month.  The relevant change has been there since
7.2.0alpha1, so everybody had ample time to check it out, and to
eventually complain before GA.

[1] <https://github.com/php/php-src/pull/2167#issue-89519969>
[2] <http://news.php.net/php.general/326472>

--
Christoph M. Becker

Christoph Becker

8 years ago

On 25.05.2018 at 12:28, Christoph M. Becker wrote:

>   if ($_GET['action'] == 'newsession') {
>    $session_name = getNewSessionName();  // user-defined function
>    session_name($session_name);
>    session_regenerate_id();
>    header('Location: ' ….);  // restart script to use new session name
> and id
>    exit;
>   }
>
> Why even call session_name($session_name) here?  To my knowledge, this
> is a no-op in this case (assuming a session has already been started).

I have to correct myself.  Actually, the call to session_regenerate_id()
did change the session name (i.e. the cookie etc.)  I'm still not
convinced, that this BC break should be reverted.

--
Christoph M. Becker

Yasuo Ohgaki

8 years ago

On Sun, May 27, 2018 at 1:16 AM Christoph M. Becker <cmbecker69@gmx.de>
wrote:

> On 25.05.2018 at 12:28, Christoph M. Becker wrote:
>
> >   if ($_GET['action'] == 'newsession') {
> >    $session_name = getNewSessionName();  // user-defined function
> >    session_name($session_name);
> >    session_regenerate_id();
> >    header('Location: ' ….);  // restart script to use new session name
> > and id
> >    exit;
> >   }
> >
> > Why even call session_name($session_name) here?  To my knowledge, this
> > is a no-op in this case (assuming a session has already been started).
>
> I have to correct myself.  Actually, the call to session_regenerate_id()
> did change the session name (i.e. the cookie etc.)  I'm still not
> convinced, that this BC break should be reverted.
>

Session module uses a few INI settings to work. As we know, INI values are
stored in modules global structure.

Since session works uses INI values as "Parameters" for module and its
submodules
while it is active, modifying these INI values caused number of unwanted
misbehaviors/crashes. i.e. It's side effect of changing globals.

These INI values must not be changed in the first place, but session module
didn't have proper internal state management. Since these side effects won't
be problem unless users abuse/misuse them, it was left until 7.2.

7.2 protects module globals (PS(session_name) is one of them) to prevent
abuse/misuse. Any function calls that cause side effects raise ERROR as
described in UPGRADING.

Since 7.2 prohibits harmful calls, users protected from some of very
hard to debug problems, e.g.
- Works in a environment, but not in other
- Works mostly, but fails sometimes

Even when users are affected by this change, there are ways to write code
that work in any PHP versions.

session_name('new_name') can be called while session is inactive in this
case. i.e. session_commit(); session_name('new_name'); session_start();

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net