Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements and Bug Fixes for gae-sessions v1.07 #35

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Improvements and Bug Fixes for gae-sessions v1.07 #35

wants to merge 1 commit into from

Conversation

TheRook
Copy link

@TheRook TheRook commented Apr 13, 2012

(efficiency)Session id length cut in more than half
(efficiency)timestamp in SID has been reduced form 10 bytes to 4 bytes with bit packing
(efficiency)add a cron job to the demos to delete stale cookies
(efficiency)Hash functions used to generate session id cut from 3 to just 1

(Security)SIDs contain more entropy
(Security)removed use of md5()

(bug)SID_LEN can now be changed without causing offset exceptions
(bug)delete_expired_sessions function was not checking remaining cookies properly

	(efficiency)Session id length cut in more than half
	(efficiency)timestamp in SID has been reduced form 10 bytes to 4 bytes with bit packing
	(efficiency)add a cron job to the demos to delete stale cookies
	(efficiency)Hash functions used to generate session id cut from 3 to just 1

	(Security)SIDs contain more entropy
	(Security)removed use of md5()

	(bug)SID_LEN can now be changed without causing offset exceptions
	(bug)delete_expired_sessions function was not checking remaining cookies properly
@erkez
Copy link

erkez commented Sep 3, 2012

This pull request introduces bugs to the latest's version. It doesn't really fix anything besides returning if there are more sessions to be deleted (not a bug - it is well explained in the code). The unpacking of the SID's timestamp is broken, because it's being encoded as base64 when generated and the decoding doesn't happen when the cookie is read; decoding wouldn't work either, because the base64 string is greater than of the SID's length (32). Just check the cookie on your browser to see if it reflects the expiration you defined; it doesn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants