Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Feature request: split long session cookie into few less-than-4096b cookies #2085

Closed
dtmax opened this Issue Dec 19, 2012 · 13 comments

Comments

Projects
None yet
5 participants

dtmax commented Dec 19, 2012

I'm using cookie-based sessions.
Sometimes, when session contains a lot of info, especially for encrypted sessions, cookie size is bigger than 4096 bytes. This is not supported by some(all?) browsers.

Idea is to split the session cookie into smaller chunks when sending "Set-cookie" header. For example, ci_session_1, ci_session_2 etc. And then recompose it back in CI core.

Contributor

AndrewPodner commented Dec 19, 2012

Anyone have an opinion as to whether using strlen() would be sufficient to test the length of the session cookie? I don't think it will be perfect, but if the test were to start splitting it up at say 4000, do you think that would suffice to satisfy the requirements of the feature request?

Contributor

narfbg commented Dec 20, 2012

Yes, 1 character === 1 byte, so strlen() should do the job. However, I'm not sure if the limit is just on the data or the whole header.

Contributor

AndrewPodner commented Dec 20, 2012

Well, it looks like it is the whole header, so that can be accounted for. My question is this: since every cookie for the domain/path is sent with every HTTP request, would implementing this feature open up the potential to dramatically increase the overhead of HTTP requests? Should some sort of limit be put on it?

Contributor

narfbg commented Dec 20, 2012

IMO, this feature is completely unnecessary - session data shouldn't be stored in a cookie, but on the server instead. The problem is - CodeIgniter has supported cookie-based sessions probably since its initial version, so if somebody needs to use them and really wants to go over 4kb, they should be well aware of that additional overhead.

Contributor

AndrewPodner commented Dec 20, 2012

I tend to agree. If I am remembering correctly, a large number of networks are set up @ 1500b per packet, and the best practice is to limit an HTTP request to a single packet to keep overhead down on the network. My concern with this feature is that the additional overhead it creates will not be isolated to the application or even the server that the app resides on, it will cascade out to the network (LAN, Internet, whatever). While this feature request is technically feasible, I believe it might be better closed and left out. There is no impact on existing functionality by not implementing it. And now for the controversial bit.... should there be consideration of deprecating the current feature at some point in favor of steering future development towards a smaller network traffic footprint on a per-request basis? Just a thought.

Contributor

narfbg commented Dec 20, 2012

Considering the page output, I don't think that you'll ever fit a page in a single packet, so that's irrelevant.

Considering what I wrote in my previous comment - I don't really care if this is implemented or not. At this point I can't absolutely reject it, nor could I motivate anybody to write it. I would agree on deprecating it though, but it's not my decision to make, especially with a database as the only alternative backend currently supported.

Contributor

AndrewPodner commented Dec 20, 2012

Right, but I was thinking more about the upstream request from the Client/Browser to the Server, which is where the cookie would be transmitted (unless I am totally thinking about this wrong, and I am only on first cup of coffee, so that is possible :) )

In any case, if it can't be closed by rejection, then I suppose inclusion is the next viable option :) I think part of it will have to be, as you alluded to, some clear documentation about possible effects of utilizing the feature.

Contributor

AndrewPodner commented Dec 20, 2012

@dtmax : I have looked at this 2 or 3 different ways, and I have not come up with a solution that isn't "hacky" or doesn't break something else. I am still also having some reservations about the overall concept of using cookie data to this degree. I am concerned that someone making heavy use of this will suffer in the performance of the application, which would be an unfortunate unintended consequence.

dtmax commented Dec 21, 2012

I read your discussion with Narfbg. Thank you for a time you spent on it.
In general, IMO, this feature could be implemented, because we already have cookie-based sessions implemented. For me, it looks more like a bug, because session data just cropped. Btw, Codeigniter few years was sending a lot of extra Set-cookie headers (#1345), which was multiplying total size of sent headers.
It's up to developer, which kind of data to store in cookie-based session. Same as which data to store in usual cookies. It also can lead to http-headers overhead and decreasing application performance. And when to switch to database-driven sessions.

Contributor

GDmac commented Dec 22, 2012

-1 This would "encourage" storing even more (possibly sensitive) data in cookies.

Contributor

narfbg commented Oct 29, 2013

Closing, if you want to store larger data - use server-side storage.

@narfbg narfbg closed this Oct 29, 2013

yehudahkay commented Jul 10, 2017

@narfbg we are using codeigniter 2 and when the userdata becomes too large the session will end, apparently because of the cookie size. However we have a ci_sessions table in the database and are using these options:

$config['sess_encrypt_cookie'] = TRUE;
$config['sess_use_database'] = TRUE;

Why is codeigniter still using the cookie even though we've set to use database? Thank you for your time.

Contributor

narfbg commented Jul 10, 2017

There's a reason why CI2 is no longer supported, and why the session library was completely rewritten in CI3. Stop using it.

@narfbg narfbg locked and limited conversation to collaborators Jul 10, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.