prevent crash when cookie doesn't contain "--" #523

Merged
merged 1 commit into from Apr 23, 2013

4 participants

@bdimcheff

This backports 881ce76 so that rack won't crash when there isn't a "--" in the rack_session cookie

@bdimcheff bdimcheff prevent crash when cookie doesn't contain "--"
This backports 881ce76 so that rack
won't crash when there isn't a "--" in the rack_session cookie
18a6b88
@spastorino spastorino commented on the diff Feb 22, 2013
lib/rack/session/cookie.rb
@@ -55,7 +55,7 @@ def load_session(env)
if @secret && session_data
session_data, digest = session_data.split("--")
- session_data = nil unless Utils.secure_compare(digest, generate_hmac(session_data))
+ session_data = nil unless session_data && digest && Utils.secure_compare(digest, generate_hmac(session_data))
@spastorino
Official Rack repositories member

just checking for digest is enough

hmm I think that if the cookie is the empty string (and thus session_data is nil) it might error without session_data &&. I'll play around with the tests again in a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@spastorino
Official Rack repositories member

This one seems ok /cc @rkh @raggi

@rkh
Official Rack repositories member

Except the tests fail.

@spastorino spastorino added a commit that referenced this pull request Apr 22, 2013
@spastorino spastorino prevent crash when cookie doesn't contain "--"
This backports 881ce76 so that rack
won't crash when there isn't a "--" in the rack_session cookie

Fixes #523
a0ffb24
@spastorino spastorino added a commit that referenced this pull request Apr 22, 2013
@spastorino spastorino prevent crash when cookie doesn't contain "--"
This backports 881ce76 so that rack
won't crash when there isn't a "--" in the rack_session cookie

Fixes #523

Conflicts:
	lib/rack/session/cookie.rb
	test/spec_session_cookie.rb
65d417e
@spastorino spastorino merged commit 4b640a0 into rack:rack-1.1 Apr 23, 2013

1 check failed

Details default The Travis build failed
@spastorino
Official Rack repositories member

For the record Travis is failing because rack-1.1 didn't have a Gemfile. Let's fix that ;)

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