cookie set multiple times #1345

Closed
opensourceame opened this Issue May 10, 2012 · 4 comments

4 participants

@opensourceame

CodeIgniter's Session library often sets the same cookie multiple times. I've tested this on numerous sites. To fix this we need a flag in the session object to mark whether the cookie has been set already:

libraries/Session.php

// line 50

var $cookie_set = false;

// line 671, at the beginning of _set_cookie()

// don't set cookie multiple times
if ($this->cookie_set)
  return false;
@petsagouris

Thanks for this, but why not do a pull request?

@Bluewind

The fix isn't that simple because if you don't use a database to store the userdata, _set_cookie() will be called multiple times to update the cookie.

A proper fix would be to set the cookie once the controller is done, rather than on each session data change. I'm not going to fix this since I only use the database for storage.

@narfbg

PR available: #1780

@narfbg narfbg closed this Oct 18, 2012
@narfbg

Please see the above referenced commit (it's in a separate branch, not merged yet).

@narfbg narfbg added a commit that referenced this issue Dec 1, 2012
@narfbg narfbg 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().
128d719
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment