Naming convention in sessions #2702

Closed
aanbar opened this Issue Oct 28, 2013 · 9 comments

Comments

Projects
None yet
4 participants
Contributor

aanbar commented Oct 28, 2013

so I spent 2 hours debugging an issue on why a script I am working on doesn't process the logins, Only to find out that there's a field named 'last_activity' in the session that's being overridden in my table.

I thought all session related variables have the "sess_" prefix, leaving this field without a prefix isn't a good idea (for now I had to change my field name to skip messing with the library).

Contributor

narfbg commented Oct 28, 2013

I think somebody else reported the same "problem" some time ago, but I am yet to understand - in what scenario does Session's last_activity override your code and what's the problem with simply using that value?

Contributor

aanbar commented Oct 28, 2013

in my case 'last_activity' contains a mysql datetime which is not updated on each action but on specific actions, while the session's last_activity is updated everytime no matter what.

the comparison in the session class assumes that last_activity is a timestamp that's why it destroys the session when it cannot calculate things.

Contributor

narfbg commented Oct 28, 2013

Ok, what do you propose then? If that's to be changed, it must maintain backwards compatibility at least to some extent.

Contributor

aanbar commented Oct 29, 2013

If this is a 'NO-CAN-DO' thing, then at least there should be a note in session documentation so no one mess with it.

It's a nightmare when you think everything works fine then it just destroys the session because of this.

Something like:

NOTE: session class uses a key 'last_activity' that's being updated automatically, the field uses timestamp and WILL destroy your session if populated by anything other than timestamp.

Contributor

narfbg commented Oct 29, 2013

Sure, I understand what the issue is, but while it causes a collision in your case, somebody else is probably using it as a feature - that's why it must maintain BC. I'm not saying that it's an absolute NO-CAN-DO thing, but if you want it changed it must happen in a way that preserves the value accessible.

Contributor

vlakoff commented Apr 12, 2014

CI writes other session fields too, namely ip_address and user_agent.

Considering the quite high probability of collision, and the pain to debug as demonstrated here, I'd suggest renaming these fields to ci_last_activity etc. and adding an upgrade note.

Sadly I can't see how we could keep BC. However, if users were directly accessing CI internals as a feature, I think it is up to them to update their code.

Contributor

jim-parry commented Nov 12, 2014

Superceded bu #3073? Can this be closed, then?

Contributor

narfbg commented Nov 12, 2014

It will be closed when #3073 is closed.

jim-parry removed the Dead Issue? label Nov 12, 2014

narfbg removed the Bug label Nov 12, 2014

Contributor

narfbg commented Jan 19, 2015

Fixed by #3073.

narfbg closed this Jan 19, 2015

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