Edit report at https://bugs.php.net/bug.php?id=76358&edit=1
ID: 76358
Comment by: 1978 dot jl at gmail dot com
Reported by: tony at marston-home dot demon dot co dot uk
Summary: Cannot change session name when session is active
Status: Assigned
Type: Bug
Package: Session related
Operating System: Windows 10
PHP Version: 7.2.5
Assigned To: yohgaki
Block user comment: N
Private report: N
New Comment:
+1, very problematic when we must migrate from 7.1 to 7.3
Previous Comments:
------------------------------------------------------------------------
[2018-12-04 19:12:02] john at zerocrates dot org
Related To: Bug #77238
------------------------------------------------------------------------
[2018-06-13 15:19:35] tony at marston-home dot demon dot co dot uk
These are the five code samples that Yasuo sent me as "proof" of the broken behaviour with
the session functions which his fix supposedly cures. Below each example is my response which shows
that the problem actually lies with the code that calls those functions and not the functions
themselves.
Example #1 - Wrong write and/or crash>
<?php
session_start();
session_name('new_name');
session_commit(); // Save handler can write session data to wrong storage.
There is no such thing as "wrong" storage in this case as session_commit() does NOT
reference $session_name when calling the write() method in the session handler, it uses $session_id.
This code does not change $session_id so the session data is put back in the same place from where
it was read.
Example #2 - Wrong name returned
<?php
session_start();
session_name('new_name');
// somewhere in the code
$my_current_session_name = session_name(); // Wrong session name. session_start() uses old one.
// This behavior can cause bug when user is distributing session storage accesses by session name.
There are a number of glaring mistakes here:
- Changing the value of session.name does not take effect until the next call to session_start().
When a session is started the value of session.name is provided in the $session_name argument in the
open() method in the session handler.
- No method or function has ever been provided which will return the value of $session_name which
was used in the call to open().
- If the value of session.name is changed while a session is active that change is not communicated
with the session handler and does not affect the session handler in any way as all read() and
write() operations are performed using the $session_id.
- If the session name is used to change the value in $save_path then it should be done in the open()
method in the session handler where both $save_path and $session_name are provided as arguments. If
these values need to be used in other methods then they should be stored as class properties and
accessed using $this->varname.
Example #3 - Wrong write and/or crash. Pattern 2.
<?php
session_start();
session_name('new_name');
// somewhere in the code
session_regenerate_id();
// Save handler can write session data to wrong storage.
// In worst cases, PHP crashes
?>
Bad usage again. session_regenerate_id() will regenerate a new id for the CURRENT session, but this
will be linked with the value of session.name which was available when the current session was
started. The value 'new_name' will not take effect until the next call to session_start(),
just as the manual says.
Example #4 - Wrong write and/or crash. Pattern 3.
<?php
session_start();
session_name('new_name');
session_write(); // Save handler can write session data to wrong storage.
// In worst cases, PHP crashes
?>
This is the same as Example #1.
Example #5 - Wrong write and/or crash. Pattern 4. + Bogus session_name() call.
<?php
session_start();
session_name('new_name');
// Save handler can write session data to wrong storage at the end of script execution.
// In worst cases, PHP crashes
// In addition, user wouldn't notice bogus session_name() call if there is no error.
?>
This is a duplicate of #1 and #4. The session is started with a particular $session_id, and as that
$session_id is never changed any updates to that session data will be written back using the same
$session_id. A change in $session_name does not change the $session_id. The $session_name and
$session_id are different entities which are accessed using different functions. They are only
brought together when the cookie is accessed, which is either by session_start() or
session_regenerate_id(). This code does nothing to cause 'new_name' to be written out as a
cookie, therefore it simply disappears.
------------------------------------------------------------------------
[2018-06-06 09:50:10] requinix@php.net
Related To: Bug #76413
------------------------------------------------------------------------
[2018-05-31 11:09:37] tony at marston-home dot demon dot co dot uk
> Not every (minor) BC breaking change requires an RFC or discussion
on the internals mailing list.
There is no such thing as a "minor" BC break. Every BC break causes code that previously
worked to suddenly fail for no good reason.
Every BC break should be discussed on the internals list so that other developers can review it to
see if it is actually justified or can be improved. It must then be voted upon, which then means
that the change MUST be subject to an RFC.
The idea that a developer can sneak in a BC break WITHOUT it being discussed on the internals list
and WITHOUT any prior warning or even a mention in the change log will create a bad smell for the
millions of application developers who expect the language to be reliable and stable. It is the fear
of BC breaks which causes developers to delay upgrading to the latest version of the language.
> This very change was submitted as pull request on Github (which is an official and publicly
> available channel)
GitHub may be the official channel for pull requests, but it is *NOT* the official channel for
discussions. There were *NO* discussions on the proposed changes to session handling which
identified this change to session_name().
> I still do not think that your interpretation of the (former) documentation is
absolutely correct; at the very least there is some room for interpretation.
The documentation at http://php.net/manual/en/function.session-name.php
is perfectly clear - the session_name() function can be used either to get the current name or set a
new one, or even both at the same time. Example #1 in the manual clearly demonstrates this usage.
This means that it has always been perfectly legitimate to call session_name() even if a session is
already active.
The documentation also states: "The session name is reset to the default value stored in
session.name at request startup time. Thus, you need to call session_name() for every request (and
before session_start() is called)."
This tells me that if you change the session name that it will not be effective until the next call
to session_start().
> Yasuo claims ⦠"the former behavior is not valid and allows crash and
> misbehaviorsâ
I have been conversing with Yasuo via private email on this claim, and he sent me five examples of
code where there was a call to session_name('newname') after a call to session_start() and
it did not work as expected. This is simply because there was *NO* call to session_start() after a
change in name EVEN THOUGH THE DOCUMENTATION SAYS THAT THERE SHOULD BE, or that the 2nd call to
session_start() failed because a session was already active. The documentation at http://php.net/manual/en/function.session-start.php
clearly states: "As of PHP 4.3.3, calling session_start() after the session was previously
started will result in an error".
It is obvious to me that if session_name('newname') is called when a session is already
active then the new name will not take effect until the next call to session_start() and that the
2nd call to session_start() must be preceded by a call to session_wtite_close(). It was a mistake on
Yasuo's part to think that it was the call to session_name() which should be preceded by
session_write_close().
This mistake was brought about by his misunderstanding of how the various session functions could
legitimately be used. He actually admitted to me in one of his emails: "I didn't expect
users to call session_name() after session_start() because it does not make sense with session
module structure".
There you have it. It didn't make sense to him, so he changed the behaviour of the function to
forbid it. Even worse, he changed the legitimate behaviour of this function without any discussion
or warning whatsoever, and without even a mention in the change log. This is a very bad precedent
which, if allowed to continue in the future, will cause millions of application developers to lose
confidence in the language. We (because I am one of those application developers) expect the
language to be reliable and stable, and by "stable" I do *NOT* mean "full of
manure".
------------------------------------------------------------------------
[2018-05-30 21:30:15] cmb@php.net
> I think that it is a sad day when somebody can decide that the
> documented behaviour of a 20 year old function does not fit it
> with their beliefs, so they go ahead and change it without
> following the correct procedure - raise RFC, discuss it on the
> internals list, then put it to a vote.
Not every (minor) BC breaking change requires an RFC or discussion
on the internals mailing list. This very change was submitted as
pull request on Github (which is an official and publicly
available channel), and since there have been no general
objections, the PR was committed. Also note, that I still do not
think that your interpretation of the (former) documentation is
absolutely correct; at the very least there is some room for
interpretation.
Anyhow, assigning to Yasuo, who recently committed a documentation
change[1] which claims that the former behavior âis not valid and
allows crash and misbehaviorsâ. Maybe some short explanation what
could go wrong would be approriate here.
[1] <http://svn.php.net/viewvc?view=revision&revision=345080>
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=76358
--
Edit this bug report at https://bugs.php.net/bug.php?id=76358&edit=1