Permalink
Browse files

Replace cookie helper set_cookie() with an improved version

as a common function.

Also deprecated CI_Input::set_cookie() which is now an alias
for this new function.

The new function will now replace cookies with the same name
that were already set (either by set_cookie() or the native
setcookie() and header() functions) in the PHP's headers
queue.
This fixes issue #1345 and supersedes PR #1780,
which were aimed at fixing the Session library's behavior
where it sent multiple cookies with the sess_cookie_name
when the session cookie value had changed.

It will now also always send the relatively new Max-Age
cookie attribute (see http://tools.ietf.org/rfc/rfc6265.txt)
and Expire will always be sent as a GMT timestamp, in an
attempt to fix reported issues with Google Chrome (see
issues #1726 and #1908).

Cookies with the Secure attribute that are intended to only
be send by the browser via encrypted connections will no
longer be send if the website is not accessed via HTTPS.

Also, the optional parameters' default values are changed to
NULL instead of actually usable values, so that config_item()
calls are only used if we're sure that the user/developer
didn't set those intentionally.

All usage of the native setcookie() function in CI has been
replaces with set_cookie().
  • Loading branch information...
2 parents b37d2bc + 66b36ef commit 128d7194ca90ebc005c523f2918194af4ea924ed @narfbg narfbg committed Nov 30, 2012
View
@@ -698,5 +698,182 @@ function function_usable($function_name)
}
}
+// ------------------------------------------------------------------------
+
+if ( ! function_exists('set_cookie'))
+{
+ /**
+ * Set cookie
+ *
+ * Sends a cookie, taking care to replace it in the PHP header() queue,
+ * if one with the same name already exists.
+ *
+ * Accepts an arbitrary number of parameters (up to 7) or an associative
+ * array in the first parameter containing all the values.
+ *
+ * @param string|mixed[] $name Cookie name or an array containing parameters
+ * @param string $value Cookie value
+ * @param int $expire Cookie expiration time in seconds
+ * @param string $domain Cookie domain (e.g.: '.yourdomain.com')
+ * @param string $path Cookie path (default: '/')
+ * @param string $prefix Cookie name prefix
+ * @param bool $secure Whether to only transfer cookies via SSL
+ * @param bool $httponly Whether to only makes the cookie accessible via HTTP (no javascript)
+ * @return void
+ */
+ function set_cookie($name, $value = '', $expire = NULL, $path = NULL, $domain = NULL, $prefix = NULL, $secure = NULL, $httponly = NULL)
+ {
+ // Were the parameters passed as an array?
+ if (is_array($name))
+ {
+ // always leave 'name' in last place, as the loop will break otherwise, due to $$item
+ foreach (array('value', 'expire', 'domain', 'path', 'prefix', 'secure', 'httponly', 'name') as $item)
+ {
+ if (isset($name[$item]))
+ {
+ $$item = $name[$item];
+ }
+ }
+ }
+
+ // Sanitize our parameters
+ if ($secure === NULL)
+ {
+ $secure = config_item('cookie_secure');
+ }
+
+ // Do we have to send the cookie at all?
+ if ($secure === TRUE && ! is_https())
+ {
+ return;
+ }
+
+ // Prepend our name prefix
+ $name = ($prefix === NULL ? config_item('cookie_prefix') : $prefix)
+ .$name;
+
+ $payload = 'Set-Cookie: '.$name.'='.urlencode($value).';';
+
+ // If we don't have a valid expiry time - expire the cookie
+ is_numeric($expire) OR $expire = -31536000;
+
+ // Only send expiry time if it's not a zero value
+ //
+ // DO NOT CHANGE THIS EXPRESSION!
+ // Multiple side effects can occur if you do, including:
+ //
+ // !== Ignores non-integer values
+ // >, >= Ignore negative values
+ if ($expire != 0)
+ {
+ // GMT is the most safe choice
+ $payload .=' Expires='.gmdate('D, d-M-Y H:i:s T', time() + $expire).';'
+ // RFC6265 describes the Max-Age attribute:
+ //
+ // - Specifies the cookie lifetime in seconds
+ // - Not supported by all user agents (naturally, as it was introduced in 2011)
+ // - User agents that don't support it will simply ignore it
+ // - User agents that support it MUST use it instead of Expires, when present
+ //
+ // Reference: http://tools.ietf.org/rfc/rfc6265.txt
+ //
+ // For the above reasons, and because some user agents might not calculate
+ // timezone differences properly (e.g. due to the system timezone setting
+ // not being correct), sending the Max-Age attribute is our safest option.
+ //
+ // The downside is - it doesn't accept negative values.
+ .($expire > 0 ? ' Max-Age='.$expire.';' : '');
+ }
+
+ $payload .= ' Path='.(empty($path) ? config_item('cookie_path') : $path).';'
+ .' Domain='.(empty($domain) ? config_item('cookie_domain') : $domain)
+ .($secure ? '; Secure' : ''); // We've already initialized this one
+
+ if ($httponly === NULL)
+ {
+ $httponly = config_item('cookie_httponly');
+ }
+
+ $payload .= ($httponly ? '; HttpOnly' : '');
+
+ // Now, let's go through the headers to see if we've already sent any cookies.
+ // To allow usage of header_remove() later, iterate in a decremental order.
+ $queue = array();
+ for ($headers = headers_list(), $i = count($headers) - 1; $i > -1; $i--)
+ {
+ // Is it a cookie? Get its name and cache it
+ if (sscanf($headers[$i], 'Set-Cookie: %[^=]', $cookie_name) !== 1)
+ {
+ $queue[$cookie_name] = $headers[$i];
+ }
+ }
+
+ // If a matching cookie name doesn't exist - just send,
+ // if it does and IF it is the only cookie - replace it.
+ if (($re = isset($queue[$name])) === FALSE OR ($re = (count($queue) === 1)))
+ {
+ header($payload, $re);
+ return;
+ }
+
+ // OK, so a matching cookie name is already queued and we have other cookies as well.
+ // We need to replace the matching cookie with our own.
+ //
+ // We'll be relying on the $re variable to determine our procedure's state and behavior:
+ //
+ // (bool) Search for the right cookie to replace and when found:
+ //
+ // TRUE: Clear all of the headers
+ // FALSE: Use header_remove() to only remove the relevant headers
+ //
+ // (null) Re-send previously removed headers
+ //
+ $re = ! is_php('5.3');
+ do
+ {
+ // Re-send the removed Set-Cookie headers.
+ // This will only happen after our cookie was sent.
+ if ($re === NULL)
+ {
+ header($payload);
+ if (prev($queue) !== FALSE)
+ {
+ continue;
+ }
+ else
+ {
+ break;
+ }
+ }
+
+ // Do we use header_remove() or do we have to replace all Set-Cookie headers?
+ if ($re === FALSE)
+ {
+ header_remove('Set-Cookie');
+ }
+
+ // If the cookie name matches ours - replace the queue header entry with our own
+ if (key($queue) === $name)
+ {
+ header($payload, $re);
+
+ // If we didn't use header_replace(), remove the old cookie matching our
+ // name so it doesn't get re-sent and set the pointer to the last entry.
+ if ($re === TRUE)
+ {
+ unset($queue[$name]);
+ end($queue);
+ }
+
+ // Switch to re-sending more
+ $re = NULL;
+ continue;
+ }
+ }
+ // We won't get to this point once $re is set to NULL
+ while (next($queue) !== FALSE);
+ }
+}
+
/* End of file Common.php */
/* Location: ./system/core/Common.php */
View
@@ -310,9 +310,6 @@ public function input_stream($index = '', $xss_clean = FALSE)
/**
* Set cookie
*
- * Accepts an arbitrary number of parameters (up to 7) or an associative
- * array in the first parameter containing all the values.
- *
* @param string|mixed[] $name Cookie name or an array containing parameters
* @param string $value Cookie value
* @param int $expire Cookie expiration time in seconds
@@ -322,56 +319,12 @@ public function input_stream($index = '', $xss_clean = FALSE)
* @param bool $secure Whether to only transfer cookies via SSL
* @param bool $httponly Whether to only makes the cookie accessible via HTTP (no javascript)
* @return void
+ * @deprecated 3.0.0 An alias for common function set_cookie()
+ * @see set_cookie()
*/
- public function set_cookie($name = '', $value = '', $expire = '', $domain = '', $path = '/', $prefix = '', $secure = FALSE, $httponly = FALSE)
+ public function set_cookie($name, $value = '', $expire = NULL, $domain = NULL, $path = NULL, $prefix = NULL, $secure = NULL, $httponly = NULL)
{
- if (is_array($name))
- {
- // always leave 'name' in last place, as the loop will break otherwise, due to $$item
- foreach (array('value', 'expire', 'domain', 'path', 'prefix', 'secure', 'httponly', 'name') as $item)
- {
- if (isset($name[$item]))
- {
- $$item = $name[$item];
- }
- }
- }
-
- if ($prefix === '' && config_item('cookie_prefix') !== '')
- {
- $prefix = config_item('cookie_prefix');
- }
-
- if ($domain == '' && config_item('cookie_domain') != '')
- {
- $domain = config_item('cookie_domain');
- }
-
- if ($path === '/' && config_item('cookie_path') !== '/')
- {
- $path = config_item('cookie_path');
- }
-
- if ($secure === FALSE && config_item('cookie_secure') !== FALSE)
- {
- $secure = config_item('cookie_secure');
- }
-
- if ($httponly === FALSE && config_item('cookie_httponly') !== FALSE)
- {
- $httponly = config_item('cookie_httponly');
- }
-
- if ( ! is_numeric($expire))
- {
- $expire = time() - 86500;
- }
- else
- {
- $expire = ($expire > 0) ? time() + $expire : 0;
- }
-
- setcookie($prefix.$name, $value, $expire, $path, $domain, $secure, $httponly);
+ set_cookie($name, $value, $expire, $domain, $path, $prefix, $secure, $httponly);
}
// --------------------------------------------------------------------
View
@@ -201,28 +201,11 @@ public function csrf_verify()
/**
* CSRF Set Cookie
*
- * @codeCoverageIgnore
* @return object
*/
public function csrf_set_cookie()
{
- $expire = time() + $this->_csrf_expire;
- $secure_cookie = (bool) config_item('cookie_secure');
-
- if ($secure_cookie && ! is_https())
- {
- return FALSE;
- }
-
- setcookie(
- $this->_csrf_cookie_name,
- $this->_csrf_hash,
- $expire,
- config_item('cookie_path'),
- config_item('cookie_domain'),
- $secure_cookie,
- config_item('cookie_httponly')
- );
+ set_cookie($this->_csrf_cookie_name, $this->_csrf_hash, $this->_csrf_expire);
log_message('debug', 'CRSF cookie Set');
return $this;
@@ -36,34 +36,6 @@
* @link http://codeigniter.com/user_guide/helpers/cookie_helper.html
*/
-// ------------------------------------------------------------------------
-
-if ( ! function_exists('set_cookie'))
-{
- /**
- * Set cookie
- *
- * Accepts seven parameters, or you can submit an associative
- * array in the first parameter containing all the values.
- *
- * @param mixed
- * @param string the value of the cookie
- * @param string the number of seconds until expiration
- * @param string the cookie domain. Usually: .yourdomain.com
- * @param string the cookie path
- * @param string the cookie prefix
- * @param bool true makes the cookie secure
- * @param bool true makes the cookie accessible via http(s) only (no javascript)
- * @return void
- */
- function set_cookie($name = '', $value = '', $expire = '', $domain = '', $path = '/', $prefix = '', $secure = FALSE, $httponly = FALSE)
- {
- // Set the config file options
- $CI =& get_instance();
- $CI->input->set_cookie($name, $value, $expire, $domain, $path, $prefix, $secure, $httponly);
- }
-}
-
// --------------------------------------------------------------------
if ( ! function_exists('get_cookie'))
@@ -306,8 +306,7 @@ public function sess_destroy()
}
// Kill the cookie
- $this->_setcookie($this->sess_cookie_name, '', ($this->now - 31500000),
- $this->cookie_path, $this->cookie_domain, 0);
+ set_cookie($this->sess_cookie_name, '', -31500000, $this->cookie_path, $this->cookie_domain, '', FALSE);
// Kill session data
$this->userdata = array();
@@ -694,32 +693,11 @@ protected function _set_cookie()
// Require message authentication
$cookie_data .= hash_hmac('sha1', $cookie_data, $this->encryption_key);
- $expire = ($this->sess_expire_on_close === TRUE) ? 0 : $this->sess_expiration + time();
+ $expire = ($this->sess_expire_on_close === TRUE) ? 0 : $this->sess_expiration;
// Set the cookie
- $this->_setcookie($this->sess_cookie_name, $cookie_data, $expire, $this->cookie_path, $this->cookie_domain,
- $this->cookie_secure, $this->cookie_httponly);
- }
-
- // ------------------------------------------------------------------------
-
- /**
- * Set a cookie with the system
- *
- * This abstraction of the setcookie call allows overriding for unit testing
- *
- * @param string Cookie name
- * @param string Cookie value
- * @param int Expiration time
- * @param string Cookie path
- * @param string Cookie domain
- * @param bool Secure connection flag
- * @param bool HTTP protocol only flag
- * @return void
- */
- protected function _setcookie($name, $value = '', $expire = 0, $path = '', $domain = '', $secure = FALSE, $httponly = FALSE)
- {
- setcookie($name, $value, $expire, $path, $domain, $secure, $httponly);
+ set_cookie($this->sess_cookie_name, $cookie_data, $expire, $this->cookie_path, $this->cookie_domain,
+ '', $this->cookie_secure, $this->cookie_httponly);
}
// ------------------------------------------------------------------------
@@ -200,7 +200,7 @@ public function sess_destroy()
{
// Clear session cookie
$params = session_get_cookie_params();
- setcookie($name, '', time() - 42000, $params['path'], $params['domain'], $params['secure'], $params['httponly']);
+ set_cookie($name, '', -42000, $params['path'], $params['domain'], '', $params['secure'], $params['httponly']);
unset($_COOKIE[$name]);
}
session_destroy();
@@ -49,4 +49,11 @@ public function test_html_escape()
);
}
+ // ------------------------------------------------------------------------
+
+ public function test_set_cookie()
+ {
+ $this->markTestSkipped('Need to find a way to overcome a headers already sent exception');
+ }
+
}
Oops, something went wrong.

2 comments on commit 128d719

Contributor

narfbg replied Dec 1, 2012

Seems like I didn't get the right issue numbers for Google Chrome, should've been these: #1999, #744

Edit: Also, another PR on duplicate cookies - #1710

Contributor

dionysiosarvanitis replied May 23, 2013

I think that call to Security/csrf_set_cookie/set_cookie() should indicate already prefixed cookie_name.

(-) set_cookie($this->_csrf_cookie_name, $this->_csrf_hash, $this->_csrf_expire);
(+) set_cookie($this->_csrf_cookie_name, $this->_csrf_hash, $this->_csrf_expire, NULL, NULL, '');

Also, in Common/set_cookie() it doesn't caching the right values. So, it should change to:

(-) if (sscanf($headers[$i], 'Set-Cookie: %[^=]', $cookie_name) !== 1)
(+) if (sscanf($headers[$i], 'Set-Cookie: %[^=]', $cookie_name) === 1)

Please sign in to comment.