Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Switch to Rack::Response#set_cookie instead of using CGI::Cookie to b…

…uild cookie headers
  • Loading branch information...
commit 3b317b7100c9a416f4e3545f3844f0c0743acdb2 1 parent 606cd61
@josh josh authored
View
4 actionpack/lib/action_controller/base.rb
@@ -254,7 +254,7 @@ class Base
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 @@ def initialize_template_class(response)
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
View
38 actionpack/lib/action_controller/cookies.rb
@@ -64,45 +64,31 @@ def initialize(controller)
# 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 <tt>[]=</tt>, you can pass in
# an options hash to delete cookies with extra data such as a <tt>:path</tt>.
- 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
View
54 actionpack/lib/action_controller/response.rb
@@ -34,14 +34,14 @@ class Response < Rack::Response
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 @@ def prepare!
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 @@ def write(str)
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 @@ def convert_expires!
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
View
5 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
#
View
6 actionpack/lib/action_controller/test_process.rb
@@ -260,14 +260,14 @@ def has_template_object?(name=nil)
!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
View
66 actionpack/test/controller/cookie_test.rb
@@ -52,33 +52,33 @@ def setup
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 @@ def test_setting_test_cookie
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 @@ def test_cookiejar_accessor
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
Please sign in to comment.
Something went wrong with that request. Please try again.