diff options
Diffstat (limited to 'actionpack')
40 files changed, 711 insertions, 462 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 4db9c4b84d..54c7771f4c 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Rails 3.0.0 [beta 4/release candidate] (unreleased)* +* Make session stores rely on request.cookie_jar and change set_session semantics to return the cookie value instead of a boolean. [José Valim] + * OAuth 2: HTTP Token Authorization support to complement Basic and Digest Authorization. [Rick Olson] * Fixed inconsistencies in form builder and view helpers #4432 [Neeraj Singh] diff --git a/actionpack/lib/action_controller/metal/helpers.rb b/actionpack/lib/action_controller/metal/helpers.rb index 1110d7c117..b6d4fb1284 100644 --- a/actionpack/lib/action_controller/metal/helpers.rb +++ b/actionpack/lib/action_controller/metal/helpers.rb @@ -104,7 +104,7 @@ module ActionController def all_application_helpers helpers = [] helpers_path.each do |path| - extract = /^#{Regexp.quote(path)}\/?(.*)_helper.rb$/ + extract = /^#{Regexp.quote(path.to_s)}\/?(.*)_helper.rb$/ helpers += Dir["#{path}/**/*_helper.rb"].map { |file| file.sub(extract, '\1') } end helpers.sort! diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 8b730a97ee..3b85a98576 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -140,7 +140,7 @@ module ActionDispatch # :nodoc: def to_a assign_default_content_type_and_charset! handle_conditional_get! - self["Set-Cookie"] = @cookie.join("\n") unless @cookie.blank? + self["Set-Cookie"] = self["Set-Cookie"].join("\n") if self["Set-Cookie"].respond_to?(:join) self["ETag"] = @_etag if @_etag super end @@ -170,7 +170,7 @@ module ActionDispatch # :nodoc: # assert_equal 'AuthorOfNewPage', r.cookies['author'] def cookies cookies = {} - if header = @cookie + if header = self["Set-Cookie"] header = header.split("\n") if header.respond_to?(:to_str) header.each do |cookie| if pair = cookie.split(';').first @@ -182,37 +182,6 @@ module ActionDispatch # :nodoc: cookies end - 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[:httponly] - 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}" - - @cookie << cookie - end - - def delete_cookie(key, value={}) - @cookie.reject! { |cookie| - cookie =~ /\A#{Rack::Utils.escape(key)}=/ - } - - set_cookie(key, - {:value => '', :path => nil, :domain => nil, - :expires => Time.at(0) }.merge(value)) - end - private def assign_default_content_type_and_charset! return if headers[CONTENT_TYPE].present? diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 42ab1d1ebb..87e8dd5010 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -52,9 +52,15 @@ module ActionDispatch # * <tt>:httponly</tt> - Whether this cookie is accessible via scripting or # only HTTP. Defaults to +false+. class Cookies + HTTP_HEADER = "Set-Cookie".freeze + TOKEN_KEY = "action_dispatch.secret_token".freeze + + # Raised when storing more than 4K of session data. + class CookieOverflow < StandardError; end + class CookieJar < Hash #:nodoc: def self.build(request) - secret = request.env["action_dispatch.secret_token"] + secret = request.env[TOKEN_KEY] new(secret).tap do |hash| hash.update(request.cookies) end @@ -134,9 +140,9 @@ module ActionDispatch @signed ||= SignedCookieJar.new(self, @secret) end - def write(response) - @set_cookies.each { |k, v| response.set_cookie(k, v) } - @delete_cookies.each { |k, v| response.delete_cookie(k, v) } + def write(headers) + @set_cookies.each { |k, v| ::Rack::Utils.set_cookie_header!(headers, k, v) } + @delete_cookies.each { |k, v| ::Rack::Utils.delete_cookie_header!(headers, k, v) } end end @@ -166,8 +172,11 @@ module ActionDispatch end class SignedCookieJar < CookieJar #:nodoc: + MAX_COOKIE_SIZE = 4096 # Cookies can typically store 4096 bytes. + SECRET_MIN_LENGTH = 30 # Characters + def initialize(parent_jar, secret) - raise "You must set config.secret_token in your app's config" if secret.blank? + ensure_secret_secure(secret) @parent_jar = parent_jar @verifier = ActiveSupport::MessageVerifier.new(secret) end @@ -176,6 +185,8 @@ module ActionDispatch if signed_message = @parent_jar[name] @verifier.verify(signed_message) end + rescue ActiveSupport::MessageVerifier::InvalidSignature + nil end def []=(key, options) @@ -186,12 +197,34 @@ module ActionDispatch options = { :value => @verifier.generate(options) } end + raise CookieOverflow if options[:value].size > MAX_COOKIE_SIZE @parent_jar[key] = options end def method_missing(method, *arguments, &block) @parent_jar.send(method, *arguments, &block) end + + protected + + # To prevent users from using something insecure like "Password" we make sure that the + # secret they've provided is at least 30 characters in length. + def ensure_secret_secure(secret) + if secret.blank? + raise ArgumentError, "A secret is required to generate an " + + "integrity hash for cookie session data. Use " + + "config.secret_token = \"some secret phrase of at " + + "least #{SECRET_MIN_LENGTH} characters\"" + + "in config/application.rb" + end + + if secret.length < SECRET_MIN_LENGTH + raise ArgumentError, "Secret should be something secure, " + + "like \"#{ActiveSupport::SecureRandom.hex(16)}\". The value you " + + "provided, \"#{secret}\", is shorter than the minimum length " + + "of #{SECRET_MIN_LENGTH} characters" + end + end end def initialize(app) @@ -202,12 +235,13 @@ module ActionDispatch status, headers, body = @app.call(env) if cookie_jar = env['action_dispatch.cookies'] - response = Rack::Response.new(body, status, headers) - cookie_jar.write(response) - response.to_a - else - [status, headers, body] + cookie_jar.write(headers) + if headers[HTTP_HEADER].respond_to?(:join) + headers[HTTP_HEADER] = headers[HTTP_HEADER].join("\n") + end end + + [status, headers, body] end end end diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index dddedc832f..15493cd2eb 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -1,5 +1,6 @@ require 'rack/utils' require 'rack/request' +require 'action_dispatch/middleware/cookies' require 'active_support/core_ext/object/blank' module ActionDispatch @@ -11,9 +12,6 @@ module ActionDispatch ENV_SESSION_KEY = 'rack.session'.freeze ENV_SESSION_OPTIONS_KEY = 'rack.session.options'.freeze - HTTP_COOKIE = 'HTTP_COOKIE'.freeze - SET_COOKIE = 'Set-Cookie'.freeze - class SessionHash < Hash def initialize(by, env) super() @@ -22,13 +20,6 @@ module ActionDispatch @loaded = false end - def session_id - ActiveSupport::Deprecation.warn( - "ActionDispatch::Session::AbstractStore::SessionHash#session_id " + - "has been deprecated. Please use request.session_options[:id] instead.", caller) - @env[ENV_SESSION_OPTIONS_KEY][:id] - end - def [](key) load! unless @loaded super(key.to_s) @@ -45,35 +36,14 @@ module ActionDispatch h end - def update(hash = nil) - if hash.nil? - ActiveSupport::Deprecation.warn('use replace instead', caller) - replace({}) - else - load! unless @loaded - super(hash.stringify_keys) - end - end - - def delete(key = nil) - if key.nil? - ActiveSupport::Deprecation.warn('use clear instead', caller) - clear - else - load! unless @loaded - super(key.to_s) - end - end - - def data - ActiveSupport::Deprecation.warn( - "ActionDispatch::Session::AbstractStore::SessionHash#data " + - "has been deprecated. Please use #to_hash instead.", caller) - to_hash + def update(hash) + load! unless @loaded + super(hash.stringify_keys) end - def close - ActiveSupport::Deprecation.warn('sessions should no longer be closed', caller) + def delete(key) + load! unless @loaded + super(key.to_s) end def inspect @@ -124,30 +94,15 @@ module ActionDispatch } def initialize(app, options = {}) - # Process legacy CGI options - options = options.symbolize_keys - if options.has_key?(:session_path) - options[:path] = options.delete(:session_path) - end - if options.has_key?(:session_key) - options[:key] = options.delete(:session_key) - end - if options.has_key?(:session_http_only) - options[:httponly] = options.delete(:session_http_only) - end - @app = app @default_options = DEFAULT_OPTIONS.merge(options) - @key = @default_options[:key] - @cookie_only = @default_options[:cookie_only] + @key = @default_options.delete(:key).freeze + @cookie_only = @default_options.delete(:cookie_only) + ensure_session_key! end def call(env) - session = SessionHash.new(self, env) - - env[ENV_SESSION_KEY] = session - env[ENV_SESSION_OPTIONS_KEY] = @default_options.dup - + prepare!(env) response = @app.call(env) session_data = env[ENV_SESSION_KEY] @@ -157,53 +112,62 @@ module ActionDispatch session_data.send(:load!) if session_data.is_a?(AbstractStore::SessionHash) && !session_data.send(:loaded?) sid = options[:id] || generate_sid + session_data = session_data.to_hash - unless set_session(env, sid, session_data.to_hash) - return response - end + value = set_session(env, sid, session_data) + return response unless value - cookie = Rack::Utils.escape(@key) + '=' + Rack::Utils.escape(sid) - cookie << "; domain=#{options[:domain]}" if options[:domain] - cookie << "; path=#{options[:path]}" if options[:path] - if options[:expire_after] - expiry = Time.now + options[:expire_after] - cookie << "; expires=#{expiry.httpdate}" - end - cookie << "; Secure" if options[:secure] - cookie << "; HttpOnly" if options[:httponly] - - headers = response[1] - unless headers[SET_COOKIE].blank? - headers[SET_COOKIE] << "\n#{cookie}" - else - headers[SET_COOKIE] = cookie + cookie = { :value => value } + unless options[:expire_after].nil? + cookie[:expires] = Time.now + options.delete(:expire_after) end + + request = ActionDispatch::Request.new(env) + set_cookie(request, cookie.merge!(options)) end response end private + + def prepare!(env) + env[ENV_SESSION_KEY] = SessionHash.new(self, env) + env[ENV_SESSION_OPTIONS_KEY] = @default_options.dup + end + def generate_sid ActiveSupport::SecureRandom.hex(16) end + def set_cookie(request, options) + request.cookie_jar[@key] = options + end + def load_session(env) request = Rack::Request.new(env) - sid = request.cookies[@key] - unless @cookie_only - sid ||= request.params[@key] - end + sid = request.cookies[@key] + sid ||= request.params[@key] unless @cookie_only sid, session = get_session(env, sid) [sid, session] end + def ensure_session_key! + if @key.blank? + raise ArgumentError, 'A key is required to write a ' + + 'cookie containing the session data. Use ' + + 'config.session_store SESSION_STORE, { :key => ' + + '"_myapp_session" } in config/application.rb' + end + end + def get_session(env, sid) raise '#get_session needs to be implemented.' end def set_session(env, sid, session_data) - raise '#set_session needs to be implemented.' + raise '#set_session needs to be implemented and should return ' << + 'the value to be stored in the cookie (usually the sid)' end end end diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index 7114f42003..92a86ee229 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -38,23 +38,11 @@ module ActionDispatch # "rake secret" and set the key in config/environment.rb. # # Note that changing digest or secret invalidates all existing sessions! - class CookieStore - # Cookies can typically store 4096 bytes. - MAX = 4096 - SECRET_MIN_LENGTH = 30 # characters - - DEFAULT_OPTIONS = { - :key => '_session_id', - :domain => nil, - :path => "/", - :expire_after => nil, - :httponly => true - }.freeze - + class CookieStore < AbstractStore class OptionsHash < Hash def initialize(by, env, default_options) - @session_data = env[CookieStore::ENV_SESSION_KEY] - default_options.each { |key, value| self[key] = value } + @session_data = env[AbstractStore::ENV_SESSION_KEY] + merge!(default_options) end def [](key) @@ -62,172 +50,38 @@ module ActionDispatch end end - ENV_SESSION_KEY = "rack.session".freeze - ENV_SESSION_OPTIONS_KEY = "rack.session.options".freeze - HTTP_SET_COOKIE = "Set-Cookie".freeze - - # Raised when storing more than 4K of session data. - class CookieOverflow < StandardError; end - def initialize(app, options = {}) - # Process legacy CGI options - options = options.symbolize_keys - if options.has_key?(:session_path) - options[:path] = options.delete(:session_path) - end - if options.has_key?(:session_key) - options[:key] = options.delete(:session_key) - end - if options.has_key?(:session_http_only) - options[:httponly] = options.delete(:session_http_only) - end - - @app = app - - # The session_key option is required. - ensure_session_key(options[:key]) - @key = options.delete(:key).freeze - - # The secret option is required. - ensure_secret_secure(options[:secret]) - @secret = options.delete(:secret).freeze - - @digest = options.delete(:digest) || 'SHA1' - @verifier = verifier_for(@secret, @digest) - - @default_options = DEFAULT_OPTIONS.merge(options).freeze - + super(app, options.merge!(:cookie_only => true)) freeze end - def call(env) - env[ENV_SESSION_KEY] = AbstractStore::SessionHash.new(self, env) - env[ENV_SESSION_OPTIONS_KEY] = OptionsHash.new(self, env, @default_options) - - status, headers, body = @app.call(env) - - session_data = env[ENV_SESSION_KEY] - options = env[ENV_SESSION_OPTIONS_KEY] - - if !session_data.is_a?(AbstractStore::SessionHash) || session_data.send(:loaded?) || options[:expire_after] - session_data.send(:load!) if session_data.is_a?(AbstractStore::SessionHash) && !session_data.send(:loaded?) - session_data = marshal(session_data.to_hash) - - raise CookieOverflow if session_data.size > MAX - - cookie = Hash.new - cookie[:value] = session_data - unless options[:expire_after].nil? - cookie[:expires] = Time.now + options[:expire_after] - end - - cookie = build_cookie(@key, cookie.merge(options)) - unless headers[HTTP_SET_COOKIE].blank? - headers[HTTP_SET_COOKIE] << "\n#{cookie}" - else - headers[HTTP_SET_COOKIE] = cookie - end - end - - [status, headers, body] - end - private - # Should be in Rack::Utils soon - def build_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[:httponly] - 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}" + + def prepare!(env) + env[ENV_SESSION_KEY] = SessionHash.new(self, env) + env[ENV_SESSION_OPTIONS_KEY] = OptionsHash.new(self, env, @default_options) end def load_session(env) - request = Rack::Request.new(env) - session_data = request.cookies[@key] - data = unmarshal(session_data) || persistent_session_id!({}) + request = ActionDispatch::Request.new(env) + data = request.cookie_jar.signed[@key] + data = persistent_session_id!(data) data.stringify_keys! [data["session_id"], data] end - # Marshal a session hash into safe cookie data. Include an integrity hash. - def marshal(session) - @verifier.generate(persistent_session_id!(session)) - end - - # Unmarshal cookie data to a hash and verify its integrity. - def unmarshal(cookie) - persistent_session_id!(@verifier.verify(cookie)) if cookie - rescue ActiveSupport::MessageVerifier::InvalidSignature - nil - end - - def ensure_session_key(key) - if key.blank? - raise ArgumentError, 'A key is required to write a ' + - 'cookie containing the session data. Use ' + - 'config.session_store :cookie_store, { :key => ' + - '"_myapp_session" } in config/application.rb' - end - end - - # To prevent users from using something insecure like "Password" we make sure that the - # secret they've provided is at least 30 characters in length. - def ensure_secret_secure(secret) - # There's no way we can do this check if they've provided a proc for the - # secret. - return true if secret.is_a?(Proc) - - if secret.blank? - raise ArgumentError, "A secret is required to generate an " + - "integrity hash for cookie session data. Use " + - "config.secret_token = \"some secret phrase of at " + - "least #{SECRET_MIN_LENGTH} characters\"" + - "in config/application.rb" - end - - if secret.length < SECRET_MIN_LENGTH - raise ArgumentError, "Secret should be something secure, " + - "like \"#{ActiveSupport::SecureRandom.hex(16)}\". The value you " + - "provided, \"#{secret}\", is shorter than the minimum length " + - "of #{SECRET_MIN_LENGTH} characters" - end - end - - def verifier_for(secret, digest) - key = secret.respond_to?(:call) ? secret.call : secret - ActiveSupport::MessageVerifier.new(key, digest) - end - - def generate_sid - ActiveSupport::SecureRandom.hex(16) - end - - def persistent_session_id!(data) - (data ||= {}).merge!(inject_persistent_session_id(data)) + def set_cookie(request, options) + request.cookie_jar.signed[@key] = options end - def inject_persistent_session_id(data) - requires_session_id?(data) ? { "session_id" => generate_sid } : {} + def set_session(env, sid, session_data) + persistent_session_id!(session_data, sid) end - def requires_session_id?(data) - if data - data.respond_to?(:key?) && !data.key?("session_id") - else - true - end + def persistent_session_id!(data, sid=nil) + data ||= {} + data["session_id"] ||= sid || generate_sid + data end end end diff --git a/actionpack/lib/action_dispatch/middleware/session/mem_cache_store.rb b/actionpack/lib/action_dispatch/middleware/session/mem_cache_store.rb index be1d5a43a2..8df8f977e8 100644 --- a/actionpack/lib/action_dispatch/middleware/session/mem_cache_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/mem_cache_store.rb @@ -38,9 +38,9 @@ module ActionDispatch options = env['rack.session.options'] expiry = options[:expire_after] || 0 @pool.set(sid, session_data, expiry) - return true + sid rescue MemCache::MemCacheError, Errno::ECONNREFUSED - return false + false end end end diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index 004c254e55..38da44d7e7 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -10,8 +10,6 @@ module ActionDispatch # Prepare dispatcher callbacks and run 'prepare' callbacks initializer "action_dispatch.prepare_dispatcher" do |app| - # TODO: This used to say unless defined?(Dispatcher). Find out why and fix. - require 'rails/dispatcher' ActionDispatch::Callbacks.to_prepare { app.routes_reloader.reload_if_changed } end end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 4b02c2deb3..8a8d21c434 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -693,6 +693,11 @@ module ActionDispatch super end + def root(options={}) + options[:on] ||= :collection if @scope[:scope_level] == :resources + super(options) + end + protected def parent_resource #:nodoc: @scope[:scope_level_resource] diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index 5555217ee2..9f56cca869 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -51,13 +51,17 @@ module ActionView autoload :MissingTemplate, 'action_view/template/error' autoload :ActionViewError, 'action_view/template/error' - autoload :TemplateError, 'action_view/template/error' + autoload :EncodingError, 'action_view/template/error' + autoload :TemplateError, 'action_view/template/error' + autoload :WrongEncodingError, 'action_view/template/error' autoload :TemplateHandler, 'action_view/template' autoload :TemplateHandlers, 'action_view/template' end autoload :TestCase, 'action_view/test_case' + + ENCODING_FLAG = '#.*coding[:=]\s*(\S+)[ \t]*' end require 'active_support/i18n' diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 4ac2ee52d6..735398d972 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -201,7 +201,7 @@ module ActionView #:nodoc: end def initialize(lookup_context = nil, assigns_for_first_render = {}, controller = nil, formats = nil) #:nodoc: - @config = nil + @config = nil @assigns = assigns_for_first_render.each { |key, value| instance_variable_set("@#{key}", value) } @helpers = self.class.helpers || Module.new @@ -212,6 +212,7 @@ module ActionView #:nodoc: @_config = ActiveSupport::InheritableOptions.new(controller.config) if controller && controller.respond_to?(:config) @_content_for = Hash.new { |h,k| h[k] = ActiveSupport::SafeBuffer.new } @_virtual_path = nil + @output_buffer = nil @lookup_context = lookup_context.is_a?(ActionView::LookupContext) ? lookup_context : ActionView::LookupContext.new(lookup_context) diff --git a/actionpack/lib/action_view/helpers/number_helper.rb b/actionpack/lib/action_view/helpers/number_helper.rb index 01fecc0f23..fccf004adf 100644 --- a/actionpack/lib/action_view/helpers/number_helper.rb +++ b/actionpack/lib/action_view/helpers/number_helper.rb @@ -13,6 +13,9 @@ module ActionView # unchanged if can't be converted into a valid number. module NumberHelper + DEFAULT_CURRENCY_VALUES = { :format => "%u%n", :unit => "$", :separator => ".", :delimiter => ",", + :precision => 2, :significant => false, :strip_insignificant_zeros => false } + # Raised when argument +number+ param given to the helpers is invalid and # the option :raise is set to +true+. class InvalidNumberError < StandardError @@ -104,9 +107,9 @@ module ActionView defaults = I18n.translate(:'number.format', :locale => options[:locale], :default => {}) currency = I18n.translate(:'number.currency.format', :locale => options[:locale], :default => {}) - defaults = defaults.merge(currency) - options = options.reverse_merge(defaults) + defaults = DEFAULT_CURRENCY_VALUES.merge(defaults).merge!(currency) + options = defaults.merge!(options) unit = options.delete(:unit) format = options.delete(:format) diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index a1a970e2d2..5d8ac6b115 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -1,12 +1,89 @@ -# encoding: utf-8 -# This is so that templates compiled in this file are UTF-8 require 'active_support/core_ext/array/wrap' require 'active_support/core_ext/object/blank' +require 'active_support/core_ext/kernel/singleton_class' module ActionView class Template extend ActiveSupport::Autoload + # === Encodings in ActionView::Template + # + # ActionView::Template is one of a few sources of potential + # encoding issues in Rails. This is because the source for + # templates are usually read from disk, and Ruby (like most + # encoding-aware programming languages) assumes that the + # String retrieved through File IO is encoded in the + # <tt>default_external</tt> encoding. In Rails, the default + # <tt>default_external</tt> encoding is UTF-8. + # + # As a result, if a user saves their template as ISO-8859-1 + # (for instance, using a non-Unicode-aware text editor), + # and uses characters outside of the ASCII range, their + # users will see diamonds with question marks in them in + # the browser. + # + # To mitigate this problem, we use a few strategies: + # 1. If the source is not valid UTF-8, we raise an exception + # when the template is compiled to alert the user + # to the problem. + # 2. The user can specify the encoding using Ruby-style + # encoding comments in any template engine. If such + # a comment is supplied, Rails will apply that encoding + # to the resulting compiled source returned by the + # template handler. + # 3. In all cases, we transcode the resulting String to + # the <tt>default_internal</tt> encoding (which defaults + # to UTF-8). + # + # This means that other parts of Rails can always assume + # that templates are encoded in UTF-8, even if the original + # source of the template was not UTF-8. + # + # From a user's perspective, the easiest thing to do is + # to save your templates as UTF-8. If you do this, you + # do not need to do anything else for things to "just work". + # + # === Instructions for template handlers + # + # The easiest thing for you to do is to simply ignore + # encodings. Rails will hand you the template source + # as the default_internal (generally UTF-8), raising + # an exception for the user before sending the template + # to you if it could not determine the original encoding. + # + # For the greatest simplicity, you can support only + # UTF-8 as the <tt>default_internal</tt>. This means + # that from the perspective of your handler, the + # entire pipeline is just UTF-8. + # + # === Advanced: Handlers with alternate metadata sources + # + # If you want to provide an alternate mechanism for + # specifying encodings (like ERB does via <%# encoding: ... %>), + # you may indicate that you are willing to accept + # BINARY data by implementing <tt>self.accepts_binary?</tt> + # on your handler. + # + # If you do, Rails will not raise an exception if + # the template's encoding could not be determined, + # assuming that you have another mechanism for + # making the determination. + # + # In this case, make sure you return a String from + # your handler encoded in the default_internal. Since + # you are handling out-of-band metadata, you are + # also responsible for alerting the user to any + # problems with converting the user's data to + # the default_internal. + # + # To do so, simply raise the raise WrongEncodingError + # as follows: + # + # raise WrongEncodingError.new( + # problematic_string, + # expected_encoding + # ) + eager_autoload do autoload :Error autoload :Handler @@ -16,20 +93,22 @@ module ActionView extend Template::Handlers - attr_reader :source, :identifier, :handler, :virtual_path, :formats + attr_reader :source, :identifier, :handler, :virtual_path, :formats, + :original_encoding - Finalizer = proc do |method_name| + Finalizer = proc do |method_name, mod| proc do - ActionView::CompiledTemplates.module_eval do + mod.module_eval do remove_possible_method method_name end end end def initialize(source, identifier, handler, details) - @source = source - @identifier = identifier - @handler = handler + @source = source + @identifier = identifier + @handler = handler + @original_encoding = nil @virtual_path = details[:virtual_path] @method_names = {} @@ -42,7 +121,13 @@ module ActionView # Notice that we use a bang in this instrumentation because you don't want to # consume this in production. This is only slow if it's being listened to. ActiveSupport::Notifications.instrument("!render_template.action_view", :virtual_path => @virtual_path) do - method_name = compile(locals, view) + if view.is_a?(ActionView::CompiledTemplates) + mod = ActionView::CompiledTemplates + else + mod = view.singleton_class + end + + method_name = compile(locals, view, mod) view.send(method_name, locals, &block) end rescue Exception => e @@ -50,7 +135,7 @@ module ActionView e.sub_template_of(self) raise e else - raise Template::Error.new(self, view.assigns, e) + raise Template::Error.new(self, view.respond_to?(:assigns) ? view.assigns : {}, e) end end @@ -75,37 +160,97 @@ module ActionView end private - def compile(locals, view) + # Among other things, this method is responsible for properly setting + # the encoding of the source. Until this point, we assume that the + # source is BINARY data. If no additional information is supplied, + # we assume the encoding is the same as Encoding.default_external. + # + # The user can also specify the encoding via a comment on the first + # line of the template (# encoding: NAME-OF-ENCODING). This will work + # with any template engine, as we process out the encoding comment + # before passing the source on to the template engine, leaving a + # blank line in its stead. + # + # Note that after we figure out the correct encoding, we then + # encode the source into Encoding.default_internal. In general, + # this means that templates will be UTF-8 inside of Rails, + # regardless of the original source encoding. + def compile(locals, view, mod) method_name = build_method_name(locals) return method_name if view.respond_to?(method_name) locals_code = locals.keys.map! { |key| "#{key} = local_assigns[:#{key}];" }.join - code = @handler.call(self) - if code.sub!(/\A(#.*coding.*)\n/, '') - encoding_comment = $1 - elsif defined?(Encoding) && Encoding.respond_to?(:default_external) - encoding_comment = "#coding:#{Encoding.default_external}" + if source.encoding_aware? + if source.sub!(/\A#{ENCODING_FLAG}/, '') + encoding = $1 + else + encoding = Encoding.default_external + end + + # Tag the source with the default external encoding + # or the encoding specified in the file + source.force_encoding(encoding) + + # If the original encoding is BINARY, the actual + # encoding is either stored out-of-band (such as + # in ERB <%# %> style magic comments) or missing. + # This is also true if the original encoding is + # something other than BINARY, but it's invalid. + if source.encoding != Encoding::BINARY && source.valid_encoding? + source.encode! + # If the assumed encoding is incorrect, check to + # see whether the handler accepts BINARY. If it + # does, it has another mechanism for determining + # the true encoding of the String. + elsif @handler.respond_to?(:accepts_binary?) && @handler.accepts_binary? + source.force_encoding(Encoding::BINARY) + # If the handler does not accept BINARY, the + # assumed encoding (either the default_external, + # or the explicit encoding specified by the user) + # is incorrect. We raise an exception here. + else + raise WrongEncodingError.new(source, encoding) + end + + # Don't validate the encoding yet -- the handler + # may treat the String as raw bytes and extract + # the encoding some other way end + code = @handler.call(self) + source = <<-end_src def #{method_name}(local_assigns) - _old_virtual_path, @_virtual_path = @_virtual_path, #{@virtual_path.inspect};_old_output_buffer = output_buffer;#{locals_code};#{code} + _old_virtual_path, @_virtual_path = @_virtual_path, #{@virtual_path.inspect};_old_output_buffer = @output_buffer;#{locals_code};#{code} ensure - @_virtual_path, self.output_buffer = _old_virtual_path, _old_output_buffer + @_virtual_path, @output_buffer = _old_virtual_path, _old_output_buffer end end_src - if encoding_comment - source = "#{encoding_comment}\n#{source}" - line = -1 - else - line = 0 + if source.encoding_aware? + # Handlers should return their source Strings in either the + # default_internal or BINARY. If the handler returns a BINARY + # String, we assume its encoding is the one we determined + # earlier, and encode the resulting source in the default_internal. + if source.encoding == Encoding::BINARY + source.force_encoding(Encoding.default_internal) + end + + # In case we get back a String from a handler that is not in + # BINARY or the default_internal, encode it to the default_internal + source.encode! + + # Now, validate that the source we got back from the template + # handler is valid in the default_internal + unless source.valid_encoding? + raise WrongEncodingError.new(@source, Encoding.default_internal) + end end begin - ActionView::CompiledTemplates.module_eval(source, identifier, line) - ObjectSpace.define_finalizer(self, Finalizer[method_name]) + mod.module_eval(source, identifier, 0) + ObjectSpace.define_finalizer(self, Finalizer[method_name, mod]) method_name rescue Exception => e # errors from template code diff --git a/actionpack/lib/action_view/template/error.rb b/actionpack/lib/action_view/template/error.rb index 6866eabf77..d3a53d2147 100644 --- a/actionpack/lib/action_view/template/error.rb +++ b/actionpack/lib/action_view/template/error.rb @@ -4,6 +4,24 @@ module ActionView class ActionViewError < StandardError #:nodoc: end + class EncodingError < StandardError #:nodoc: + end + + class WrongEncodingError < EncodingError #:nodoc: + def initialize(string, encoding) + @string, @encoding = string, encoding + end + + def message + "Your template was not saved as valid #{@encoding}. Please " \ + "either specify #{@encoding} as the encoding for your template " \ + "in your text editor, or mark the template with its " \ + "encoding by inserting the following as the first line " \ + "of the template:\n\n# encoding: <name of correct encoding>.\n\n" \ + "The source of your template was:\n\n#{@string}" + end + end + class MissingTemplate < ActionViewError #:nodoc: attr_reader :path diff --git a/actionpack/lib/action_view/template/handlers/erb.rb b/actionpack/lib/action_view/template/handlers/erb.rb index 237746437a..cbed0108cf 100644 --- a/actionpack/lib/action_view/template/handlers/erb.rb +++ b/actionpack/lib/action_view/template/handlers/erb.rb @@ -1,9 +1,15 @@ require 'active_support/core_ext/class/attribute_accessors' require 'active_support/core_ext/string/output_safety' +require "action_view/template" require 'erubis' module ActionView class OutputBuffer < ActiveSupport::SafeBuffer + def initialize(*) + super + encode! if encoding_aware? + end + def <<(value) super(value.to_s) end @@ -17,65 +23,108 @@ module ActionView end end - module Template::Handlers - class Erubis < ::Erubis::Eruby - def add_preamble(src) - src << "@output_buffer = ActionView::OutputBuffer.new;" - end + class Template + module Handlers + class Erubis < ::Erubis::Eruby + def add_preamble(src) + src << "@output_buffer = ActionView::OutputBuffer.new;" + end - def add_text(src, text) - return if text.empty? - src << "@output_buffer.safe_concat('" << escape_text(text) << "');" - end + def add_text(src, text) + return if text.empty? + src << "@output_buffer.safe_concat('" << escape_text(text) << "');" + end - BLOCK_EXPR = /\s+(do|\{)(\s*\|[^|]*\|)?\s*\Z/ + BLOCK_EXPR = /\s+(do|\{)(\s*\|[^|]*\|)?\s*\Z/ - def add_expr_literal(src, code) - if code =~ BLOCK_EXPR - src << '@output_buffer.append= ' << code - else - src << '@output_buffer.append= (' << code << ');' + def add_expr_literal(src, code) + if code =~ BLOCK_EXPR + src << '@output_buffer.append= ' << code + else + src << '@output_buffer.append= (' << code << ');' + end end - end - def add_stmt(src, code) - if code =~ BLOCK_EXPR - src << '@output_buffer.append_if_string= ' << code - else - super + def add_stmt(src, code) + if code =~ BLOCK_EXPR + src << '@output_buffer.append_if_string= ' << code + else + super + end end - end - def add_expr_escaped(src, code) - src << '@output_buffer.append= ' << escaped_expr(code) << ';' - end + def add_expr_escaped(src, code) + src << '@output_buffer.append= ' << escaped_expr(code) << ';' + end - def add_postamble(src) - src << '@output_buffer.to_s' + def add_postamble(src) + src << '@output_buffer.to_s' + end end - end - class ERB < Template::Handler - include Compilable + class ERB < Handler + include Compilable + + ## + # :singleton-method: + # Specify trim mode for the ERB compiler. Defaults to '-'. + # See ERb documentation for suitable values. + cattr_accessor :erb_trim_mode + self.erb_trim_mode = '-' + + self.default_format = Mime::HTML + + cattr_accessor :erb_implementation + self.erb_implementation = Erubis - ## - # :singleton-method: - # Specify trim mode for the ERB compiler. Defaults to '-'. - # See ERb documentation for suitable values. - cattr_accessor :erb_trim_mode - self.erb_trim_mode = '-' + ENCODING_TAG = Regexp.new("\\A(<%#{ENCODING_FLAG}-?%>)[ \\t]*") - self.default_format = Mime::HTML + def self.accepts_binary? + true + end + + def compile(template) + if template.source.encoding_aware? + # Even though Rails has given us a String tagged with the + # default_internal encoding (likely UTF-8), it is possible + # that the String is actually encoded using a different + # encoding, specified via an ERB magic comment. If the + # String is not actually UTF-8, the regular expression + # engine will (correctly) raise an exception. For now, + # we'll reset the String to BINARY so we can run regular + # expressions against it + template_source = template.source.dup.force_encoding("BINARY") + + # Erubis does not have direct support for encodings. + # As a result, we will extract the ERB-style magic + # comment, give the String to Erubis as BINARY data, + # and then tag the resulting String with the extracted + # encoding later + erb = template_source.gsub(ENCODING_TAG, '') + encoding = $2 - cattr_accessor :erb_implementation - self.erb_implementation = Erubis + if !encoding && (template.source.encoding == Encoding::BINARY) + raise WrongEncodingError.new(template_source, Encoding.default_external) + end + else + erb = template.source.dup + end - def compile(template) - source = template.source.gsub(/\A(<%(#.*coding[:=]\s*(\S+)\s*)-?%>)\s*\n?/, '') - erb = "<% __in_erb_template=true %>#{source}" - result = self.class.erb_implementation.new(erb, :trim=>(self.class.erb_trim_mode == "-")).src - result = "#{$2}\n#{result}" if $2 - result + result = self.class.erb_implementation.new( + erb, + :trim => (self.class.erb_trim_mode == "-") + ).src + + # If an encoding tag was found, tag the String + # we're returning with that encoding. Otherwise, + # return a BINARY String, which is what ERB + # returns. Note that if a magic comment was + # not specified, we will return the data to + # Rails as BINARY, which will then use its + # own encoding logic to create a UTF-8 String. + result = "\n#{result}".force_encoding(encoding).encode if encoding + result + end end end end diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index a223b3a55f..ef44925951 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -70,7 +70,10 @@ module ActionView Dir[query].reject { |p| File.directory?(p) }.map do |p| handler, format = extract_handler_and_format(p, formats) - Template.new(File.read(p), File.expand_path(p), handler, + + contents = File.open(p, "rb") {|io| io.read } + + Template.new(contents, File.expand_path(p), handler, :virtual_path => path, :format => format) end end diff --git a/actionpack/test/abstract/translation_test.rb b/actionpack/test/abstract/translation_test.rb index 0bf61a6556..09ebfab85e 100644 --- a/actionpack/test/abstract/translation_test.rb +++ b/actionpack/test/abstract/translation_test.rb @@ -9,18 +9,18 @@ class TranslationControllerTest < Test::Unit::TestCase end def test_action_controller_base_responds_to_translate - assert @controller.respond_to?(:translate) + assert_respond_to @controller, :translate end def test_action_controller_base_responds_to_t - assert @controller.respond_to?(:t) + assert_respond_to @controller, :t end def test_action_controller_base_responds_to_localize - assert @controller.respond_to?(:localize) + assert_respond_to @controller, :localize end def test_action_controller_base_responds_to_l - assert @controller.respond_to?(:l) + assert_respond_to @controller, :l end end
\ No newline at end of file diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 89ba0990f1..d2e5d2e965 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -12,6 +12,10 @@ $:.unshift(File.dirname(__FILE__) + '/fixtures/alternate_helpers') ENV['TMPDIR'] = File.join(File.dirname(__FILE__), 'tmp') +if defined?(Encoding.default_internal) + Encoding.default_internal = "UTF-8" +end + require 'test/unit' require 'abstract_controller' require 'action_controller' @@ -158,6 +162,7 @@ class ActionController::IntegrationTest < ActiveSupport::TestCase middleware.use "ActionDispatch::Cookies" middleware.use "ActionDispatch::Flash" middleware.use "ActionDispatch::Head" + yield(middleware) if block_given? end end diff --git a/actionpack/test/activerecord/active_record_store_test.rb b/actionpack/test/activerecord/active_record_store_test.rb index 61bee1b66c..6d4b8e1e40 100644 --- a/actionpack/test/activerecord/active_record_store_test.rb +++ b/actionpack/test/activerecord/active_record_store_test.rb @@ -152,12 +152,18 @@ class ActiveRecordStoreTest < ActionController::IntegrationTest end private + def with_test_route_set(options = {}) with_routing do |set| set.draw do |map| match ':action', :to => 'active_record_store_test/test' end - @app = ActiveRecord::SessionStore.new(set, options.reverse_merge(:key => '_session_id')) + + @app = self.class.build_app(set) do |middleware| + middleware.use ActiveRecord::SessionStore, options.reverse_merge(:key => '_session_id') + middleware.delete "ActionDispatch::ShowExceptions" + end + yield end end diff --git a/actionpack/test/controller/assert_select_test.rb b/actionpack/test/controller/assert_select_test.rb index 7012c4c9b0..4f8ad23174 100644 --- a/actionpack/test/controller/assert_select_test.rb +++ b/actionpack/test/controller/assert_select_test.rb @@ -212,12 +212,12 @@ class AssertSelectTest < ActionController::TestCase assert_nothing_raised { assert_select "div", "bar" } assert_nothing_raised { assert_select "div", /\w*/ } assert_nothing_raised { assert_select "div", :text => /\w*/, :count=>2 } - assert_raise(Assertion) { assert_select "div", :text=>"foo", :count=>2 } + assert_raise(Assertion) { assert_select "div", :text=>"foo", :count=>2 } assert_nothing_raised { assert_select "div", :html=>"<span>bar</span>" } assert_nothing_raised { assert_select "div", :html=>"<span>bar</span>" } assert_nothing_raised { assert_select "div", :html=>/\w*/ } assert_nothing_raised { assert_select "div", :html=>/\w*/, :count=>2 } - assert_raise(Assertion) { assert_select "div", :html=>"<span>foo</span>", :count=>2 } + assert_raise(Assertion) { assert_select "div", :html=>"<span>foo</span>", :count=>2 } end end diff --git a/actionpack/test/controller/capture_test.rb b/actionpack/test/controller/capture_test.rb index d1dbd535c4..47253f22b8 100644 --- a/actionpack/test/controller/capture_test.rb +++ b/actionpack/test/controller/capture_test.rb @@ -68,6 +68,6 @@ class CaptureTest < ActionController::TestCase private def expected_content_for_output - "<title>Putting stuff in the title!</title>\n\nGreat stuff!" + "<title>Putting stuff in the title!</title>\nGreat stuff!" end end diff --git a/actionpack/test/controller/cookie_test.rb b/actionpack/test/controller/cookie_test.rb index 4971866e7c..f65eda5c69 100644 --- a/actionpack/test/controller/cookie_test.rb +++ b/actionpack/test/controller/cookie_test.rb @@ -58,6 +58,17 @@ class CookieTest < ActionController::TestCase head :ok end + def raise_data_overflow + cookies.signed[:foo] = 'bye!' * 1024 + head :ok + end + + def tampered_cookies + cookies[:tampered] = "BAh7BjoIZm9vIghiYXI%3D--123456780" + cookies.signed[:tampered] + head :ok + end + def set_permanent_signed_cookie cookies.permanent.signed[:remember_me] = 100 head :ok @@ -74,7 +85,7 @@ class CookieTest < ActionController::TestCase def setup super - @request.env["action_dispatch.secret_token"] = "thisISverySECRET123" + @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" @request.host = "www.nextangle.com" end @@ -163,6 +174,48 @@ class CookieTest < ActionController::TestCase assert_equal({"user_name" => "david"}, @response.cookies) end + def test_raise_data_overflow + assert_raise(ActionDispatch::Cookies::CookieOverflow) do + get :raise_data_overflow + end + end + + def test_tampered_cookies + assert_nothing_raised do + get :tampered_cookies + assert_response :success + end + end + + def test_raises_argument_error_if_missing_secret + assert_raise(ArgumentError, nil.inspect) { + @request.env["action_dispatch.secret_token"] = nil + get :set_signed_cookie + } + + assert_raise(ArgumentError, ''.inspect) { + @request.env["action_dispatch.secret_token"] = "" + get :set_signed_cookie + } + end + + def test_raises_argument_error_if_secret_is_probably_insecure + assert_raise(ArgumentError, "password".inspect) { + @request.env["action_dispatch.secret_token"] = "password" + get :set_signed_cookie + } + + assert_raise(ArgumentError, "secret".inspect) { + @request.env["action_dispatch.secret_token"] = "secret" + get :set_signed_cookie + } + + assert_raise(ArgumentError, "12345678901234567890123456789".inspect) { + @request.env["action_dispatch.secret_token"] = "12345678901234567890123456789" + get :set_signed_cookie + } + end + private def assert_cookie_header(expected) header = @response.headers["Set-Cookie"] diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index ea740f7233..d5704eba78 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -515,7 +515,7 @@ class FilterTest < ActionController::TestCase assert assigns["ran_proc_filter2"] test_process(AnomolousYetValidConditionController, "show_without_filter") - assert_equal nil, assigns["ran_filter"] + assert_nil assigns["ran_filter"] assert !assigns["ran_class_filter"] assert !assigns["ran_proc_filter1"] assert !assigns["ran_proc_filter2"] @@ -530,16 +530,16 @@ class FilterTest < ActionController::TestCase test_process(ConditionalCollectionFilterController) assert_equal %w( ensure_login ), assigns["ran_filter"] test_process(ConditionalCollectionFilterController, "show_without_filter") - assert_equal nil, assigns["ran_filter"] + assert_nil assigns["ran_filter"] test_process(ConditionalCollectionFilterController, "another_action") - assert_equal nil, assigns["ran_filter"] + assert_nil assigns["ran_filter"] end def test_running_only_condition_filters test_process(OnlyConditionSymController) assert_equal %w( ensure_login ), assigns["ran_filter"] test_process(OnlyConditionSymController, "show_without_filter") - assert_equal nil, assigns["ran_filter"] + assert_nil assigns["ran_filter"] test_process(OnlyConditionProcController) assert assigns["ran_proc_filter"] @@ -556,7 +556,7 @@ class FilterTest < ActionController::TestCase test_process(ExceptConditionSymController) assert_equal %w( ensure_login ), assigns["ran_filter"] test_process(ExceptConditionSymController, "show_without_filter") - assert_equal nil, assigns["ran_filter"] + assert_nil assigns["ran_filter"] test_process(ExceptConditionProcController) assert assigns["ran_proc_filter"] @@ -573,7 +573,7 @@ class FilterTest < ActionController::TestCase test_process(BeforeAndAfterConditionController) assert_equal %w( ensure_login clean_up_tmp), assigns["ran_filter"] test_process(BeforeAndAfterConditionController, "show_without_filter") - assert_equal nil, assigns["ran_filter"] + assert_nil assigns["ran_filter"] end def test_around_filter @@ -674,7 +674,7 @@ class FilterTest < ActionController::TestCase def test_changing_the_requirements test_process(ChangingTheRequirementsController, "go_wild") - assert_equal nil, assigns['ran_filter'] + assert_nil assigns['ran_filter'] end def test_a_rescuing_around_filter diff --git a/actionpack/test/controller/flash_test.rb b/actionpack/test/controller/flash_test.rb index c662ce264b..01c8fd90a5 100644 --- a/actionpack/test/controller/flash_test.rb +++ b/actionpack/test/controller/flash_test.rb @@ -237,10 +237,19 @@ class FlashIntegrationTest < ActionController::IntegrationTest end private + + # Overwrite get to send SessionSecret in env hash + def get(path, parameters = nil, env = {}) + env["action_dispatch.secret_token"] ||= SessionSecret + super + end + def with_test_route_set with_routing do |set| set.draw do |map| - match ':action', :to => ActionDispatch::Session::CookieStore.new(FlashIntegrationTest::TestController, :key => FlashIntegrationTest::SessionKey, :secret => FlashIntegrationTest::SessionSecret) + match ':action', :to => ActionDispatch::Session::CookieStore.new( + FlashIntegrationTest::TestController, :key => SessionKey, :secret => SessionSecret + ) end yield end diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 20dc96d38d..5ee8e2b6ae 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -176,8 +176,8 @@ class IntegrationTestTest < Test::Unit::TestCase session1 = @test.open_session { |sess| } session2 = @test.open_session # implicit session - assert session1.respond_to?(:assert_template), "open_session makes assert_template available" - assert session2.respond_to?(:assert_template), "open_session makes assert_template available" + assert_respond_to session1, :assert_template, "open_session makes assert_template available" + assert_respond_to session2, :assert_template, "open_session makes assert_template available" assert !session1.equal?(session2) end diff --git a/actionpack/test/controller/new_base/bare_metal_test.rb b/actionpack/test/controller/new_base/bare_metal_test.rb index df4b39a103..8a06e8d2a0 100644 --- a/actionpack/test/controller/new_base/bare_metal_test.rb +++ b/actionpack/test/controller/new_base/bare_metal_test.rb @@ -18,10 +18,10 @@ module BareMetalTest string << part end - assert headers.is_a?(Hash), "Headers must be a Hash" + assert_kind_of Hash, headers, "Headers must be a Hash" assert headers["Content-Type"], "Content-Type must exist" assert_equal "Hello world", string end end -end
\ No newline at end of file +end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 52049f2a8a..2b1f2a27df 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -1079,7 +1079,7 @@ class RenderTest < ActionController::TestCase def test_action_talk_to_layout get :action_talk_to_layout - assert_equal "<title>Talking to the layout</title>\n\nAction was here!", @response.body + assert_equal "<title>Talking to the layout</title>\nAction was here!", @response.body end # :addressed: @@ -1096,7 +1096,7 @@ class RenderTest < ActionController::TestCase def test_yield_content_for assert_not_deprecated { get :yield_content_for } - assert_equal "<title>Putting stuff in the title!</title>\n\nGreat stuff!\n", @response.body + assert_equal "<title>Putting stuff in the title!</title>\nGreat stuff!\n", @response.body end def test_overwritting_rendering_relative_file_with_extension diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index 36b8055810..c7c8360ae6 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -55,8 +55,8 @@ class SendFileTest < ActionController::TestCase response = nil assert_nothing_raised { response = process('file') } assert_not_nil response - assert response.body_parts.respond_to?(:each) - assert response.body_parts.respond_to?(:to_path) + assert_respond_to response.body_parts, :each + assert_respond_to response.body_parts, :to_path require 'stringio' output = StringIO.new diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index c7f7f3102d..c20fa10f63 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -113,6 +113,10 @@ class ResponseTest < ActiveSupport::TestCase status, headers, body = @response.to_a assert_equal "user_name=david; path=/\nlogin=foo%26bar; path=/; expires=Mon, 10-Oct-2005 05:00:00 GMT", headers["Set-Cookie"] assert_equal({"login" => "foo&bar", "user_name" => "david"}, @response.cookies) + + @response.delete_cookie("login") + status, headers, body = @response.to_a + assert_equal({"user_name" => "david", "login" => nil}, @response.cookies) end test "read cache control" do diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 651a7a6be0..180889ddf2 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -186,6 +186,8 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end resources :products, :constraints => { :id => /\d{4}/ } do + root :to => "products#root" + get :favorite, :on => :collection resources :images end @@ -963,7 +965,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get '/products/1' assert_equal 'pass', @response.headers['X-Cascade'] get '/products' - assert_equal 'products#index', @response.body + assert_equal 'products#root', @response.body + get '/products/favorite' + assert_equal 'products#favorite', @response.body get '/products/0001' assert_equal 'products#show', @response.body @@ -981,6 +985,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + def test_root_works_in_the_resources_scope + get '/products' + assert_equal 'products#root', @response.body + assert_equal '/products', products_root_path + end + def test_module_scope with_test_routes do get '/token' diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index d2c1758af1..21d11ff31c 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -55,42 +55,13 @@ class CookieStoreTest < ActionController::IntegrationTest } end - def test_raises_argument_error_if_missing_secret - assert_raise(ArgumentError, nil.inspect) { - ActionDispatch::Session::CookieStore.new(nil, - :key => SessionKey, :secret => nil) - } - - assert_raise(ArgumentError, ''.inspect) { - ActionDispatch::Session::CookieStore.new(nil, - :key => SessionKey, :secret => '') - } - end - - def test_raises_argument_error_if_secret_is_probably_insecure - assert_raise(ArgumentError, "password".inspect) { - ActionDispatch::Session::CookieStore.new(nil, - :key => SessionKey, :secret => "password") - } - - assert_raise(ArgumentError, "secret".inspect) { - ActionDispatch::Session::CookieStore.new(nil, - :key => SessionKey, :secret => "secret") - } - - assert_raise(ArgumentError, "12345678901234567890123456789".inspect) { - ActionDispatch::Session::CookieStore.new(nil, - :key => SessionKey, :secret => "12345678901234567890123456789") - } - end - def test_setting_session_value with_test_route_set do get '/set_session_value' assert_response :success assert_equal "_myapp_session=#{response.body}; path=/; HttpOnly", headers['Set-Cookie'] - end + end end def test_getting_session_value @@ -99,7 +70,7 @@ class CookieStoreTest < ActionController::IntegrationTest get '/get_session_value' assert_response :success assert_equal 'foo: "bar"', response.body - end + end end def test_getting_session_id @@ -127,7 +98,7 @@ class CookieStoreTest < ActionController::IntegrationTest def test_close_raises_when_data_overflows with_test_route_set do - assert_raise(ActionDispatch::Session::CookieStore::CookieOverflow) { + assert_raise(ActionDispatch::Cookies::CookieOverflow) { get '/raise_data_overflow' } end @@ -209,30 +180,33 @@ class CookieStoreTest < ActionController::IntegrationTest get '/no_session_access' assert_response :success - # Mystery bug that came up in 2.3 as well. What is this trying to test?! - # assert_equal "_myapp_session=#{cookie_body}; path=/; expires=#{expected_expiry}; HttpOnly", - # headers['Set-Cookie'] + assert_equal "_myapp_session=#{cookie_body}; path=/; expires=#{expected_expiry}; HttpOnly", + headers['Set-Cookie'] end end private + + # Overwrite get to send SessionSecret in env hash + def get(path, parameters = nil, env = {}) + env["action_dispatch.secret_token"] ||= SessionSecret + super + end + def with_test_route_set(options = {}) with_routing do |set| set.draw do |map| match ':action', :to => ::CookieStoreTest::TestController end - options = {:key => SessionKey, :secret => SessionSecret}.merge(options) - @app = ActionDispatch::Session::CookieStore.new(set, options) + + options = { :key => SessionKey }.merge!(options) + + @app = self.class.build_app(set) do |middleware| + middleware.use ActionDispatch::Session::CookieStore, options + middleware.delete "ActionDispatch::ShowExceptions" + end + yield end end - - def unmarshal_session(cookie_string) - session = Rack::Utils.parse_query(cookie_string, ';,').inject({}) {|h,(k,v)| - h[k] = Array === v ? v.first : v - h - }[SessionKey] - verifier = ActiveSupport::MessageVerifier.new(SessionSecret, 'SHA1') - verifier.verify(session) - end end diff --git a/actionpack/test/dispatch/session/mem_cache_store_test.rb b/actionpack/test/dispatch/session/mem_cache_store_test.rb index 5a1dcb4dab..8858a398e0 100644 --- a/actionpack/test/dispatch/session/mem_cache_store_test.rb +++ b/actionpack/test/dispatch/session/mem_cache_store_test.rb @@ -114,7 +114,12 @@ class MemCacheStoreTest < ActionController::IntegrationTest set.draw do |map| match ':action', :to => ::MemCacheStoreTest::TestController end - @app = ActionDispatch::Session::MemCacheStore.new(set, :key => '_session_id') + + @app = self.class.build_app(set) do |middleware| + middleware.use ActionDispatch::Session::MemCacheStore, :key => '_session_id' + middleware.delete "ActionDispatch::ShowExceptions" + end + yield end end diff --git a/actionpack/test/dispatch/session/test_session_test.rb b/actionpack/test/dispatch/session/test_session_test.rb index c8dc4ab461..31ce97a25b 100644 --- a/actionpack/test/dispatch/session/test_session_test.rb +++ b/actionpack/test/dispatch/session/test_session_test.rb @@ -2,18 +2,6 @@ require 'abstract_unit' require 'stringio' class ActionController::TestSessionTest < ActiveSupport::TestCase - def test_calling_delete_without_parameters_raises_deprecation_warning_and_calls_to_clear_test_session - assert_deprecated(/use clear instead/){ ActionController::TestSession.new.delete } - end - - def test_calling_update_without_parameters_raises_deprecation_warning_and_calls_to_clear_test_session - assert_deprecated(/use replace instead/){ ActionController::TestSession.new.update } - end - - def test_calling_close_raises_deprecation_warning - assert_deprecated(/sessions should no longer be closed/){ ActionController::TestSession.new.close } - end - def test_ctor_allows_setting session = ActionController::TestSession.new({:one => 'one', :two => 'two'}) assert_equal('one', session[:one]) diff --git a/actionpack/test/fixtures/test/content_for.erb b/actionpack/test/fixtures/test/content_for.erb index 0e47ca8c3d..1fb829f54c 100644 --- a/actionpack/test/fixtures/test/content_for.erb +++ b/actionpack/test/fixtures/test/content_for.erb @@ -1,2 +1 @@ -<% content_for :title do %>Putting stuff in the title!<% end %> -Great stuff!
\ No newline at end of file +<% content_for :title do -%>Putting stuff in the title!<% end -%>Great stuff!
\ No newline at end of file diff --git a/actionpack/test/fixtures/test/content_for_concatenated.erb b/actionpack/test/fixtures/test/content_for_concatenated.erb index fb6b4b05d7..e65f629574 100644 --- a/actionpack/test/fixtures/test/content_for_concatenated.erb +++ b/actionpack/test/fixtures/test/content_for_concatenated.erb @@ -1,3 +1,3 @@ <% content_for :title, "Putting stuff " - content_for :title, "in the title!" %> + content_for :title, "in the title!" -%> Great stuff!
\ No newline at end of file diff --git a/actionpack/test/fixtures/test/content_for_with_parameter.erb b/actionpack/test/fixtures/test/content_for_with_parameter.erb index 57aecbac05..aeb6f73ce0 100644 --- a/actionpack/test/fixtures/test/content_for_with_parameter.erb +++ b/actionpack/test/fixtures/test/content_for_with_parameter.erb @@ -1,2 +1,2 @@ -<% content_for :title, "Putting stuff in the title!" %> +<% content_for :title, "Putting stuff in the title!" -%> Great stuff!
\ No newline at end of file diff --git a/actionpack/test/fixtures/test/non_erb_block_content_for.builder b/actionpack/test/fixtures/test/non_erb_block_content_for.builder index a94643561c..d539a425a4 100644 --- a/actionpack/test/fixtures/test/non_erb_block_content_for.builder +++ b/actionpack/test/fixtures/test/non_erb_block_content_for.builder @@ -1,4 +1,4 @@ content_for :title do 'Putting stuff in the title!' end -xml << "\nGreat stuff!" +xml << "Great stuff!" diff --git a/actionpack/test/template/number_helper_i18n_test.rb b/actionpack/test/template/number_helper_i18n_test.rb index f730a0d7f5..8561019461 100644 --- a/actionpack/test/template/number_helper_i18n_test.rb +++ b/actionpack/test/template/number_helper_i18n_test.rb @@ -45,6 +45,12 @@ class NumberHelperTest < ActionView::TestCase assert_equal("&$ - 10.00", number_to_currency(10, :locale => 'ts')) end + def test_number_to_currency_with_clean_i18n_settings + clean_i18n do + assert_equal("$10.00", number_to_currency(10)) + end + end + def test_number_with_precision #Delimiter was set to "" assert_equal("10000", number_with_precision(10000, :locale => 'ts')) @@ -92,4 +98,15 @@ class NumberHelperTest < ActionView::TestCase #Significant was set to true with precision 2, with custom translated units assert_equal "4.3 cm", number_to_human(0.0432, :locale => 'ts', :units => :custom_units_for_number_to_human) end + + private + def clean_i18n + load_path = I18n.load_path.dup + I18n.load_path.clear + I18n.reload! + yield + ensure + I18n.load_path = load_path + I18n.reload! + end end diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index d0212024ae..aca96e0a24 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -232,13 +232,13 @@ module RenderTestCases # TODO: Move to deprecated_tests.rb def test_render_with_nested_layout_deprecated assert_deprecated do - assert_equal %(<title>title</title>\n\n\n<div id="column">column</div>\n<div id="content">content</div>\n), + assert_equal %(<title>title</title>\n\n<div id="column">column</div>\n<div id="content">content</div>\n), @view.render(:file => "test/deprecated_nested_layout.erb", :layout => "layouts/yield") end end def test_render_with_nested_layout - assert_equal %(<title>title</title>\n\n\n<div id="column">column</div>\n<div id="content">content</div>\n), + assert_equal %(<title>title</title>\n\n<div id="column">column</div>\n<div id="content">content</div>\n), @view.render(:file => "test/nested_layout.erb", :layout => "layouts/yield") end @@ -284,7 +284,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase with_external_encoding Encoding::ASCII_8BIT do result = @view.render(:file => "test/utf8_magic.html.erb", :layouts => "layouts/yield") assert_equal Encoding::UTF_8, result.encoding - assert_equal "Русский текст\n\nUTF-8\nUTF-8\nUTF-8\n", result + assert_equal "\nРусский \nтекст\n\nUTF-8\nUTF-8\nUTF-8\n", result end end @@ -302,7 +302,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase result = @view.render(:file => "test/utf8.html.erb", :layouts => "layouts/yield") flunk 'Should have raised incompatible encoding error' rescue ActionView::Template::Error => error - assert_match 'invalid byte sequence in Shift_JIS', error.original_exception.message + assert_match 'Your template was not saved as valid Shift_JIS', error.original_exception.message end end end @@ -313,7 +313,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase result = @view.render(:file => "test/utf8_magic_with_bare_partial.html.erb", :layouts => "layouts/yield") flunk 'Should have raised incompatible encoding error' rescue ActionView::Template::Error => error - assert_match 'invalid byte sequence in Shift_JIS', error.original_exception.message + assert_match 'Your template was not saved as valid Shift_JIS', error.original_exception.message end end end diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb new file mode 100644 index 0000000000..995d728d50 --- /dev/null +++ b/actionpack/test/template/template_test.rb @@ -0,0 +1,130 @@ +require "abstract_unit" + +if "ruby".encoding_aware? + # These are the normal settings that will be set up by Railties + # TODO: Have these tests support other combinations of these values + Encoding.default_internal = "UTF-8" + Encoding.default_external = "UTF-8" +end + +class TestERBTemplate < ActiveSupport::TestCase + ERBHandler = ActionView::Template::Handlers::ERB + + class Context + def initialize + @output_buffer = "original" + end + + def hello + "Hello" + end + + def partial + ActionView::Template.new( + "<%= @_virtual_path %>", + "partial", + ERBHandler, + :virtual_path => "partial" + ) + end + + def logger + require "logger" + Logger.new(STDERR) + end + + def my_buffer + @output_buffer + end + end + + def new_template(body = "<%= hello %>", handler = ERBHandler, details = {}) + ActionView::Template.new(body, "hello template", ERBHandler, {:virtual_path => "hello"}) + end + + def render(locals = {}) + @template.render(@obj, locals) + end + + def setup + @obj = Context.new + end + + def test_basic_template + @template = new_template + assert_equal "Hello", render + end + + def test_locals + @template = new_template("<%= my_local %>") + assert_equal "I'm a local", render(:my_local => "I'm a local") + end + + def test_restores_buffer + @template = new_template + assert_equal "Hello", render + assert_equal "original", @obj.my_buffer + end + + def test_virtual_path + @template = new_template("<%= @_virtual_path %>" \ + "<%= partial.render(self, {}) %>" \ + "<%= @_virtual_path %>") + assert_equal "hellopartialhello", render + end + + if "ruby".encoding_aware? + def test_resulting_string_is_utf8 + @template = new_template + assert_equal Encoding::UTF_8, render.encoding + end + + def test_no_magic_comment_word_with_utf_8 + @template = new_template("hello \u{fc}mlat") + assert_equal Encoding::UTF_8, render.encoding + assert_equal "hello \u{fc}mlat", render + end + + # This test ensures that if the default_external + # is set to something other than UTF-8, we don't + # get any errors and get back a UTF-8 String. + def test_default_external_works + Encoding.default_external = "ISO-8859-1" + @template = new_template("hello \xFCmlat") + assert_equal Encoding::UTF_8, render.encoding + assert_equal "hello \u{fc}mlat", render + ensure + Encoding.default_external = "UTF-8" + end + + def test_encoding_can_be_specified_with_magic_comment + @template = new_template("# encoding: ISO-8859-1\nhello \xFCmlat") + assert_equal Encoding::UTF_8, render.encoding + assert_equal "\nhello \u{fc}mlat", render + end + + # TODO: This is currently handled inside ERB. The case of explicitly + # lying about encodings via the normal Rails API should be handled + # inside Rails. + def test_lying_with_magic_comment + assert_raises(ActionView::Template::Error) do + @template = new_template("# encoding: UTF-8\nhello \xFCmlat") + render + end + end + + def test_encoding_can_be_specified_with_magic_comment_in_erb + @template = new_template("<%# encoding: ISO-8859-1 %>hello \xFCmlat") + result = render + assert_equal Encoding::UTF_8, render.encoding + assert_equal "hello \u{fc}mlat", render + end + + def test_error_when_template_isnt_valid_utf8 + assert_raises(ActionView::Template::Error, /\xFC/) do + @template = new_template("hello \xFCmlat") + render + end + end + end +end
\ No newline at end of file |