rack/rack — pull request #599 by atotic (2013-08-31 07:53:23+00:00)
Title: Do not send multiple cookies with same name/domain/path

I've triggered this bug in the wild. I had a client polling rack server every second, and connection was kept alive. All of these are necessary to trigger the bug.

Bug:
I use 'rack.session' with :expires setting.
This causes Rack::Session::Abstract::ID.set_cookie to call
Utils.set_cookie_header for every response.

Every time Utils.set_cookie_header was called, new cookie was added. It was a duplicate of an existing session cookie, but because of :expires flag, cookie was being saved anyway.

Because I was polling every second, and keep-alive kept the connection open, the number of cookie headers kept growing. I noticed the bug when my response headers were 250K! Simple HTTP dump is attached showing the respone.

Fix:
Fix can either happen in Rack::Session::Abstract::ID.set_cookie, or in Utils.set_cookie_header.

I was unclear why does Rack::Session::Abstract::ID.set_cookie always sets the cookie if session has :expires set, so I did not touch that code.

I decided that the right fix is inside Utils.set_cookie_header. According to cookie spec, RFC 6265 section 4.1.2
"If the user agent receives a new cookie with the same cookie-name,
domain-value, and path-value as a cookie that it has already stored, the existing cookie is evicted and replaced with the new cookie."

I've modified Rack::Utils.set_cookie header to remove other cookies with same name/domain/path.

Sample bug session:

~> telnet localhost 27000
Trying ::1...
telnet: connect to address ::1: Connection refused
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
GET /get_work HTTP/1.1
hostname: localhost

HTTP/1.1 204 No Content
Content-Type: text/plain
Content-Length: 0
Set-Cookie: rack.session=%7B%22session_id%22%3A%22e54c5fc4f77c4349%22%2C%22user_id%22%3A2%2C%22user_id_expires%22%3A1377981804%7D--bb1c857cf9e6e5e776f791de4a4ec6d1a5905e80; path=/; expires=Sat, 31 Aug 2013 20:43:24 -0000
Set-Cookie: rack.session=%7B%22session_id%22%3A%22e54c5fc4f77c4349%22%2C%22user_id%22%3A2%2C%22user_id_expires%22%3A1377981804%7D--bb1c857cf9e6e5e776f791de4a4ec6d1a5905e80; path=/; expires=Sat, 31 Aug 2013 20:43:24 -0000
Set-Cookie: rack.session=%7B%22session_id%22%3A%22e54c5fc4f77c4349%22%2C%22user_id%22%3A2%2C%22user_id_expires%22%3A1377981804%7D--bb1c857cf9e6e5e776f791de4a4ec6d1a5905e80; path=/; expires=Sat, 31 Aug 2013 20:43:24 -0000
Set-Cookie: rack.session=%7B%22session_id%22%3A%22e54c5fc4f77c4349%22%2C%22user_id%22%3A2%2C%22user_id_expires%22%3A1377981804%7D--bb1c857cf9e6e5e776f791de4a4ec6d1a5905e80; path=/; expires=Sat, 31 Aug 2013 20:43:24 -0000
Set-Cookie: rack.session=%7B%22session_id%22%3A%22e54c5fc4f77c4349%22%2C%22user_id%22%3A2%2C%22user_id_expires%22%3A1377981804%7D--bb1c857cf9e6e5e776f791de4a4ec6d1a5905e80; path=/; expires=Sat, 31 Aug 2013 20:43:24 -0000
Set-Cookie: rack.session=%7B%22session_id%22%3A%22e54c5fc4f77c4349%22%2C%22user_id%22%3A2%2C%22user_id_expires%22%3A1377981804%7D--bb1c857cf9e6e5e776f791de4a4ec6d1a5905e80; path=/; expires=Sat, 31 Aug 2013 20:43:24 -0000
Set-Cookie: rack.session=%7B%22session_id%22%3A%22e54c5fc4f77c4349%22%2C%22user_id%22%3A2%2C%22user_id_expires%22%3A1377981804%7D--bb1c857cf9e6e5e776f791de4a4ec6d1a5905e80; path=/; expires=Sat, 31 Aug 2013 20:43:24 -0000