Skip to content

Commit

Permalink
[HttpFoundation] Refactor away options property.
Browse files Browse the repository at this point in the history
It does not make sense to try and store session ini directives since they can be changes outside
of the class as they are part of the global state.

Coding stan
  • Loading branch information
Drak committed Mar 14, 2012
1 parent 21221f7 commit 39526df
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 30 deletions.
Expand Up @@ -76,7 +76,7 @@ public function start()
*/
public function regenerate($destroy = false)
{
if ($this->options['auto_start'] && !$this->started) {
if (!$this->started) {
$this->start();
}

Expand Down Expand Up @@ -124,6 +124,22 @@ public function clear()
$this->loadSession($this->sessionData);
}

/**
* {@inheritdoc}
*/
public function getBag($name)
{
if (!isset($this->bags[$name])) {
throw new \InvalidArgumentException(sprintf('The SessionBagInterface %s is not registered.', $name));
}

if (!$this->started) {
$this->start();
}

return $this->bags[$name];
}

/**
* Generates a session ID.
*
Expand Down
Expand Up @@ -30,11 +30,6 @@ class SessionStorage implements SessionStorageInterface
*/
protected $bags;

/**
* @var array
*/
protected $options = array();

/**
* @var boolean
*/
Expand Down Expand Up @@ -93,6 +88,11 @@ class SessionStorage implements SessionStorageInterface
*/
public function __construct(array $options = array(), $handler = null)
{
// sensible defaults
ini_set('session.auto_start', 0); // by default we prefer to explicitly start the session using the class.
ini_set('session.cache_limiter', ''); // disable by default because it's managed by HeaderBag (if used)
ini_set('session.use_cookies', 1);

$this->setOptions($options);
$this->setSaveHandler($handler);
}
Expand All @@ -116,7 +116,15 @@ public function start()
return true;
}

if ($this->options['use_cookies'] && headers_sent()) {
// catch condition where session was started automatically by PHP
if (!$this->started && !$this->closed && $this->saveHandler->isActive()
&& $this->saveHandler->isSessionHandlerInterface()) {
$this->loadSession();

return true;
}

if (ini_get('session.use_cookies') && headers_sent()) {
throw new \RuntimeException('Failed to start the session because headers have already been sent.');
}

Expand All @@ -131,9 +139,6 @@ public function start()
$this->saveHandler->setActive(false);
}

$this->started = true;
$this->closed = false;

return true;
}

Expand Down Expand Up @@ -205,8 +210,10 @@ public function getBag($name)
throw new \InvalidArgumentException(sprintf('The SessionBagInterface %s is not registered.', $name));
}

if ($this->options['auto_start'] && !$this->started) {
if (ini_get('session.auto_start') && !$this->started) {
$this->start();
} else if ($this->saveHandler->isActive() && !$this->started) {
$this->loadSession();
}

return $this->bags[$name];
Expand All @@ -218,30 +225,13 @@ public function getBag($name)
* For convenience we omit 'session.' from the beginning of the keys.
* Explicitly ignores other ini keys.
*
* session_get_cookie_params() overrides values.
*
* @param array $options
* @param array $options Session ini directives array(key => value).
*
* @see http://php.net/session.configuration
*/
public function setOptions(array $options)
{
$this->options = $options;

// set defaults for certain values
$defaults = array(
'cache_limiter' => '', // disable by default because it's managed by HeaderBag (if used)
'auto_start' => false,
'use_cookies' => true,
);

foreach ($defaults as $key => $value) {
if (!isset($this->options[$key])) {
$this->options[$key] = $value;
}
}

foreach ($this->options as $key => $value) {
foreach ($options as $key => $value) {
if (in_array($key, array(
'auto_start', 'cache_limiter', 'cookie_domain', 'cookie_httponly',
'cookie_lifetime', 'cookie_path', 'cookie_secure',
Expand Down Expand Up @@ -322,5 +312,8 @@ protected function loadSession(array &$session = null)
$session[$key] = isset($session[$key]) ? $session[$key] : array();
$bag->initialize($session[$key]);
}

$this->started = true;
$this->closed = false;
}
}

0 comments on commit 39526df

Please sign in to comment.