This project is archived and is in readonly mode.
[PATCH] Session fixes: sessions should not be created until written to; and session data should be destroyed on session reset
-
Michael Lovitt
- Tag set to patch
-
Michael Lovitt
- Tag changed from patch to 3.x, patch
-
Jeremy Kemper
- State changed from new to open
Great patch, Michael. Could you backport to 2-3-stable as well?
-
Repository
(from [49f52c3d910c8f183afc3a54ea2ae9667f23085e]) Sessions should not be created until written to and session data should be destroyed on reset.
[#4938]
Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/49f52c3d910c8f183afc3a54ea2ae9... -
Michael Lovitt
Jeremy, a backported patch for 2-3-stable is attached. Thanks!
-
Jeremy Kemper
- Milestone set to 2.3.9
See also http://github.com/rails/rails/commit/d69ebb849a78c07a4efc869789c4bc... - needs test coverage.
-
Michael Lovitt
José Valim tells me that the repro steps for his issue are to store a class in the session when using the cookie store; problems were occurring during deserialization. I'll attempt to repro myself and add some test coverage, and will submit an updated patch (incorporating José's changes) for 2-3-stable.
-
Michael Lovitt
- Importance changed from to Low
José committed a revised fix for the deserialization issue:
http://github.com/rails/rails/commit/21c99e93883c1cf32474ad65a507e6...
The specific error condition: the cookie store contains a serialized object of a class that has not yet been loaded into the environment. Auto-loading missing classes is generally handled for all stores within the abstract store, but was failing to happen under certain conditions in the case of the cookie store.
While adding test coverage for José's fix, I discovered one more broken case: when the cookie store contains a serialized object of an unloaded class, an error occurs if the session id is read before the session data is read. I've attached a patch for this, which also tidies up some of the related code, and includes test coverage for all of the above, so hopefully this behavior won't get broken again.
To come: a revised 2-3-master patch that includes my original improvements plus these latest fixes and tests.
-
Repository
(from [ebee77a28a7267d5f23a28ba23c1eb88a2d7d527]) Fixed that an ArgumentError is thrown when request.session_options[:id] is read in the following scenario: when the cookie store is used, and the session contains a serialized object of an unloaded class, and no session data accesses have occurred yet. Pushed the stale_session_check responsibility out of the SessionHash and down into the session store, closer to where the deserialization actually occurs. Added some test coverage for this case and others related to deserialization of unloaded types.
[#4938]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/ebee77a28a7267d5f23a28ba23c1eb... -
José Valim
Thanks Michael!
-
Michael Lovitt
- Tag changed from 3.x, patch to 2.3.x, 3.x, patch
Ok, an updated 2-3-stable patch is attached. It includes all the original session improvements and associated tests, plus the work José and I both did on the cookie store deserialization issues.
-
Santiago Pastorino
- Assigned user set to José Valim
-
José Valim
Michael, after applying your patch I get three failures on Ruby 1.9.2. Could you please investigate? Thanks a lot!
-
Michael Lovitt
José: I was unable to reproduce the failures you mentioned. After applying the patch, all actionpack tests pass for me on both Ruby 1.9.2 and 1.8.7.
But the patch no longer applies cleanly to 2-3-stable, due to some recent unrelated session commits, so I've attached a new one. José, if you still get failures with this patch, could you tell me the specific failures you're seeing as well as the specific 1.9.2 release you're using?
Fotos: I will take a closer look at #4450 and will respond in the case.
-
Repository
- State changed from open to resolved
(from [257a29d3cca91913325a8bfef0f06b7ec6c4654b]) Sessions should not be created until written to and session data should be destroyed on reset. [#4938 state:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/257a29d3cca91913325a8bfef0f06b... -
PacoGuzman
Hi!
I'm calling 2 times reset_session in the same controller action (in a specific condition), but after update my up from 2.3.8 to 2.3.9, I'm getting the following error:
undefined method
destroy' for {}:HashInside ActionController::Request#reset_session
Any advice?
