Problem

According to libraries/joomla/session.php, the function set() should return the old value of the variable:

/**
     * Set data into the session store.
     *
     * @param   string  $name       Name of a variable.
     * @param   mixed   $value      Value of a variable.
     * @param   string  $namespace  Namespace to use, default to 'default'.
     *
     * @return  mixed  Old value of a variable.
     *
     * @since   11.1
     */
    public function set($name, $value = null, $namespace = 'default')

This is no longer working and I'm guessing since the hardening patch was introduced.

Test instructions

Tested with Joomla! 3.4.8, haven't had a chance to test the staging branch just yet.
Run the following code:

$session = JFactory::getSession();
$session->set('test', 1);
echo $session->set('test', 2);

Expected result:

Actual result:

As you can see, it no longer returns the old value, it will return the current (set) value instead.

OctavianC - open - 8 Jan 2016

bertmert - comment - 8 Jan 2016

Confirmed.
Correct me if I'm wrong (and don't kill me verbally ;-) )
I think reason is that $session is represented by a Registry object now ($this->data in class JSession).
Thus line https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/session/session.php#L528 in JSession::set() returns the new value (expected behavior for Registry objects)

return $this->data->set($namespace . '.' . $name, $value);

I don't know if a fix like this is allowed and makes sense

 public function set($name, $value = null, $namespace = 'default')
 {
  // Add prefix to namespace to avoid collisions
  $namespace = '__' . $namespace;

  if ($this->_state !== 'active')
  {
   // @TODO :: generated error here
   return null;
  }

  $old = $this->data->get($namespace . '.' . $name, null);

  $this->data->set($namespace . '.' . $name, $value);

  return $old;
 }

OctavianC - comment - 11 Jan 2016

I consider it a bug since it breaks backwards compatibility.

bertmert - comment - 11 Jan 2016

I've posted my fix not knowing if it is acceptable.
Test it if you want to and provide a PullRequest if it works without crashing something.
Then it's on a higher discussion level or so and people can test it in detail.
Otherwise you have to wait until someone lets us know if it's a BC issue and should be fixed or not.

(Session is a delicate subject at the moment and I'm too shy to provide a PR.)

brianteeman - change - 13 Jan 2016

Labels

Added: ?

brianteeman - change - 1 Mar 2016

Add a Comment

Older
Newer

Status	New	⇒	Closed
Closed_Date	0000-00-00 00:00:00	⇒	2016-03-02 08:22:28
Closed_By		⇒	brianteeman

Joomla! Issue Tracker - CMS

[#8859] - JFactory::getSession()->set() no longer working according to docs

Problem

Test instructions

Expected result:

Actual result:

Add a Comment