Bug in native session library (sess_update)?

[eluser]ciKD[/eluser]
Do you think you can post it in the bug-forum? So everybody can see it?

[eluser]sheldonnbbaker[/eluser]
[quote author="ciKD" date="1270342647"]Do you think you can post it in the bug-forum? So everybody can see it?[/quote]

If I find time this week to clean it up and document the changes, I'll post it.

[eluser]ciKD[/eluser]
Thanks from me and probably many others!

[eluser]Vik[/eluser]
Sheldon, did you post your updated code yet? If not, can you email it to me by any chance?

[eluser]WanWizard[/eluser]
A quick analysis shows that what happens is

- [Req 1] browser requests a URI, session_id A is read
- [Req 2] browser requests a another URI, session_id A is read
- [Req 2] sess_update() is called, session_id B is written to the cookie
- [Req 2] request is processed quickly
- [Req 2] calls sess_write(), which updates the session and writes the cookie for session_id B
- [Req 1] now finishes
- [Req 1] calls sess_write(), which updates the session and writes the cookie (with the old session_id A!)

To fix this, sess_write() should check the result of $this->db->update, as when this happens, the record with the old session ID no longer exists. So if no rows were updated, the session_id was invalid, and sess_write() should not call $this->_set_cookie() !

You will then have the problem that you loose all modifications to the session, since it's not written to the database because of the now invalid session_id. As suggested this could be fixed by storing the old session_id in the session record and checking that as well when an update is done. Downside is that your session_id rotation time doubles, as both the current and the previous session_id would be valid, so you would have to take that into account when defining the session_id update time.

[eluser]WanWizard[/eluser]
For those interested: you can find my session library extension here, which now contains a fix for this issue (and fixes some other bugs as well).

Note that this needs an extra field 'previous_session_id' in the sessions table

[eluser]Vik[/eluser]
Thank you very much, WanWizard!

[eluser]WanWizard[/eluser]
Be careful when you implement it, as it has sess_write() disabled by default.

By default, CI does an update of the session record very time you use set_userdata(). Which I find unnecessary, it adds dozens of extra queries to a the processing of a URI request.
All ExiteCMS requests are routed through the same controller, at the end I call 'sess_write(TRUE)' to write the session. I also added this to my redirect helper, as CI assumes the session updates are already written when you redirect.

If you want the default CI behaviour back, change the sess_write() method definition to read:

Code:

function sess_write( $write = TRUE)