From 3b317b7100c9a416f4e3545f3844f0c0743acdb2 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 20 Dec 2008 21:25:09 -0600 Subject: Switch to Rack::Response#set_cookie instead of using CGI::Cookie to build cookie headers --- actionpack/lib/action_controller/base.rb | 4 +- actionpack/lib/action_controller/cookies.rb | 38 +++++--------- actionpack/lib/action_controller/response.rb | 54 ++++++++++++------- actionpack/lib/action_controller/test_case.rb | 5 +- actionpack/lib/action_controller/test_process.rb | 6 +-- actionpack/test/controller/cookie_test.rb | 66 +++++------------------- 6 files changed, 66 insertions(+), 107 deletions(-) diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index eae17d6dd5..3e001a2ed6 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -254,7 +254,7 @@ module ActionController #:nodoc: cattr_reader :protected_instance_variables # Controller specific instance variables which will not be accessible inside views. @@protected_instance_variables = %w(@assigns @performed_redirect @performed_render @variables_added @request_origin @url @parent_controller - @action_name @before_filter_chain_aborted @action_cache_path @_session @_cookies @_headers @_params + @action_name @before_filter_chain_aborted @action_cache_path @_session @_headers @_params @_flash @_response) # Prepends all the URL-generating helpers from AssetHelper. This makes it possible to easily move javascripts, stylesheets, @@ -1193,7 +1193,7 @@ module ActionController #:nodoc: end def assign_shortcuts(request, response) - @_request, @_params, @_cookies = request, request.parameters, request.cookies + @_request, @_params = request, request.parameters @_response = response @_response.session = request.session diff --git a/actionpack/lib/action_controller/cookies.rb b/actionpack/lib/action_controller/cookies.rb index 0e058085ec..840ceb5abd 100644 --- a/actionpack/lib/action_controller/cookies.rb +++ b/actionpack/lib/action_controller/cookies.rb @@ -64,45 +64,31 @@ module ActionController #:nodoc: # Returns the value of the cookie by +name+, or +nil+ if no such cookie exists. def [](name) - cookie = @cookies[name.to_s] - if cookie && cookie.respond_to?(:value) - cookie.size > 1 ? cookie.value : cookie.value[0] - else - cookie - end + super(name.to_s) end # Sets the cookie named +name+. The second argument may be the very cookie # value, or a hash of options as documented above. - def []=(name, options) + def []=(key, options) if options.is_a?(Hash) - options = options.inject({}) { |options, pair| options[pair.first.to_s] = pair.last; options } - options["name"] = name.to_s + options.symbolize_keys! else - options = { "name" => name.to_s, "value" => options } + options = { :value => options } end - set_cookie(options) + options[:path] = "/" unless options.has_key?(:path) + super(key.to_s, options[:value]) + @controller.response.set_cookie(key, options) end # Removes the cookie on the client machine by setting the value to an empty string # and setting its expiration date into the past. Like []=, you can pass in # an options hash to delete cookies with extra data such as a :path. - def delete(name, options = {}) - options.stringify_keys! - set_cookie(options.merge("name" => name.to_s, "value" => "", "expires" => Time.at(0))) + def delete(key, options = {}) + options.symbolize_keys! + options[:path] = "/" unless options.has_key?(:path) + super(key.to_s) + @controller.response.delete_cookie(key, options) end - - private - # Builds a CGI::Cookie object and adds the cookie to the response headers. - # - # The path of the cookie defaults to "/" if there's none in +options+, and - # everything is passed to the CGI::Cookie constructor. - def set_cookie(options) #:doc: - options["path"] = "/" unless options["path"] - cookie = CGI::Cookie.new(options) - @controller.logger.info "Cookie set: #{cookie}" unless @controller.logger.nil? - @controller.response.headers["cookie"] << cookie - end end end diff --git a/actionpack/lib/action_controller/response.rb b/actionpack/lib/action_controller/response.rb index 866616bac3..64319fe102 100644 --- a/actionpack/lib/action_controller/response.rb +++ b/actionpack/lib/action_controller/response.rb @@ -34,14 +34,14 @@ module ActionController # :nodoc: DEFAULT_HEADERS = { "Cache-Control" => "no-cache" } attr_accessor :request - attr_accessor :session, :cookies, :assigns, :template, :layout + attr_accessor :session, :assigns, :template, :layout attr_accessor :redirected_to, :redirected_to_method_params delegate :default_charset, :to => 'ActionController::Base' def initialize @status = 200 - @header = DEFAULT_HEADERS.merge("cookie" => []) + @header = DEFAULT_HEADERS.dup @writer = lambda { |x| @body << x } @block = nil @@ -143,10 +143,9 @@ module ActionController # :nodoc: handle_conditional_get! set_content_length! convert_content_type! - convert_language! convert_expires! - set_cookies! + convert_cookies! end def each(&callback) @@ -168,6 +167,35 @@ module ActionController # :nodoc: str end + # Over Rack::Response#set_cookie to add HttpOnly option + def set_cookie(key, value) + case value + when Hash + domain = "; domain=" + value[:domain] if value[:domain] + path = "; path=" + value[:path] if value[:path] + # According to RFC 2109, we need dashes here. + # N.B.: cgi.rb uses spaces... + expires = "; expires=" + value[:expires].clone.gmtime. + strftime("%a, %d-%b-%Y %H:%M:%S GMT") if value[:expires] + secure = "; secure" if value[:secure] + httponly = "; HttpOnly" if value[:http_only] + value = value[:value] + end + value = [value] unless Array === value + cookie = ::Rack::Utils.escape(key) + "=" + + value.map { |v| ::Rack::Utils.escape v }.join("&") + + "#{domain}#{path}#{expires}#{secure}#{httponly}" + + case self["Set-Cookie"] + when Array + self["Set-Cookie"] << cookie + when String + self["Set-Cookie"] = [self["Set-Cookie"], cookie] + when nil + self["Set-Cookie"] = cookie + end + end + private def handle_conditional_get! if etag? || last_modified? @@ -217,22 +245,8 @@ module ActionController # :nodoc: headers["Expires"] = headers.delete("") if headers["expires"] end - def set_cookies! - # Convert 'cookie' header to 'Set-Cookie' headers. - # Because Set-Cookie header can appear more the once in the response body, - # we store it in a line break separated string that will be translated to - # multiple Set-Cookie header by the handler. - if cookie = headers.delete('cookie') - cookies = [] - - case cookie - when Array then cookie.each { |c| cookies << c.to_s } - when Hash then cookie.each { |_, c| cookies << c.to_s } - else cookies << cookie.to_s - end - - headers['Set-Cookie'] = [headers['Set-Cookie'], cookies].flatten.compact - end + def convert_cookies! + headers['Set-Cookie'] = Array(headers['Set-Cookie']).compact end end end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 79a8e1364d..7ed1a3e160 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -93,10 +93,7 @@ module ActionController # and cookies, though. For sessions, you just do: # # @request.session[:key] = "value" - # - # For cookies, you need to manually create the cookie, like this: - # - # @request.cookies["key"] = CGI::Cookie.new("key", "value") + # @request.cookies["key"] = "value" # # == Testing named routes # diff --git a/actionpack/lib/action_controller/test_process.rb b/actionpack/lib/action_controller/test_process.rb index c4d7d52951..45dcf8b2c2 100644 --- a/actionpack/lib/action_controller/test_process.rb +++ b/actionpack/lib/action_controller/test_process.rb @@ -260,14 +260,14 @@ module ActionController #:nodoc: !template_objects[name].nil? end - # Returns the response cookies, converted to a Hash of (name => CGI::Cookie) pairs + # Returns the response cookies, converted to a Hash of (name => value) pairs # - # assert_equal ['AuthorOfNewPage'], r.cookies['author'].value + # assert_equal 'AuthorOfNewPage', r.cookies['author'] def cookies cookies = {} Array(headers['Set-Cookie']).each do |cookie| key, value = cookie.split(";").first.split("=") - cookies[key] = [value].compact + cookies[key] = value end cookies end diff --git a/actionpack/test/controller/cookie_test.rb b/actionpack/test/controller/cookie_test.rb index 4b969519c6..84d272b1c7 100644 --- a/actionpack/test/controller/cookie_test.rb +++ b/actionpack/test/controller/cookie_test.rb @@ -52,33 +52,33 @@ class CookieTest < Test::Unit::TestCase def test_setting_cookie get :authenticate assert_equal ["user_name=david; path=/"], @response.headers["Set-Cookie"] - assert_equal({"user_name" => ["david"]}, @response.cookies) + assert_equal({"user_name" => "david"}, @response.cookies) end def test_setting_cookie_for_fourteen_days get :authenticate_for_fourteen_days - assert_equal ["user_name=david; path=/; expires=Mon, 10 Oct 2005 05:00:00 GMT"], @response.headers["Set-Cookie"] - assert_equal({"user_name" => ["david"]}, @response.cookies) + assert_equal ["user_name=david; path=/; expires=Mon, 10-Oct-2005 05:00:00 GMT"], @response.headers["Set-Cookie"] + assert_equal({"user_name" => "david"}, @response.cookies) end def test_setting_cookie_for_fourteen_days_with_symbols get :authenticate_for_fourteen_days_with_symbols - assert_equal ["user_name=david; path=/; expires=Mon, 10 Oct 2005 05:00:00 GMT"], @response.headers["Set-Cookie"] - assert_equal({"user_name" => ["david"]}, @response.cookies) + assert_equal ["user_name=david; path=/; expires=Mon, 10-Oct-2005 05:00:00 GMT"], @response.headers["Set-Cookie"] + assert_equal({"user_name" => "david"}, @response.cookies) end def test_setting_cookie_with_http_only get :authenticate_with_http_only assert_equal ["user_name=david; path=/; HttpOnly"], @response.headers["Set-Cookie"] - assert_equal({"user_name" => ["david"]}, @response.cookies) + assert_equal({"user_name" => "david"}, @response.cookies) end def test_multiple_cookies get :set_multiple_cookies assert_equal 2, @response.cookies.size - assert_equal "user_name=david; path=/; expires=Mon, 10 Oct 2005 05:00:00 GMT", @response.headers["Set-Cookie"][0] + assert_equal "user_name=david; path=/; expires=Mon, 10-Oct-2005 05:00:00 GMT", @response.headers["Set-Cookie"][0] assert_equal "login=XJ-122; path=/", @response.headers["Set-Cookie"][1] - assert_equal({"login" => ["XJ-122"], "user_name" => ["david"]}, @response.cookies) + assert_equal({"login" => "XJ-122", "user_name" => "david"}, @response.cookies) end def test_setting_test_cookie @@ -87,12 +87,12 @@ class CookieTest < Test::Unit::TestCase def test_expiring_cookie get :logout - assert_equal ["user_name=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT"], @response.headers["Set-Cookie"] - assert_equal({"user_name" => []}, @response.cookies) + assert_equal ["user_name=; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT"], @response.headers["Set-Cookie"] + assert_equal({"user_name" => nil}, @response.cookies) end def test_cookiejar_accessor - @request.cookies["user_name"] = CGI::Cookie.new("name" => "user_name", "value" => "david", "expires" => Time.local(2025, 10, 10)) + @request.cookies["user_name"] = "david" @controller.request = @request jar = ActionController::CookieJar.new(@controller) assert_equal "david", jar["user_name"] @@ -100,52 +100,14 @@ class CookieTest < Test::Unit::TestCase end def test_cookiejar_accessor_with_array_value - a = %w{1 2 3} - @request.cookies["pages"] = CGI::Cookie.new("name" => "pages", "value" => a, "expires" => Time.local(2025, 10, 10)) + @request.cookies["pages"] = %w{1 2 3} @controller.request = @request jar = ActionController::CookieJar.new(@controller) - assert_equal a, jar["pages"] + assert_equal %w{1 2 3}, jar["pages"] end def test_delete_cookie_with_path get :delete_cookie_with_path - assert_equal ["user_name=; path=/beaten; expires=Thu, 01 Jan 1970 00:00:00 GMT"], @response.headers["Set-Cookie"] - end - - def test_cookie_to_s_simple_values - assert_equal 'myname=myvalue; path=', CGI::Cookie.new('myname', 'myvalue').to_s - end - - def test_cookie_to_s_hash - cookie_str = CGI::Cookie.new( - 'name' => 'myname', - 'value' => 'myvalue', - 'domain' => 'mydomain', - 'path' => 'mypath', - 'expires' => Time.utc(2007, 10, 20), - 'secure' => true, - 'http_only' => true).to_s - assert_equal 'myname=myvalue; domain=mydomain; path=mypath; expires=Sat, 20 Oct 2007 00:00:00 GMT; secure; HttpOnly', cookie_str - end - - def test_cookie_to_s_hash_default_not_secure_not_http_only - cookie_str = CGI::Cookie.new( - 'name' => 'myname', - 'value' => 'myvalue', - 'domain' => 'mydomain', - 'path' => 'mypath', - 'expires' => Time.utc(2007, 10, 20)) - assert cookie_str !~ /secure/ - assert cookie_str !~ /HttpOnly/ - end - - def test_cookies_should_not_be_split_on_ampersand_values - cookies = CGI::Cookie.parse('return_to=http://rubyonrails.org/search?term=api&scope=all&global=true') - assert_equal({"return_to" => ["http://rubyonrails.org/search?term=api&scope=all&global=true"]}, cookies) - end - - def test_cookies_should_not_be_split_on_values_with_newlines - cookies = CGI::Cookie.new("name" => "val", "value" => "this\nis\na\ntest") - assert cookies.size == 1 + assert_equal ["user_name=; path=/beaten; expires=Thu, 01-Jan-1970 00:00:00 GMT"], @response.headers["Set-Cookie"] end end -- cgit v1.2.3