diff options
Diffstat (limited to 'actionpack')
41 files changed, 702 insertions, 660 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 6a68c057ac..8eea4ccd41 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,21 @@ +* Using strings or symbols for middleware class names is deprecated. Convert + things like this: + + middleware.use "Foo::Bar" + + to this: + + middleware.use Foo::Bar + +* ActionController::TestSession now accepts a default value as well as + a block for generating a default value based off the key provided. + + This fixes calls to session#fetch in ApplicationController instances that + take more two arguments or a block from raising `ArgumentError: wrong + number of arguments (2 for 1)` when performing controller tests. + + *Matthew Gerrior* + * Fix `ActionController::Parameters#fetch` overwriting `KeyError` returned by default block. diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 17371a5392..55734b9774 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -252,7 +252,7 @@ module ActionController # Define some internal variables that should not be propagated to the view. PROTECTED_IVARS = AbstractController::Rendering::DEFAULT_PROTECTED_INSTANCE_VARIABLES + [ - :@_status, :@_headers, :@_params, :@_env, :@_response, :@_request, + :@_status, :@_headers, :@_params, :@_response, :@_request, :@_view_runtime, :@_stream, :@_url_options, :@_action_has_layout ] def _protected_ivars # :nodoc: diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index ae111e4951..914b0d4b30 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/array/extract_options' require 'action_dispatch/middleware/stack' +require 'active_support/deprecation' module ActionController # Extend ActionDispatch middleware stack to make it aware of options @@ -11,22 +12,14 @@ module ActionController # class MiddlewareStack < ActionDispatch::MiddlewareStack #:nodoc: class Middleware < ActionDispatch::MiddlewareStack::Middleware #:nodoc: - def initialize(klass, *args, &block) - options = args.extract_options! - @only = Array(options.delete(:only)).map(&:to_s) - @except = Array(options.delete(:except)).map(&:to_s) - args << options unless options.empty? - super + def initialize(klass, args, actions, strategy, block) + @actions = actions + @strategy = strategy + super(klass, args, block) end def valid?(action) - if @only.present? - @only.include?(action) - elsif @except.present? - !@except.include?(action) - else - true - end + @strategy.call @actions, action end end @@ -37,6 +30,32 @@ module ActionController middleware.valid?(action) ? middleware.build(a) : a end end + + private + + INCLUDE = ->(list, action) { list.include? action } + EXCLUDE = ->(list, action) { !list.include? action } + NULL = ->(list, action) { true } + + def build_middleware(klass, args, block) + options = args.extract_options! + only = Array(options.delete(:only)).map(&:to_s) + except = Array(options.delete(:except)).map(&:to_s) + args << options unless options.empty? + + strategy = NULL + list = nil + + if only.any? + strategy = INCLUDE + list = only + elsif except.any? + strategy = EXCLUDE + list = except + end + + Middleware.new(get_class(klass), args, list, strategy, block) + end end # <tt>ActionController::Metal</tt> is the simplest possible controller, providing a @@ -98,11 +117,10 @@ module ActionController class Metal < AbstractController::Base abstract! - attr_internal_writer :env - def env - @_env ||= {} + @_request.env end + deprecate :env # Returns the last part of the controller's name, underscored, without the ending # <tt>Controller</tt>. For instance, PostsController returns <tt>posts</tt>. @@ -197,8 +215,7 @@ module ActionController def set_request!(request) #:nodoc: @_request = request - @_env = request.env - @_env['action_controller.instance'] = self + @_request.controller_instance = self end def to_a #:nodoc: @@ -232,13 +249,13 @@ module ActionController end # Returns a Rack endpoint for the given action name. - def self.action(name, klass = ActionDispatch::Request) + def self.action(name) if middleware_stack.any? middleware_stack.build(name) do |env| - new.dispatch(name, klass.new(env)) + new.dispatch(name, ActionDispatch::Request.new(env)) end else - lambda { |env| new.dispatch(name, klass.new(env)) } + lambda { |env| new.dispatch(name, ActionDispatch::Request.new(env)) } end end end diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 9e09242872..d21a778d8d 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -139,7 +139,7 @@ module ActionController #:nodoc: request.session = NullSessionHash.new(request.env) request.env['action_dispatch.request.flash_hash'] = nil request.env['rack.session.options'] = { skip: true } - request.env['action_dispatch.cookies'] = NullCookieJar.build(request) + request.cookie_jar = NullCookieJar.build(request, {}) end protected @@ -160,14 +160,6 @@ module ActionController #:nodoc: end class NullCookieJar < ActionDispatch::Cookies::CookieJar #:nodoc: - def self.build(request) - key_generator = request.env[ActionDispatch::Cookies::GENERATOR_KEY] - host = request.host - secure = request.ssl? - - new(key_generator, host, secure, options_for_env({})) - end - def write(*) # nothing end diff --git a/actionpack/lib/action_controller/metal/streaming.rb b/actionpack/lib/action_controller/metal/streaming.rb index af31de1f3a..a6115674aa 100644 --- a/actionpack/lib/action_controller/metal/streaming.rb +++ b/actionpack/lib/action_controller/metal/streaming.rb @@ -199,7 +199,7 @@ module ActionController #:nodoc: def _process_options(options) #:nodoc: super if options[:stream] - if env["HTTP_VERSION"] == "HTTP/1.0" + if request.version == "HTTP/1.0" options.delete(:stream) else headers["Cache-Control"] ||= "no-cache" diff --git a/actionpack/lib/action_controller/middleware.rb b/actionpack/lib/action_controller/middleware.rb deleted file mode 100644 index 437fec3dc6..0000000000 --- a/actionpack/lib/action_controller/middleware.rb +++ /dev/null @@ -1,39 +0,0 @@ -module ActionController - class Middleware < Metal - class ActionMiddleware - def initialize(controller, app) - @controller, @app = controller, app - end - - def call(env) - request = ActionDispatch::Request.new(env) - @controller.build(@app).dispatch(:index, request) - end - end - - class << self - alias build new - - def new(app) - ActionMiddleware.new(self, app) - end - end - - attr_internal :app - - def process(action) - response = super - self.status, self.headers, self.response_body = response if response.is_a?(Array) - response - end - - def initialize(app) - super() - @_app = app - end - - def index - call(env) - end - end -end
\ No newline at end of file diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index f922e134f7..39069f7378 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -44,12 +44,12 @@ module ActionController def assign_parameters(routes, controller_path, action, parameters = {}) parameters = parameters.symbolize_keys - generated_path, extra_keys = routes.generate_extras(parameters.merge(:controller => controller_path, :action => action)) + generated_path, query_string_keys = routes.generate_extras(parameters.merge(:controller => controller_path, :action => action)) non_path_parameters = {} path_parameters = {} parameters.each do |key, value| - if extra_keys.include?(key) || key == :action || key == :controller + if query_string_keys.include?(key) || key == :action || key == :controller non_path_parameters[key] = value else if value.is_a?(Array) @@ -171,6 +171,10 @@ module ActionController clear end + def fetch(*args, &block) + @data.fetch(*args, &block) + end + private def load! diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 6985cec5f5..de28cd0998 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -34,12 +34,14 @@ module ActionDispatch HTTP_ACCEPT HTTP_ACCEPT_CHARSET HTTP_ACCEPT_ENCODING HTTP_ACCEPT_LANGUAGE HTTP_CACHE_CONTROL HTTP_FROM - HTTP_NEGOTIATE HTTP_PRAGMA ].freeze + HTTP_NEGOTIATE HTTP_PRAGMA HTTP_CLIENT_IP + HTTP_X_FORWARDED_FOR HTTP_VERSION + ].freeze ENV_METHODS.each do |env| class_eval <<-METHOD, __FILE__, __LINE__ + 1 def #{env.sub(/^HTTP_/n, '').downcase} # def accept_charset - @env["#{env}"] # @env["HTTP_ACCEPT_CHARSET"] + @env["#{env}".freeze] # @env["HTTP_ACCEPT_CHARSET".freeze] end # end METHOD end @@ -103,13 +105,17 @@ module ActionDispatch # the application should use), this \method returns the overridden # value, not the original. def request_method - @request_method ||= check_method(env["REQUEST_METHOD"]) + @request_method ||= check_method(super) end def routes # :nodoc: env["action_dispatch.routes".freeze] end + def routes=(routes) # :nodoc: + env["action_dispatch.routes".freeze] = routes + end + def original_script_name # :nodoc: env['ORIGINAL_SCRIPT_NAME'.freeze] end @@ -118,12 +124,31 @@ module ActionDispatch env[_routes.env_key] end + def engine_script_name=(name) # :nodoc: + env[routes.env_key] = name.dup + end + def request_method=(request_method) #:nodoc: if check_method(request_method) @request_method = env["REQUEST_METHOD"] = request_method end end + def controller_instance # :nodoc: + env['action_controller.instance'.freeze] + end + + def controller_instance=(controller) # :nodoc: + env['action_controller.instance'.freeze] = controller + end + + def show_exceptions? # :nodoc: + # We're treating `nil` as "unset", and we want the default setting to be + # `true`. This logic should be extracted to `env_config` and calculated + # once. + !(env['action_dispatch.show_exceptions'.freeze] == false) + end + # Returns a symbol form of the #request_method def request_method_symbol HTTP_METHOD_LOOKUP[request_method] @@ -210,6 +235,10 @@ module ActionDispatch @remote_ip ||= (@env["action_dispatch.remote_ip"] || ip).to_s end + def remote_ip=(remote_ip) + @env["action_dispatch.remote_ip".freeze] = remote_ip + end + ACTION_DISPATCH_REQUEST_ID = "action_dispatch.request_id".freeze # :nodoc: # Returns the unique request id, which is based on either the X-Request-Id header that can @@ -318,6 +347,14 @@ module ActionDispatch LOCALHOST =~ remote_addr && LOCALHOST =~ remote_ip end + def request_parameters=(params) + env["action_dispatch.request.request_parameters".freeze] = params + end + + def logger + env["action_dispatch.logger".freeze] + end + private def check_method(name) HTTP_METHOD_LOOKUP[name] || raise(ActionController::UnknownHttpMethod, "#{name}, accepted HTTP methods are #{HTTP_METHODS[0...-1].join(', ')}, and #{HTTP_METHODS[-1]}") diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb index 540e11a4a0..a221f4c5af 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -28,7 +28,13 @@ module ActionDispatch raise(ArgumentError, ':tempfile is required') unless @tempfile @original_filename = hash[:filename] - @original_filename &&= @original_filename.encode "UTF-8" + if @original_filename + begin + @original_filename.encode!(Encoding::UTF_8) + rescue EncodingError + @original_filename.force_encoding(Encoding::UTF_8) + end + end @content_type = hash[:type] @headers = hash[:head] end diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 07d97bd6bd..cf4f654ed6 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -8,8 +8,50 @@ require 'active_support/json' module ActionDispatch class Request < Rack::Request def cookie_jar - env['action_dispatch.cookies'] ||= Cookies::CookieJar.build(env, host, ssl?, cookies) + env['action_dispatch.cookies'.freeze] ||= Cookies::CookieJar.build(self, cookies) end + + # :stopdoc: + def have_cookie_jar? + env.key? 'action_dispatch.cookies'.freeze + end + + def cookie_jar=(jar) + env['action_dispatch.cookies'.freeze] = jar + end + + def key_generator + env[Cookies::GENERATOR_KEY] + end + + def signed_cookie_salt + env[Cookies::SIGNED_COOKIE_SALT] + end + + def encrypted_cookie_salt + env[Cookies::ENCRYPTED_COOKIE_SALT] + end + + def encrypted_signed_cookie_salt + env[Cookies::ENCRYPTED_SIGNED_COOKIE_SALT] + end + + def secret_token + env[Cookies::SECRET_TOKEN] + end + + def secret_key_base + env[Cookies::SECRET_KEY_BASE] + end + + def cookies_serializer + env[Cookies::COOKIES_SERIALIZER] + end + + def cookies_digest + env[Cookies::COOKIES_DIGEST] + end + # :startdoc: end # \Cookies are read and written through ActionController#cookies. @@ -118,7 +160,7 @@ module ActionDispatch # cookies.permanent.signed[:remember_me] = current_user.id # # => Set-Cookie: remember_me=BAhU--848956038e692d7046deab32b7131856ab20e14e; path=/; expires=Sun, 16-Dec-2029 03:24:16 GMT def permanent - @permanent ||= PermanentCookieJar.new(self, @key_generator, @options) + @permanent ||= PermanentCookieJar.new(self) end # Returns a jar that'll automatically generate a signed representation of cookie value and verify it when reading from @@ -138,10 +180,10 @@ module ActionDispatch # cookies.signed[:discount] # => 45 def signed @signed ||= - if @options[:upgrade_legacy_signed_cookies] - UpgradeLegacySignedCookieJar.new(self, @key_generator, @options) + if upgrade_legacy_signed_cookies? + UpgradeLegacySignedCookieJar.new(self) else - SignedCookieJar.new(self, @key_generator, @options) + SignedCookieJar.new(self) end end @@ -161,10 +203,10 @@ module ActionDispatch # cookies.encrypted[:discount] # => 45 def encrypted @encrypted ||= - if @options[:upgrade_legacy_signed_cookies] - UpgradeLegacyEncryptedCookieJar.new(self, @key_generator, @options) + if upgrade_legacy_signed_cookies? + UpgradeLegacyEncryptedCookieJar.new(self) else - EncryptedCookieJar.new(self, @key_generator, @options) + EncryptedCookieJar.new(self) end end @@ -172,12 +214,26 @@ module ActionDispatch # Used by ActionDispatch::Session::CookieStore to avoid the need to introduce new cookie stores. def signed_or_encrypted @signed_or_encrypted ||= - if @options[:secret_key_base].present? + if request.secret_key_base.present? encrypted else signed end end + + protected + + def request; @parent_jar.request; end + + private + + def upgrade_legacy_signed_cookies? + request.secret_token.present? && request.secret_key_base.present? + end + + def key_generator + request.key_generator + end end # Passing the ActiveSupport::MessageEncryptor::NullSerializer downstream @@ -187,7 +243,7 @@ module ActionDispatch module VerifyAndUpgradeLegacySignedMessage # :nodoc: def initialize(*args) super - @legacy_verifier = ActiveSupport::MessageVerifier.new(@options[:secret_token], serializer: ActiveSupport::MessageEncryptor::NullSerializer) + @legacy_verifier = ActiveSupport::MessageVerifier.new(request.secret_token, serializer: ActiveSupport::MessageEncryptor::NullSerializer) end def verify_and_upgrade_legacy_signed_message(name, signed_message) @@ -216,34 +272,18 @@ module ActionDispatch # $& => example.local DOMAIN_REGEXP = /[^.]*\.([^.]*|..\...|...\...)$/ - def self.options_for_env(env) #:nodoc: - { signed_cookie_salt: env[SIGNED_COOKIE_SALT] || '', - encrypted_cookie_salt: env[ENCRYPTED_COOKIE_SALT] || '', - encrypted_signed_cookie_salt: env[ENCRYPTED_SIGNED_COOKIE_SALT] || '', - secret_token: env[SECRET_TOKEN], - secret_key_base: env[SECRET_KEY_BASE], - upgrade_legacy_signed_cookies: env[SECRET_TOKEN].present? && env[SECRET_KEY_BASE].present?, - serializer: env[COOKIES_SERIALIZER], - digest: env[COOKIES_DIGEST] - } - end - - def self.build(env, host, secure, cookies) - key_generator = env[GENERATOR_KEY] - options = options_for_env env - - new(key_generator, host, secure, options).tap do |hash| + def self.build(req, cookies) + new(req).tap do |hash| hash.update(cookies) end end - def initialize(key_generator, host = nil, secure = false, options = {}) - @key_generator = key_generator + attr_reader :request + + def initialize(request) @set_cookies = {} @delete_cookies = {} - @host = host - @secure = secure - @options = options + @request = request @cookies = {} @committed = false end @@ -292,12 +332,12 @@ module ActionDispatch # if host is not ip and matches domain regexp # (ip confirms to domain regexp so we explicitly check for ip) - options[:domain] = if (@host !~ /^[\d.]+$/) && (@host =~ domain_regexp) + options[:domain] = if (request.host !~ /^[\d.]+$/) && (request.host =~ domain_regexp) ".#{$&}" end elsif options[:domain].is_a? Array # if host matches one of the supplied domains without a dot in front of it - options[:domain] = options[:domain].find {|domain| @host.include? domain.sub(/^\./, '') } + options[:domain] = options[:domain].find {|domain| request.host.include? domain.sub(/^\./, '') } end end @@ -356,27 +396,20 @@ module ActionDispatch @delete_cookies.each { |k, v| ::Rack::Utils.delete_cookie_header!(headers, k, v) } end - def recycle! #:nodoc: - @set_cookies = {} - @delete_cookies = {} - end - mattr_accessor :always_write_cookie self.always_write_cookie = false private def write_cookie?(cookie) - @secure || !cookie[:secure] || always_write_cookie + request.ssl? || !cookie[:secure] || always_write_cookie end end class PermanentCookieJar #:nodoc: include ChainedCookieJars - def initialize(parent_jar, key_generator, options = {}) + def initialize(parent_jar) @parent_jar = parent_jar - @key_generator = key_generator - @options = options end def [](name) @@ -410,7 +443,7 @@ module ActionDispatch protected def needs_migration?(value) - @options[:serializer] == :hybrid && value.start_with?(MARSHAL_SIGNATURE) + request.cookies_serializer == :hybrid && value.start_with?(MARSHAL_SIGNATURE) end def serialize(value) @@ -430,7 +463,7 @@ module ActionDispatch end def serializer - serializer = @options[:serializer] || :marshal + serializer = request.cookies_serializer || :marshal case serializer when :marshal Marshal @@ -442,7 +475,7 @@ module ActionDispatch end def digest - @options[:digest] || 'SHA1' + request.cookies_digest || 'SHA1' end end @@ -450,10 +483,9 @@ module ActionDispatch include ChainedCookieJars include SerializedCookieJars - def initialize(parent_jar, key_generator, options = {}) + def initialize(parent_jar) @parent_jar = parent_jar - @options = options - secret = key_generator.generate_key(@options[:signed_cookie_salt]) + secret = key_generator.generate_key(request.signed_cookie_salt) @verifier = ActiveSupport::MessageVerifier.new(secret, digest: digest, serializer: ActiveSupport::MessageEncryptor::NullSerializer) end @@ -505,16 +537,16 @@ module ActionDispatch include ChainedCookieJars include SerializedCookieJars - def initialize(parent_jar, key_generator, options = {}) + def initialize(parent_jar) + @parent_jar = parent_jar + if ActiveSupport::LegacyKeyGenerator === key_generator raise "You didn't set secrets.secret_key_base, which is required for this cookie jar. " + "Read the upgrade documentation to learn more about this new config option." end - @parent_jar = parent_jar - @options = options - secret = key_generator.generate_key(@options[:encrypted_cookie_salt]) - sign_secret = key_generator.generate_key(@options[:encrypted_signed_cookie_salt]) + secret = key_generator.generate_key(request.encrypted_cookie_salt || '') + sign_secret = key_generator.generate_key(request.encrypted_signed_cookie_salt || '') @encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, digest: digest, serializer: ActiveSupport::MessageEncryptor::NullSerializer) end @@ -568,9 +600,12 @@ module ActionDispatch end def call(env) + request = ActionDispatch::Request.new env + status, headers, body = @app.call(env) - if cookie_jar = env['action_dispatch.cookies'] + if request.have_cookie_jar? + cookie_jar = request.cookie_jar unless cookie_jar.committed? cookie_jar.write(headers) if headers[HTTP_HEADER].respond_to?(:join) diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 9082aac271..226a688fe2 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -44,6 +44,7 @@ module ActionDispatch end def call(env) + request = ActionDispatch::Request.new env _, headers, body = response = @app.call(env) if headers['X-Cascade'] == 'pass' @@ -53,14 +54,15 @@ module ActionDispatch response rescue Exception => exception - raise exception if env['action_dispatch.show_exceptions'] == false + raise exception unless request.show_exceptions? render_exception(env, exception) end private def render_exception(env, exception) - wrapper = ExceptionWrapper.new(env, exception) + backtrace_cleaner = env['action_dispatch.backtrace_cleaner'] + wrapper = ExceptionWrapper.new(backtrace_cleaner, exception) log_error(env, wrapper) if env['action_dispatch.show_detailed_exceptions'] diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index 8c3d45584d..039efc3af8 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -31,10 +31,10 @@ module ActionDispatch 'ActionView::Template::Error' => 'template_error' ) - attr_reader :env, :exception, :line_number, :file + attr_reader :backtrace_cleaner, :exception, :line_number, :file - def initialize(env, exception) - @env = env + def initialize(backtrace_cleaner, exception) + @backtrace_cleaner = backtrace_cleaner @exception = original_exception(exception) expand_backtrace if exception.is_a?(SyntaxError) || exception.try(:original_exception).try(:is_a?, SyntaxError) @@ -125,10 +125,6 @@ module ActionDispatch end end - def backtrace_cleaner - @backtrace_cleaner ||= @env['action_dispatch.backtrace_cleaner'] - end - def source_fragment(path, line) return unless Rails.respond_to?(:root) && Rails.root full_path = Rails.root.join(path) diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index 59639a010e..23da169b22 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -8,6 +8,14 @@ module ActionDispatch def flash @env[Flash::KEY] ||= Flash::FlashHash.from_session_value(session["flash"]) end + + def flash=(flash) + @env[Flash::KEY] = flash + end + + def flash_hash # :nodoc: + @env[Flash::KEY] + end end # The flash provides a way to pass temporary primitive-types (String, Array, Hash) between actions. Anything you place in the flash will be exposed @@ -263,14 +271,15 @@ module ActionDispatch end def call(env) + req = ActionDispatch::Request.new env @app.call(env) ensure session = Request::Session.find(env) || {} - flash_hash = env[KEY] + flash_hash = req.flash_hash if flash_hash && (flash_hash.present? || session.key?('flash')) session["flash"] = flash_hash.to_session_value - env[KEY] = flash_hash.dup + req.flash = flash_hash.dup end if (!session.respond_to?(:loaded?) || session.loaded?) && # (reset_session uses {}, which doesn't implement #loaded?) diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index 2617956c74..e65279a285 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -26,30 +26,29 @@ module ActionDispatch end def call(env) - default = env["action_dispatch.request.request_parameters"] - env["action_dispatch.request.request_parameters"] = parse_formatted_parameters(env, @parsers, default) + request = Request.new(env) + + request.request_parameters = parse_formatted_parameters(request, @parsers) @app.call(env) end private - def parse_formatted_parameters(env, parsers, default) - request = Request.new(env) - - return default if request.content_length.zero? + def parse_formatted_parameters(request, parsers) + return if request.content_length.zero? - strategy = parsers.fetch(request.content_mime_type) { return default } + strategy = parsers.fetch(request.content_mime_type) { return nil } strategy.call(request.raw_post) rescue => e # JSON or Ruby code block errors - logger(env).debug "Error occurred while parsing request parameters.\nContents:\n\n#{request.raw_post}" + logger(request).debug "Error occurred while parsing request parameters.\nContents:\n\n#{request.raw_post}" raise ParseError.new(e.message, e) end - def logger(env) - env['action_dispatch.logger'] || ActiveSupport::Logger.new($stderr) + def logger(request) + request.logger || ActiveSupport::Logger.new($stderr) end end end diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 9f894e2ec6..aee2334da9 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -74,16 +74,17 @@ module ActionDispatch # requests. For those requests that do need to know the IP, the # GetIp#calculate_ip method will calculate the memoized client IP address. def call(env) - env["action_dispatch.remote_ip"] = GetIp.new(env, check_ip, proxies) - @app.call(env) + req = ActionDispatch::Request.new env + req.remote_ip = GetIp.new(req, check_ip, proxies) + @app.call(req.env) end # The GetIp class exists as a way to defer processing of the request data # into an actual IP address. If the ActionDispatch::Request#remote_ip method # is called, this class will calculate the value and then memoize it. class GetIp - def initialize(env, check_ip, proxies) - @env = env + def initialize(req, check_ip, proxies) + @req = req @check_ip = check_ip @proxies = proxies end @@ -108,11 +109,11 @@ module ActionDispatch # the last address left, which was presumably set by one of those proxies. def calculate_ip # Set by the Rack web server, this is a single value. - remote_addr = ips_from('REMOTE_ADDR').last + remote_addr = ips_from(@req.remote_addr).last # Could be a CSV list and/or repeated headers that were concatenated. - client_ips = ips_from('HTTP_CLIENT_IP').reverse - forwarded_ips = ips_from('HTTP_X_FORWARDED_FOR').reverse + client_ips = ips_from(@req.client_ip).reverse + forwarded_ips = ips_from(@req.x_forwarded_for).reverse # +Client-Ip+ and +X-Forwarded-For+ should not, generally, both be set. # If they are both set, it means that this request passed through two @@ -123,8 +124,8 @@ module ActionDispatch if should_check_ip && !forwarded_ips.include?(client_ips.last) # We don't know which came from the proxy, and which from the user raise IpSpoofAttackError, "IP spoofing attack?! " + - "HTTP_CLIENT_IP=#{@env['HTTP_CLIENT_IP'].inspect} " + - "HTTP_X_FORWARDED_FOR=#{@env['HTTP_X_FORWARDED_FOR'].inspect}" + "HTTP_CLIENT_IP=#{@req.client_ip.inspect} " + + "HTTP_X_FORWARDED_FOR=#{@req.x_forwarded_for.inspect}" end # We assume these things about the IP headers: @@ -147,8 +148,9 @@ module ActionDispatch protected def ips_from(header) + return [] unless header # Split the comma-separated list into an array of strings - ips = @env[header] ? @env[header].strip.split(/[,\s]+/) : [] + ips = header.strip.split(/[,\s]+/) ips.select do |ip| begin # Only return IPs that are valid according to the IPAddr#new method diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index f0779279c1..12d8dab8eb 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -27,19 +27,21 @@ module ActionDispatch end def call(env) + request = ActionDispatch::Request.new env @app.call(env) rescue Exception => exception - if env['action_dispatch.show_exceptions'] == false - raise exception - else + if request.show_exceptions? render_exception(env, exception) + else + raise exception end end private def render_exception(env, exception) - wrapper = ExceptionWrapper.new(env, exception) + backtrace_cleaner = env['action_dispatch.backtrace_cleaner'] + wrapper = ExceptionWrapper.new(backtrace_cleaner, exception) status = wrapper.status_code env["action_dispatch.exception"] = wrapper.exception env["action_dispatch.original_path"] = env["PATH_INFO"] diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb index bbf734f103..0430ce3b9a 100644 --- a/actionpack/lib/action_dispatch/middleware/stack.rb +++ b/actionpack/lib/action_dispatch/middleware/stack.rb @@ -4,36 +4,15 @@ require "active_support/dependencies" module ActionDispatch class MiddlewareStack class Middleware - attr_reader :args, :block, :name, :classcache + attr_reader :args, :block, :klass - def initialize(klass_or_name, *args, &block) - @klass = nil - - if klass_or_name.respond_to?(:name) - @klass = klass_or_name - @name = @klass.name - else - @name = klass_or_name.to_s - end - - @classcache = ActiveSupport::Dependencies::Reference - @args, @block = args, block + def initialize(klass, args, block) + @klass = klass + @args = args + @block = block end - def klass - @klass || classcache[@name] - end - - def ==(middleware) - case middleware - when Middleware - klass == middleware.klass - when Class - klass == middleware - else - normalize(@name) == normalize(middleware) - end - end + def name; klass.name; end def inspect klass.to_s @@ -42,12 +21,6 @@ module ActionDispatch def build(app) klass.new(app, *args, &block) end - - private - - def normalize(object) - object.to_s.strip.sub(/^::/, '') - end end include Enumerable @@ -75,19 +48,17 @@ module ActionDispatch middlewares[i] end - def unshift(*args, &block) - middleware = self.class::Middleware.new(*args, &block) - middlewares.unshift(middleware) + def unshift(klass, *args, &block) + middlewares.unshift(build_middleware(klass, args, block)) end def initialize_copy(other) self.middlewares = other.middlewares.dup end - def insert(index, *args, &block) + def insert(index, klass, *args, &block) index = assert_index(index, :before) - middleware = self.class::Middleware.new(*args, &block) - middlewares.insert(index, middleware) + middlewares.insert(index, build_middleware(klass, args, block)) end alias_method :insert_before, :insert @@ -104,26 +75,48 @@ module ActionDispatch end def delete(target) - middlewares.delete target + target = get_class target + middlewares.delete_if { |m| m.klass == target } end - def use(*args, &block) - middleware = self.class::Middleware.new(*args, &block) - middlewares.push(middleware) + def use(klass, *args, &block) + middlewares.push(build_middleware(klass, args, block)) end - def build(app = nil, &block) - app ||= block - raise "MiddlewareStack#build requires an app" unless app + def build(app = Proc.new) middlewares.freeze.reverse.inject(app) { |a, e| e.build(a) } end protected def assert_index(index, where) - i = index.is_a?(Integer) ? index : middlewares.index(index) + index = get_class index + i = index.is_a?(Integer) ? index : middlewares.index { |m| m.klass == index } raise "No such middleware to insert #{where}: #{index.inspect}" unless i i end + + private + + def get_class(klass) + if klass.is_a?(String) || klass.is_a?(Symbol) + classcache = ActiveSupport::Dependencies::Reference + converted_klass = classcache[klass.to_s] + ActiveSupport::Deprecation.warn <<-eowarn +Passing strings or symbols to the middleware builder is deprecated, please change +them to actual class references. For example: + + "#{klass}" => #{converted_klass} + + eowarn + converted_klass + else + klass + end + end + + def build_middleware(klass, args, block) + Middleware.new(get_class(klass), args, block) + end end end diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index f38da4fdf6..9462ae4278 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -48,26 +48,30 @@ module ActionDispatch end def call(env) - path = env['PATH_INFO'] + serve ActionDispatch::Request.new env + end + + def serve(request) + path = request.path_info gzip_path = gzip_file_path(path) - if gzip_path && gzip_encoding_accepted?(env) - env['PATH_INFO'] = gzip_path - status, headers, body = @file_server.call(env) + if gzip_path && gzip_encoding_accepted?(request) + request.path_info = gzip_path + status, headers, body = @file_server.call(request.env) if status == 304 return [status, headers, body] end headers['Content-Encoding'] = 'gzip' headers['Content-Type'] = content_type(path) else - status, headers, body = @file_server.call(env) + status, headers, body = @file_server.call(request.env) end headers['Vary'] = 'Accept-Encoding' if gzip_path return [status, headers, body] ensure - env['PATH_INFO'] = path + request.path_info = path end private @@ -79,8 +83,8 @@ module ActionDispatch ::Rack::Mime.mime_type(::File.extname(path), 'text/plain'.freeze) end - def gzip_encoding_accepted?(env) - env['HTTP_ACCEPT_ENCODING'] =~ /\bgzip\b/i + def gzip_encoding_accepted?(request) + request.accept_encoding =~ /\bgzip\b/i end def gzip_file_path(path) @@ -110,16 +114,17 @@ module ActionDispatch end def call(env) - case env['REQUEST_METHOD'] - when 'GET', 'HEAD' - path = env['PATH_INFO'].chomp('/'.freeze) + req = ActionDispatch::Request.new env + + if req.get? || req.head? + path = req.path_info.chomp('/'.freeze) if match = @file_handler.match?(path) - env['PATH_INFO'] = match - return @file_handler.call(env) + req.path_info = match + return @file_handler.serve(req) end end - @app.call(env) + @app.call(req.env) end end end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 64352d5742..6a6af41320 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -16,7 +16,10 @@ module ActionDispatch class Constraints < Endpoint #:nodoc: attr_reader :app, :constraints - def initialize(app, constraints, dispatcher_p) + SERVE = ->(app, req) { app.serve req } + CALL = ->(app, req) { app.call req.env } + + def initialize(app, constraints, strategy) # Unwrap Constraints objects. I don't actually think it's possible # to pass a Constraints object to this constructor, but there were # multiple places that kept testing children of this object. I @@ -26,12 +29,12 @@ module ActionDispatch app = app.app end - @dispatcher = dispatcher_p + @strategy = strategy @app, @constraints, = app, constraints end - def dispatcher?; @dispatcher; end + def dispatcher?; @strategy == SERVE; end def matches?(req) @constraints.all? do |constraint| @@ -43,11 +46,7 @@ module ActionDispatch def serve(req) return [ 404, {'X-Cascade' => 'pass'}, [] ] unless matches?(req) - if dispatcher? - @app.serve req - else - @app.call req.env - end + @strategy.call @app, req end private @@ -62,7 +61,7 @@ module ActionDispatch attr_reader :requirements, :conditions, :defaults attr_reader :to, :default_controller, :default_action, :as, :anchor - def self.build(scope, set, path, as, options) + def self.build(scope, set, path, as, controller, default_action, to, options) options = scope[:options].merge(options) if scope[:options] options.delete :only @@ -71,19 +70,19 @@ module ActionDispatch options.delete :shallow_prefix options.delete :shallow - defaults = (scope[:defaults] || {}).merge options.delete(:defaults) || {} + defaults = (scope[:defaults] || {}).dup - new scope, set, path, defaults, as, options + new scope, set, path, defaults, as, controller, default_action, scope[:module], to, options end - def initialize(scope, set, path, defaults, as, options) + def initialize(scope, set, path, defaults, as, controller, default_action, modyoule, to, options) @requirements, @conditions = {}, {} @defaults = defaults @set = set - @to = options.delete :to - @default_controller = options.delete(:controller) || scope[:controller] - @default_action = options.delete(:action) || scope[:action] + @to = to + @default_controller = controller + @default_action = default_action @as = as @anchor = options.delete :anchor @@ -95,7 +94,7 @@ module ActionDispatch ast = path_ast path path_params = path_params ast - options = normalize_options!(options, formatted, path_params, ast, scope[:module]) + options = normalize_options!(options, formatted, path_params, ast, modyoule) split_constraints(path_params, scope[:constraints]) if scope[:constraints] @@ -241,11 +240,11 @@ module ActionDispatch def app(blocks) if to.respond_to?(:call) - Constraints.new(to, blocks, false) + Constraints.new(to, blocks, Constraints::CALL) elsif blocks.any? - Constraints.new(dispatcher(defaults), blocks, true) + Constraints.new(dispatcher(defaults.key?(:controller)), blocks, Constraints::SERVE) else - dispatcher(defaults) + dispatcher(defaults.key?(:controller)) end end @@ -335,8 +334,8 @@ module ActionDispatch parser.parse path end - def dispatcher(defaults) - @set.dispatcher defaults + def dispatcher(raise_on_name_error) + @set.dispatcher raise_on_name_error end end @@ -448,10 +447,10 @@ module ActionDispatch # resources :user, param: :name # # You can override <tt>ActiveRecord::Base#to_param</tt> of a related - # model to constructe an URL. + # model to construct an URL: # # class User < ActiveRecord::Base - # def to_param # overridden + # def to_param # name # end # end @@ -685,7 +684,11 @@ module ActionDispatch def map_method(method, args, &block) options = args.extract_options! options[:via] = method - match(*args, options, &block) + if options.key?(:defaults) + defaults(options.delete(:defaults)) { match(*args, options, &block) } + else + match(*args, options, &block) + end self end end @@ -788,8 +791,8 @@ module ActionDispatch end if options[:constraints].is_a?(Hash) - defaults = options[:constraints].select do - |k, v| URL_OPTIONS.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum)) + defaults = options[:constraints].select do |k, v| + URL_OPTIONS.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum)) end (options[:defaults] ||= {}).reverse_merge!(defaults) @@ -823,9 +826,11 @@ module ActionDispatch # controller "food" do # match "bacon", action: :bacon, via: :get # end - def controller(controller, options={}) - options[:controller] = controller - scope(options) { yield } + def controller(controller) + @scope = @scope.new(controller: controller) + yield + ensure + @scope = @scope.parent end # Scopes routes to a specific namespace. For example: @@ -871,13 +876,14 @@ module ActionDispatch defaults = { module: path, - path: options.fetch(:path, path), as: options.fetch(:as, path), shallow_path: options.fetch(:path, path), shallow_prefix: options.fetch(:as, path) } - scope(defaults.merge!(options)) { yield } + path_scope(options.delete(:path) { path }) do + scope(defaults.merge!(options)) { yield } + end end # === Parameter Restriction @@ -945,7 +951,10 @@ module ActionDispatch # end # Using this, the +:id+ parameter here will default to 'home'. def defaults(defaults = {}) - scope(:defaults => defaults) { yield } + @scope = @scope.new(defaults: merge_defaults_scope(@scope[:defaults], defaults)) + yield + ensure + @scope = @scope.parent end private @@ -1057,14 +1066,14 @@ module ActionDispatch class Resource #:nodoc: attr_reader :controller, :path, :options, :param - def initialize(entities, api_only = false, options = {}) + def initialize(entities, api_only, shallow, options = {}) @name = entities.to_s @path = (options[:path] || @name).to_s @controller = (options[:controller] || @name).to_s @as = options[:as] @param = (options[:param] || :id).to_sym @options = options - @shallow = false + @shallow = shallow @api_only = api_only end @@ -1107,7 +1116,7 @@ module ActionDispatch end def resource_scope - { :controller => controller } + controller end alias :collection_scope :path @@ -1130,17 +1139,15 @@ module ActionDispatch "#{path}/:#{nested_param}" end - def shallow=(value) - @shallow = value - end - def shallow? @shallow end + + def singleton?; false; end end class SingletonResource < Resource #:nodoc: - def initialize(entities, api_only, options) + def initialize(entities, api_only, shallow, options) super @as = nil @controller = (options[:controller] || plural).to_s @@ -1168,6 +1175,8 @@ module ActionDispatch alias :member_scope :path alias :nested_scope :path + + def singleton?; true; end end def resources_path_names(options) @@ -1202,20 +1211,22 @@ module ActionDispatch return self end - resource_scope(:resource, SingletonResource.new(resources.pop, api_only?, options)) do - yield if block_given? + with_scope_level(:resource) do + resource_scope(SingletonResource.new(resources.pop, api_only?, @scope[:shallow], options)) do + yield if block_given? - concerns(options[:concerns]) if options[:concerns] + concerns(options[:concerns]) if options[:concerns] - collection do - post :create - end if parent_resource.actions.include?(:create) + collection do + post :create + end if parent_resource.actions.include?(:create) - new do - get :new - end if parent_resource.actions.include?(:new) + new do + get :new + end if parent_resource.actions.include?(:new) - set_member_mappings_for_resource + set_member_mappings_for_resource + end end self @@ -1360,21 +1371,23 @@ module ActionDispatch return self end - resource_scope(:resources, Resource.new(resources.pop, api_only?, options)) do - yield if block_given? + with_scope_level(:resources) do + resource_scope(Resource.new(resources.pop, api_only?, @scope[:shallow], options)) do + yield if block_given? - concerns(options[:concerns]) if options[:concerns] + concerns(options[:concerns]) if options[:concerns] - collection do - get :index if parent_resource.actions.include?(:index) - post :create if parent_resource.actions.include?(:create) - end + collection do + get :index if parent_resource.actions.include?(:index) + post :create if parent_resource.actions.include?(:create) + end - new do - get :new - end if parent_resource.actions.include?(:new) + new do + get :new + end if parent_resource.actions.include?(:new) - set_member_mappings_for_resource + set_member_mappings_for_resource + end end self @@ -1398,7 +1411,7 @@ module ActionDispatch end with_scope_level(:collection) do - scope(parent_resource.collection_scope) do + path_scope(parent_resource.collection_scope) do yield end end @@ -1422,9 +1435,11 @@ module ActionDispatch with_scope_level(:member) do if shallow? - shallow_scope(parent_resource.member_scope) { yield } + shallow_scope { + path_scope(parent_resource.member_scope) { yield } + } else - scope(parent_resource.member_scope) { yield } + path_scope(parent_resource.member_scope) { yield } end end end @@ -1435,7 +1450,7 @@ module ActionDispatch end with_scope_level(:new) do - scope(parent_resource.new_scope(action_path(:new))) do + path_scope(parent_resource.new_scope(action_path(:new))) do yield end end @@ -1448,9 +1463,15 @@ module ActionDispatch with_scope_level(:nested) do if shallow? && shallow_nesting_depth >= 1 - shallow_scope(parent_resource.nested_scope, nested_options) { yield } + shallow_scope do + path_scope(parent_resource.nested_scope) do + scope(nested_options) { yield } + end + end else - scope(parent_resource.nested_scope, nested_options) { yield } + path_scope(parent_resource.nested_scope) do + scope(nested_options) { yield } + end end end end @@ -1465,13 +1486,14 @@ module ActionDispatch end def shallow - scope(:shallow => true) do - yield - end + @scope = @scope.new(shallow: true) + yield + ensure + @scope = @scope.parent end def shallow? - parent_resource.instance_of?(Resource) && @scope[:shallow] + !parent_resource.singleton? && @scope[:shallow] end # Matches a url pattern to one or more routes. @@ -1515,48 +1537,65 @@ module ActionDispatch options[:to] ||= "#{@scope[:controller]}##{@scope[:action]}" end - paths.each do |_path| - route_options = options.dup - route_options[:path] ||= _path if _path.is_a?(String) + controller = options.delete(:controller) || @scope[:controller] + option_path = options.delete :path + to = options.delete :to - path_without_format = _path.to_s.sub(/\(\.:format\)$/, '') - if using_match_shorthand?(path_without_format, route_options) - route_options[:to] ||= path_without_format.gsub(%r{^/}, "").sub(%r{/([^/]*)$}, '#\1') - route_options[:to].tr!("-", "_") - end + path_types = paths.group_by(&:class) + path_types.fetch(String, []).each do |_path| + route_options = options.dup + process_path(route_options, controller, _path, option_path || _path, to) + end - decomposed_match(_path, route_options) + path_types.fetch(Symbol, []).each do |action| + route_options = options.dup + decomposed_match(action, controller, route_options, option_path, to) end + self end - def using_match_shorthand?(path, options) - path && (options[:to] || options[:action]).nil? && path =~ %r{^/?[-\w]+/[-\w/]+$} + def process_path(options, controller, path, option_path, to) + path_without_format = path.sub(/\(\.:format\)$/, '') + if using_match_shorthand?(path_without_format, to, options[:action]) + to ||= path_without_format.gsub(%r{^/}, "").sub(%r{/([^/]*)$}, '#\1') + to.tr!("-", "_") + end + + decomposed_match(path, controller, options, option_path, to) + end + + def using_match_shorthand?(path, to, action) + return false if to || action + + path =~ %r{^/?[-\w]+/[-\w/]+$} end - def decomposed_match(path, options) # :nodoc: + def decomposed_match(path, controller, options, _path, to) # :nodoc: if on = options.delete(:on) - send(on) { decomposed_match(path, options) } + send(on) { decomposed_match(path, controller, options, _path, to) } else case @scope.scope_level when :resources - nested { decomposed_match(path, options) } + nested { decomposed_match(path, controller, options, _path, to) } when :resource - member { decomposed_match(path, options) } + member { decomposed_match(path, controller, options, _path, to) } else - add_route(path, options) + add_route(path, controller, options, _path, to) end end end - def add_route(action, options) # :nodoc: - path = path_for_action(action, options.delete(:path)) + def add_route(action, controller, options, _path, to) # :nodoc: + path = path_for_action(action, _path) raise ArgumentError, "path is required" if path.blank? action = action.to_s + default_action = options.delete(:action) || @scope[:action] + if action =~ /^[\w\-\/]+$/ - options[:action] ||= action.tr('-', '_') unless action.include?("/") + default_action ||= action.tr('-', '_') unless action.include?("/") else action = nil end @@ -1567,7 +1606,7 @@ module ActionDispatch name_for_action(options.delete(:as), action) end - mapping = Mapping.build(@scope, @set, URI.parser.escape(path), as, options) + mapping = Mapping.build(@scope, @set, URI.parser.escape(path), as, controller, default_action, to, options) app, conditions, requirements, defaults, as, anchor = mapping.to_route @set.add_route(app, conditions, requirements, defaults, as, anchor) end @@ -1583,7 +1622,7 @@ module ActionDispatch if @scope.resources? with_scope_level(:root) do - scope(parent_resource.path) do + path_scope(parent_resource.path) do super(options) end end @@ -1659,18 +1698,6 @@ module ActionDispatch @scope.nested? end - def with_exclusive_scope - begin - @scope = @scope.new(:as => nil, :path => nil) - - with_scope_level(:exclusive) do - yield - end - ensure - @scope = @scope.parent - end - end - def with_scope_level(kind) @scope = @scope.new_level(kind) yield @@ -1678,16 +1705,11 @@ module ActionDispatch @scope = @scope.parent end - def resource_scope(kind, resource) #:nodoc: - resource.shallow = @scope[:shallow] + def resource_scope(resource) #:nodoc: @scope = @scope.new(:scope_level_resource => resource) - @nesting.push(resource) - with_scope_level(kind) do - scope(parent_resource.resource_scope) { yield } - end + controller(resource.resource_scope) { yield } ensure - @nesting.pop @scope = @scope.parent end @@ -1700,12 +1722,10 @@ module ActionDispatch options end - def nesting_depth #:nodoc: - @nesting.size - end - def shallow_nesting_depth #:nodoc: - @nesting.count(&:shallow?) + @scope.find_all { |node| + node.frame[:scope_level_resource] + }.count { |node| node.frame[:scope_level_resource].shallow? } end def param_constraint? #:nodoc: @@ -1720,27 +1740,28 @@ module ActionDispatch resource_method_scope? && CANONICAL_ACTIONS.include?(action.to_s) end - def shallow_scope(path, options = {}) #:nodoc: + def shallow_scope #:nodoc: scope = { :as => @scope[:shallow_prefix], :path => @scope[:shallow_path] } @scope = @scope.new scope - scope(path, options) { yield } + yield ensure @scope = @scope.parent end def path_for_action(action, path) #:nodoc: - if path.blank? && canonical_action?(action) + return "#{@scope[:path]}/#{path}" if path + + if canonical_action?(action) @scope[:path].to_s else - "#{@scope[:path]}/#{action_path(action, path)}" + "#{@scope[:path]}/#{action_path(action)}" end end - def action_path(name, path = nil) #:nodoc: - name = name.to_sym if name.is_a?(String) - path || @scope[:path_names][name] || name.to_s + def action_path(name) #:nodoc: + @scope[:path_names][name.to_sym] || name end def prefix_name_for_action(as, action) #:nodoc: @@ -1796,6 +1817,14 @@ module ActionDispatch def api_only? @set.api_only? end + private + + def path_scope(path) + @scope = @scope.new(path: merge_path_scope(@scope[:path], path)) + yield + ensure + @scope = @scope.parent + end end # Routing Concerns allow you to declare common routes that can be reused @@ -1913,7 +1942,7 @@ module ActionDispatch attr_reader :parent, :scope_level - def initialize(hash, parent = {}, scope_level = nil) + def initialize(hash, parent = NULL, scope_level = nil) @hash = hash @parent = parent @scope_level = scope_level @@ -1961,27 +1990,34 @@ module ActionDispatch end def new_level(level) - self.class.new(self, self, level) - end - - def fetch(key, &block) - @hash.fetch(key, &block) + self.class.new(frame, self, level) end def [](key) - @hash.fetch(key) { @parent[key] } + scope = find { |node| node.frame.key? key } + scope && scope.frame[key] end - def []=(k,v) - @hash[k] = v + include Enumerable + + def each + node = self + loop do + break if node.equal? NULL + yield node + node = node.parent + end end + + def frame; @hash; end + + NULL = Scope.new(nil, nil) end def initialize(set) #:nodoc: @set = set @scope = Scope.new({ :path_names => @set.resources_path_names }) @concerns = {} - @nesting = [] end include Base diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb index 3c1c4fadf6..d6987f4d09 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -24,7 +24,7 @@ module ActionDispatch def serve(req) req.check_path_parameters! uri = URI.parse(path(req.path_parameters, req)) - + unless uri.host if relative_path?(uri.path) uri.path = "#{req.script_name}/#{uri.path}" @@ -32,7 +32,7 @@ module ActionDispatch uri.path = req.script_name.empty? ? "/" : req.script_name end end - + uri.scheme ||= req.scheme uri.host ||= req.host uri.port ||= req.port unless req.standard_port? @@ -124,7 +124,7 @@ module ActionDispatch url_options[:script_name] = request.script_name end end - + ActionDispatch::Http::URL.url_for url_options end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index c968381c26..2fe61c7aa6 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -21,8 +21,8 @@ module ActionDispatch alias inspect to_s class Dispatcher < Routing::Endpoint - def initialize(defaults) - @defaults = defaults + def initialize(raise_on_name_error) + @raise_on_name_error = raise_on_name_error @controller_class_names = ThreadSafe::Cache.new end @@ -34,12 +34,11 @@ module ActionDispatch prepare_params!(params) - # Just raise undefined constant errors if a controller was specified as default. - unless controller = controller(params, @defaults.key?(:controller)) + controller = controller(params, @raise_on_name_error) do return [404, {'X-Cascade' => 'pass'}, []] end - dispatch(controller, params[:action], req.env) + dispatch(controller, params[:action], req) end def prepare_params!(params) @@ -53,24 +52,26 @@ module ActionDispatch # segment, as in :controller(/:action), we should simply return nil and # delegate the control back to Rack cascade. Besides, if this is not a default # controller, it means we should respect the @scope[:module] parameter. - def controller(params, default_controller=true) - if params && params.key?(:controller) - controller_param = params[:controller] - controller_reference(controller_param) - end + def controller(params, raise_on_name_error=true) + controller_reference params.fetch(:controller) { yield } rescue NameError => e - raise ActionController::RoutingError, e.message, e.backtrace if default_controller + raise ActionController::RoutingError, e.message, e.backtrace if raise_on_name_error + yield end - private + protected + + attr_reader :controller_class_names def controller_reference(controller_param) - const_name = @controller_class_names[controller_param] ||= "#{controller_param.camelize}Controller" + const_name = controller_class_names[controller_param] ||= "#{controller_param.camelize}Controller" ActiveSupport::Dependencies.constantize(const_name) end - def dispatch(controller, action, env) - controller.action(action).call(env) + private + + def dispatch(controller, action, req) + controller.action(action).call(req.env) end def normalize_controller!(params) @@ -267,7 +268,7 @@ module ActionDispatch path_params -= controller_options.keys path_params -= result.keys end - inner_options.each do |key, _| + inner_options.each_key do |key| path_params.delete(key) end @@ -313,7 +314,7 @@ module ActionDispatch attr_accessor :formatter, :set, :named_routes, :default_scope, :router attr_accessor :disable_clear_and_finalize, :resources_path_names - attr_accessor :default_url_options + attr_accessor :default_url_options, :dispatcher_class attr_reader :env_key alias :routes :set @@ -356,6 +357,7 @@ module ActionDispatch @set = Journey::Routes.new @router = Journey::Router.new @set @formatter = Journey::Formatter.new @set + @dispatcher_class = Routing::RouteSet::Dispatcher end def relative_url_root @@ -413,8 +415,8 @@ module ActionDispatch @prepend.each { |blk| eval_block(blk) } end - def dispatcher(defaults) - Routing::RouteSet::Dispatcher.new(defaults) + def dispatcher(raise_on_name_error) + dispatcher_class.new(raise_on_name_error) end module MountedHelpers @@ -587,10 +589,8 @@ module ActionDispatch PARAMETERIZE = lambda do |name, value| if name == :controller value - elsif value.is_a?(Array) - value.map(&:to_param).join('/') - elsif param = value.to_param - param + else + value.to_param end end @@ -815,12 +815,12 @@ module ActionDispatch if app.matches?(req) && app.dispatcher? dispatcher = app.app - if dispatcher.controller(params, false) - dispatcher.prepare_params!(params) - return params - else + dispatcher.controller(params, false) do raise ActionController::RoutingError, "A route matches #{path.inspect}, but references missing controller: #{params[:controller].camelize}Controller" end + + dispatcher.prepare_params!(params) + return params end end diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index d0e3ea818e..54e24ed6bf 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -86,8 +86,8 @@ module ActionDispatch end # Load routes.rb if it hasn't been loaded. - generated_path, extra_keys = @routes.generate_extras(options, defaults) - found_extras = options.reject { |k, _| ! extra_keys.include? k } + generated_path, query_string_keys = @routes.generate_extras(options, defaults) + found_extras = options.reject { |k, _| ! query_string_keys.include? k } msg = message || sprintf("found extras <%s>, not <%s>", found_extras, extras) assert_equal(extras, found_extras, msg) diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 0298962409..0cdc6d4e77 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -325,7 +325,11 @@ module ActionDispatch if path =~ %r{://} location = URI.parse(path) https! URI::HTTPS === location if location.scheme - host! "#{location.host}:#{location.port}" if location.host + if url_host = location.host + default = Rack::Request::DEFAULT_PORTS[location.scheme] + url_host += ":#{location.port}" if default != location.port + host! url_host + end path = location.query ? "#{location.path}?#{location.query}" : location.path end @@ -374,7 +378,7 @@ module ActionDispatch @html_document = nil @url_options = nil - @controller = session.last_request.env['action_controller.instance'] + @controller = @request.controller_instance response.status end diff --git a/actionpack/lib/action_dispatch/testing/test_process.rb b/actionpack/lib/action_dispatch/testing/test_process.rb index 494644cd46..c28d701b48 100644 --- a/actionpack/lib/action_dispatch/testing/test_process.rb +++ b/actionpack/lib/action_dispatch/testing/test_process.rb @@ -19,7 +19,7 @@ module ActionDispatch end def cookies - @cookie_jar ||= Cookies::CookieJar.build(@request.env, @request.host, @request.ssl?, @request.cookies) + @cookie_jar ||= Cookies::CookieJar.build(@request, @request.cookies) end def redirect_to_url diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index cc610b6d75..60e2cea8a2 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -63,6 +63,10 @@ FIXTURE_LOAD_PATH = File.join(File.dirname(__FILE__), 'fixtures') SharedTestRoutes = ActionDispatch::Routing::RouteSet.new +SharedTestRoutes.draw do + get ':controller(/:action)' +end + module ActionDispatch module SharedRoutes def before_setup @@ -70,35 +74,10 @@ module ActionDispatch super end end - - # Hold off drawing routes until all the possible controller classes - # have been loaded. - module DrawOnce - class << self - attr_accessor :drew - end - self.drew = false - - def before_setup - super - return if DrawOnce.drew - - SharedTestRoutes.draw do - get ':controller(/:action)' - end - - ActionDispatch::IntegrationTest.app.routes.draw do - get ':controller(/:action)' - end - - DrawOnce.drew = true - end - end end module ActiveSupport class TestCase - include ActionDispatch::DrawOnce if RUBY_ENGINE == "ruby" && PROCESS_COUNT > 0 parallelize_me! end @@ -119,23 +98,25 @@ class RoutedRackApp end class ActionDispatch::IntegrationTest < ActiveSupport::TestCase - include ActionDispatch::SharedRoutes - def self.build_app(routes = nil) RoutedRackApp.new(routes || ActionDispatch::Routing::RouteSet.new) do |middleware| - middleware.use "ActionDispatch::ShowExceptions", ActionDispatch::PublicExceptions.new("#{FIXTURE_LOAD_PATH}/public") - middleware.use "ActionDispatch::DebugExceptions" - middleware.use "ActionDispatch::Callbacks" - middleware.use "ActionDispatch::ParamsParser" - middleware.use "ActionDispatch::Cookies" - middleware.use "ActionDispatch::Flash" - middleware.use "Rack::Head" + middleware.use ActionDispatch::ShowExceptions, ActionDispatch::PublicExceptions.new("#{FIXTURE_LOAD_PATH}/public") + middleware.use ActionDispatch::DebugExceptions + middleware.use ActionDispatch::Callbacks + middleware.use ActionDispatch::ParamsParser + middleware.use ActionDispatch::Cookies + middleware.use ActionDispatch::Flash + middleware.use Rack::Head yield(middleware) if block_given? end end self.app = build_app + app.routes.draw do + get ':controller(/:action)' + end + # Stub Rails dispatcher so it does not get controller references and # simply return the controller#action as Rack::Body. class StubDispatcher < ::ActionDispatch::Routing::RouteSet::Dispatcher @@ -149,14 +130,10 @@ class ActionDispatch::IntegrationTest < ActiveSupport::TestCase end end - def self.stub_controllers - old_dispatcher = ActionDispatch::Routing::RouteSet::Dispatcher - ActionDispatch::Routing::RouteSet.module_eval { remove_const :Dispatcher } - ActionDispatch::Routing::RouteSet.module_eval { const_set :Dispatcher, StubDispatcher } - yield ActionDispatch::Routing::RouteSet.new - ensure - ActionDispatch::Routing::RouteSet.module_eval { remove_const :Dispatcher } - ActionDispatch::Routing::RouteSet.module_eval { const_set :Dispatcher, old_dispatcher } + def self.stub_controllers(config = nil) + route_set = ActionDispatch::Routing::RouteSet.new(*[config].compact) + route_set.dispatcher_class = StubDispatcher + yield route_set end def with_routing(&block) diff --git a/actionpack/test/controller/flash_test.rb b/actionpack/test/controller/flash_test.rb index 64543f0659..b063d769a4 100644 --- a/actionpack/test/controller/flash_test.rb +++ b/actionpack/test/controller/flash_test.rb @@ -329,7 +329,7 @@ class FlashIntegrationTest < ActionDispatch::IntegrationTest @app = self.class.build_app(set) do |middleware| middleware.use ActionDispatch::Session::CookieStore, :key => SessionKey middleware.use ActionDispatch::Flash - middleware.delete "ActionDispatch::ShowExceptions" + middleware.delete ActionDispatch::ShowExceptions end yield diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 0b9be8d671..dc4c32b07e 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -755,6 +755,25 @@ class MetalIntegrationTest < ActionDispatch::IntegrationTest assert_equal "http://test.com/", @request.env["HTTP_REFERER"] end + def test_ignores_common_ports_in_host + get "http://test.com" + assert_equal "test.com", @request.env["HTTP_HOST"] + + get "https://test.com" + assert_equal "test.com", @request.env["HTTP_HOST"] + end + + def test_keeps_uncommon_ports_in_host + get "http://test.com:123" + assert_equal "test.com:123", @request.env["HTTP_HOST"] + + get "http://test.com:443" + assert_equal "test.com:443", @request.env["HTTP_HOST"] + + get "https://test.com:80" + assert_equal "test.com:80", @request.env["HTTP_HOST"] + end + end class ApplicationIntegrationTest < ActionDispatch::IntegrationTest diff --git a/actionpack/test/controller/new_base/metal_test.rb b/actionpack/test/controller/new_base/metal_test.rb deleted file mode 100644 index 537b93387a..0000000000 --- a/actionpack/test/controller/new_base/metal_test.rb +++ /dev/null @@ -1,43 +0,0 @@ -require 'abstract_unit' - -module MetalTest - class MetalMiddleware < ActionController::Middleware - def call(env) - if env["PATH_INFO"] =~ /authed/ - app.call(env) - else - [401, headers, "Not authed!"] - end - end - end - - class Endpoint - def call(env) - [200, {}, "Hello World"] - end - end - - class TestMiddleware < ActiveSupport::TestCase - def setup - @app = Rack::Builder.new do - use MetalTest::MetalMiddleware - run MetalTest::Endpoint.new - end.to_app - end - - test "it can call the next app by using @app" do - env = Rack::MockRequest.env_for("/authed") - response = @app.call(env) - - assert_equal ["Hello World"], response[2] - end - - test "it can return a response using the normal AC::Metal techniques" do - env = Rack::MockRequest.env_for("/") - response = @app.call(env) - - assert_equal ["Not authed!"], response[2] - assert_equal 401, response[0] - end - end -end diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index 5a279639cc..04d6cc1792 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -1128,14 +1128,14 @@ class ResourcesTest < ActionController::TestCase end def assert_restful_routes_for(controller_name, options = {}) - options[:options] ||= {} - options[:options][:controller] = options[:controller] || controller_name.to_s + route_options = (options[:options] ||= {}).dup + route_options[:controller] = options[:controller] || controller_name.to_s if options[:shallow] options[:shallow_options] ||= {} - options[:shallow_options][:controller] = options[:options][:controller] + options[:shallow_options][:controller] = route_options[:controller] else - options[:shallow_options] = options[:options] + options[:shallow_options] = route_options end new_action = @routes.resources_path_names[:new] || "new" @@ -1154,7 +1154,7 @@ class ResourcesTest < ActionController::TestCase edit_member_path = "#{member_path}/#{edit_action}" formatted_edit_member_path = "#{member_path}/#{edit_action}.xml" - with_options(options[:options]) do |controller| + with_options(route_options) do |controller| controller.assert_routing collection_path, :action => 'index' controller.assert_routing new_path, :action => 'new' controller.assert_routing "#{collection_path}.xml", :action => 'index', :format => 'xml' @@ -1168,23 +1168,23 @@ class ResourcesTest < ActionController::TestCase controller.assert_routing formatted_edit_member_path, :action => 'edit', :id => '1', :format => 'xml' end - assert_recognizes(options[:options].merge(:action => 'index'), :path => collection_path, :method => :get) - assert_recognizes(options[:options].merge(:action => 'new'), :path => new_path, :method => :get) - assert_recognizes(options[:options].merge(:action => 'create'), :path => collection_path, :method => :post) + assert_recognizes(route_options.merge(:action => 'index'), :path => collection_path, :method => :get) + assert_recognizes(route_options.merge(:action => 'new'), :path => new_path, :method => :get) + assert_recognizes(route_options.merge(:action => 'create'), :path => collection_path, :method => :post) assert_recognizes(options[:shallow_options].merge(:action => 'show', :id => '1'), :path => member_path, :method => :get) assert_recognizes(options[:shallow_options].merge(:action => 'edit', :id => '1'), :path => edit_member_path, :method => :get) assert_recognizes(options[:shallow_options].merge(:action => 'update', :id => '1'), :path => member_path, :method => :put) assert_recognizes(options[:shallow_options].merge(:action => 'destroy', :id => '1'), :path => member_path, :method => :delete) - assert_recognizes(options[:options].merge(:action => 'index', :format => 'xml'), :path => "#{collection_path}.xml", :method => :get) - assert_recognizes(options[:options].merge(:action => 'new', :format => 'xml'), :path => "#{new_path}.xml", :method => :get) - assert_recognizes(options[:options].merge(:action => 'create', :format => 'xml'), :path => "#{collection_path}.xml", :method => :post) + assert_recognizes(route_options.merge(:action => 'index', :format => 'xml'), :path => "#{collection_path}.xml", :method => :get) + assert_recognizes(route_options.merge(:action => 'new', :format => 'xml'), :path => "#{new_path}.xml", :method => :get) + assert_recognizes(route_options.merge(:action => 'create', :format => 'xml'), :path => "#{collection_path}.xml", :method => :post) assert_recognizes(options[:shallow_options].merge(:action => 'show', :id => '1', :format => 'xml'), :path => "#{member_path}.xml", :method => :get) assert_recognizes(options[:shallow_options].merge(:action => 'edit', :id => '1', :format => 'xml'), :path => formatted_edit_member_path, :method => :get) assert_recognizes(options[:shallow_options].merge(:action => 'update', :id => '1', :format => 'xml'), :path => "#{member_path}.xml", :method => :put) assert_recognizes(options[:shallow_options].merge(:action => 'destroy', :id => '1', :format => 'xml'), :path => "#{member_path}.xml", :method => :delete) - yield options[:options] if block_given? + yield route_options if block_given? end # test named routes like foo_path and foos_path map to the correct options. @@ -1195,20 +1195,20 @@ class ResourcesTest < ActionController::TestCase end singular_name ||= controller_name.to_s.singularize - options[:options] ||= {} - options[:options][:controller] = options[:controller] || controller_name.to_s + route_options = (options[:options] ||= {}).dup + route_options[:controller] = options[:controller] || controller_name.to_s if options[:shallow] options[:shallow_options] ||= {} - options[:shallow_options][:controller] = options[:options][:controller] + options[:shallow_options][:controller] = route_options[:controller] else - options[:shallow_options] = options[:options] + options[:shallow_options] = route_options end - @controller = "#{options[:options][:controller].camelize}Controller".constantize.new + @controller = "#{route_options[:controller].camelize}Controller".constantize.new @controller.singleton_class.include(@routes.url_helpers) - get :index, params: options[:options] - options[:options].delete :action + get :index, params: route_options + route_options.delete :action path = "#{options[:as] || controller_name}" shallow_path = "/#{options[:shallow] ? options[:namespace] : options[:path_prefix]}#{path}" @@ -1223,29 +1223,29 @@ class ResourcesTest < ActionController::TestCase edit_action = options[:path_names][:edit] || "edit" end - assert_named_route "#{full_path}", "#{name_prefix}#{controller_name}_path", options[:options] - assert_named_route "#{full_path}.xml", "#{name_prefix}#{controller_name}_path", options[:options].merge(:format => 'xml') + assert_named_route "#{full_path}", "#{name_prefix}#{controller_name}_path", route_options + assert_named_route "#{full_path}.xml", "#{name_prefix}#{controller_name}_path", route_options.merge(:format => 'xml') assert_named_route "#{shallow_path}/1", "#{shallow_prefix}#{singular_name}_path", options[:shallow_options].merge(:id => '1') assert_named_route "#{shallow_path}/1.xml", "#{shallow_prefix}#{singular_name}_path", options[:shallow_options].merge(:id => '1', :format => 'xml') - assert_named_route "#{full_path}/#{new_action}", "new_#{name_prefix}#{singular_name}_path", options[:options] - assert_named_route "#{full_path}/#{new_action}.xml", "new_#{name_prefix}#{singular_name}_path", options[:options].merge(:format => 'xml') + assert_named_route "#{full_path}/#{new_action}", "new_#{name_prefix}#{singular_name}_path", route_options + assert_named_route "#{full_path}/#{new_action}.xml", "new_#{name_prefix}#{singular_name}_path", route_options.merge(:format => 'xml') assert_named_route "#{shallow_path}/1/#{edit_action}", "edit_#{shallow_prefix}#{singular_name}_path", options[:shallow_options].merge(:id => '1') assert_named_route "#{shallow_path}/1/#{edit_action}.xml", "edit_#{shallow_prefix}#{singular_name}_path", options[:shallow_options].merge(:id => '1', :format => 'xml') - yield options[:options] if block_given? + yield route_options if block_given? end def assert_singleton_routes_for(singleton_name, options = {}) - options[:options] ||= {} - options[:options][:controller] = options[:controller] || singleton_name.to_s.pluralize + route_options = (options[:options] ||= {}).dup + route_options[:controller] = options[:controller] || singleton_name.to_s.pluralize full_path = "/#{options[:path_prefix]}#{options[:as] || singleton_name}" new_path = "#{full_path}/new" edit_path = "#{full_path}/edit" formatted_edit_path = "#{full_path}/edit.xml" - with_options options[:options] do |controller| + with_options route_options do |controller| controller.assert_routing full_path, :action => 'show' controller.assert_routing new_path, :action => 'new' controller.assert_routing edit_path, :action => 'edit' @@ -1254,40 +1254,41 @@ class ResourcesTest < ActionController::TestCase controller.assert_routing formatted_edit_path, :action => 'edit', :format => 'xml' end - assert_recognizes(options[:options].merge(:action => 'show'), :path => full_path, :method => :get) - assert_recognizes(options[:options].merge(:action => 'new'), :path => new_path, :method => :get) - assert_recognizes(options[:options].merge(:action => 'edit'), :path => edit_path, :method => :get) - assert_recognizes(options[:options].merge(:action => 'create'), :path => full_path, :method => :post) - assert_recognizes(options[:options].merge(:action => 'update'), :path => full_path, :method => :put) - assert_recognizes(options[:options].merge(:action => 'destroy'), :path => full_path, :method => :delete) + assert_recognizes(route_options.merge(:action => 'show'), :path => full_path, :method => :get) + assert_recognizes(route_options.merge(:action => 'new'), :path => new_path, :method => :get) + assert_recognizes(route_options.merge(:action => 'edit'), :path => edit_path, :method => :get) + assert_recognizes(route_options.merge(:action => 'create'), :path => full_path, :method => :post) + assert_recognizes(route_options.merge(:action => 'update'), :path => full_path, :method => :put) + assert_recognizes(route_options.merge(:action => 'destroy'), :path => full_path, :method => :delete) - assert_recognizes(options[:options].merge(:action => 'show', :format => 'xml'), :path => "#{full_path}.xml", :method => :get) - assert_recognizes(options[:options].merge(:action => 'new', :format => 'xml'), :path => "#{new_path}.xml", :method => :get) - assert_recognizes(options[:options].merge(:action => 'edit', :format => 'xml'), :path => formatted_edit_path, :method => :get) - assert_recognizes(options[:options].merge(:action => 'create', :format => 'xml'), :path => "#{full_path}.xml", :method => :post) - assert_recognizes(options[:options].merge(:action => 'update', :format => 'xml'), :path => "#{full_path}.xml", :method => :put) - assert_recognizes(options[:options].merge(:action => 'destroy', :format => 'xml'), :path => "#{full_path}.xml", :method => :delete) + assert_recognizes(route_options.merge(:action => 'show', :format => 'xml'), :path => "#{full_path}.xml", :method => :get) + assert_recognizes(route_options.merge(:action => 'new', :format => 'xml'), :path => "#{new_path}.xml", :method => :get) + assert_recognizes(route_options.merge(:action => 'edit', :format => 'xml'), :path => formatted_edit_path, :method => :get) + assert_recognizes(route_options.merge(:action => 'create', :format => 'xml'), :path => "#{full_path}.xml", :method => :post) + assert_recognizes(route_options.merge(:action => 'update', :format => 'xml'), :path => "#{full_path}.xml", :method => :put) + assert_recognizes(route_options.merge(:action => 'destroy', :format => 'xml'), :path => "#{full_path}.xml", :method => :delete) - yield options[:options] if block_given? + yield route_options if block_given? end def assert_singleton_named_routes_for(singleton_name, options = {}) - (options[:options] ||= {})[:controller] ||= singleton_name.to_s.pluralize - @controller = "#{options[:options][:controller].camelize}Controller".constantize.new + route_options = (options[:options] ||= {}).dup + controller_name = route_options[:controller] || options[:controller] || singleton_name.to_s.pluralize + @controller = "#{controller_name.camelize}Controller".constantize.new @controller.singleton_class.include(@routes.url_helpers) - get :show, params: options[:options] - options[:options].delete :action + get :show, params: route_options + route_options.delete :action full_path = "/#{options[:path_prefix]}#{options[:as] || singleton_name}" name_prefix = options[:name_prefix] - assert_named_route "#{full_path}", "#{name_prefix}#{singleton_name}_path", options[:options] - assert_named_route "#{full_path}.xml", "#{name_prefix}#{singleton_name}_path", options[:options].merge(:format => 'xml') + assert_named_route "#{full_path}", "#{name_prefix}#{singleton_name}_path", route_options + assert_named_route "#{full_path}.xml", "#{name_prefix}#{singleton_name}_path", route_options.merge(:format => 'xml') - assert_named_route "#{full_path}/new", "new_#{name_prefix}#{singleton_name}_path", options[:options] - assert_named_route "#{full_path}/new.xml", "new_#{name_prefix}#{singleton_name}_path", options[:options].merge(:format => 'xml') - assert_named_route "#{full_path}/edit", "edit_#{name_prefix}#{singleton_name}_path", options[:options] - assert_named_route "#{full_path}/edit.xml", "edit_#{name_prefix}#{singleton_name}_path", options[:options].merge(:format => 'xml') + assert_named_route "#{full_path}/new", "new_#{name_prefix}#{singleton_name}_path", route_options + assert_named_route "#{full_path}/new.xml", "new_#{name_prefix}#{singleton_name}_path", route_options.merge(:format => 'xml') + assert_named_route "#{full_path}/edit", "edit_#{name_prefix}#{singleton_name}_path", route_options + assert_named_route "#{full_path}/edit.xml", "edit_#{name_prefix}#{singleton_name}_path", route_options.merge(:format => 'xml') end def assert_named_route(expected, route, options) diff --git a/actionpack/test/dispatch/exception_wrapper_test.rb b/actionpack/test/dispatch/exception_wrapper_test.rb index 7a29a7ff97..3dac582407 100644 --- a/actionpack/test/dispatch/exception_wrapper_test.rb +++ b/actionpack/test/dispatch/exception_wrapper_test.rb @@ -19,15 +19,13 @@ module ActionDispatch setup do Rails.stubs(:root).returns(Pathname.new('.')) - cleaner = ActiveSupport::BacktraceCleaner.new - cleaner.add_silencer { |line| line !~ /^lib/ } - - @environment = { 'action_dispatch.backtrace_cleaner' => cleaner } + @cleaner = ActiveSupport::BacktraceCleaner.new + @cleaner.add_silencer { |line| line !~ /^lib/ } end test '#source_extracts fetches source fragments for every backtrace entry' do exception = TestError.new("lib/file.rb:42:in `index'") - wrapper = ExceptionWrapper.new({}, exception) + wrapper = ExceptionWrapper.new(nil, exception) wrapper.expects(:source_fragment).with('lib/file.rb', 42).returns('foo') @@ -37,7 +35,7 @@ module ActionDispatch test '#source_extracts works with Windows paths' do exc = TestError.new("c:/path/to/rails/app/controller.rb:27:in 'index':") - wrapper = ExceptionWrapper.new({}, exc) + wrapper = ExceptionWrapper.new(nil, exc) wrapper.expects(:source_fragment).with('c:/path/to/rails/app/controller.rb', 27).returns('nothing') assert_equal [ code: 'nothing', line_number: 27 ], wrapper.source_extracts @@ -46,7 +44,7 @@ module ActionDispatch test '#source_extracts works with non standard backtrace' do exc = TestError.new('invalid') - wrapper = ExceptionWrapper.new({}, exc) + wrapper = ExceptionWrapper.new(nil, exc) wrapper.expects(:source_fragment).with('invalid', 0).returns('nothing') assert_equal [ code: 'nothing', line_number: 0 ], wrapper.source_extracts @@ -54,14 +52,14 @@ module ActionDispatch test '#application_trace returns traces only from the application' do exception = TestError.new(caller.prepend("lib/file.rb:42:in `index'")) - wrapper = ExceptionWrapper.new(@environment, exception) + wrapper = ExceptionWrapper.new(@cleaner, exception) assert_equal [ "lib/file.rb:42:in `index'" ], wrapper.application_trace end test '#application_trace cannot be nil' do - nil_backtrace_wrapper = ExceptionWrapper.new(@environment, BadlyDefinedError.new) - nil_cleaner_wrapper = ExceptionWrapper.new({}, BadlyDefinedError.new) + nil_backtrace_wrapper = ExceptionWrapper.new(@cleaner, BadlyDefinedError.new) + nil_cleaner_wrapper = ExceptionWrapper.new(nil, BadlyDefinedError.new) assert_equal [], nil_backtrace_wrapper.application_trace assert_equal [], nil_cleaner_wrapper.application_trace @@ -69,14 +67,14 @@ module ActionDispatch test '#framework_trace returns traces outside the application' do exception = TestError.new(caller.prepend("lib/file.rb:42:in `index'")) - wrapper = ExceptionWrapper.new(@environment, exception) + wrapper = ExceptionWrapper.new(@cleaner, exception) assert_equal caller, wrapper.framework_trace end test '#framework_trace cannot be nil' do - nil_backtrace_wrapper = ExceptionWrapper.new(@environment, BadlyDefinedError.new) - nil_cleaner_wrapper = ExceptionWrapper.new({}, BadlyDefinedError.new) + nil_backtrace_wrapper = ExceptionWrapper.new(@cleaner, BadlyDefinedError.new) + nil_cleaner_wrapper = ExceptionWrapper.new(nil, BadlyDefinedError.new) assert_equal [], nil_backtrace_wrapper.framework_trace assert_equal [], nil_cleaner_wrapper.framework_trace @@ -84,14 +82,14 @@ module ActionDispatch test '#full_trace returns application and framework traces' do exception = TestError.new(caller.prepend("lib/file.rb:42:in `index'")) - wrapper = ExceptionWrapper.new(@environment, exception) + wrapper = ExceptionWrapper.new(@cleaner, exception) assert_equal exception.backtrace, wrapper.full_trace end test '#full_trace cannot be nil' do - nil_backtrace_wrapper = ExceptionWrapper.new(@environment, BadlyDefinedError.new) - nil_cleaner_wrapper = ExceptionWrapper.new({}, BadlyDefinedError.new) + nil_backtrace_wrapper = ExceptionWrapper.new(@cleaner, BadlyDefinedError.new) + nil_cleaner_wrapper = ExceptionWrapper.new(nil, BadlyDefinedError.new) assert_equal [], nil_backtrace_wrapper.full_trace assert_equal [], nil_cleaner_wrapper.full_trace @@ -99,7 +97,7 @@ module ActionDispatch test '#traces returns every trace by category enumerated with an index' do exception = TestError.new("lib/file.rb:42:in `index'", "/gems/rack.rb:43:in `index'") - wrapper = ExceptionWrapper.new(@environment, exception) + wrapper = ExceptionWrapper.new(@cleaner, exception) assert_equal({ 'Application Trace' => [ id: 0, trace: "lib/file.rb:42:in `index'" ], diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb index 889f9a4736..b0815c41c8 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -19,6 +19,10 @@ module ActionDispatch ActionDispatch::Request end + def dispatcher_class + RouteSet::Dispatcher + end + def add_route(*args) routes << args end @@ -37,8 +41,8 @@ module ActionDispatch end def test_mapping_requirements - options = { :controller => 'foo', :action => 'bar', :via => :get } - m = Mapper::Mapping.build({}, FakeSet.new, '/store/:name(*rest)', nil, options) + options = { :via => :get } + m = Mapper::Mapping.build({}, FakeSet.new, '/store/:name(*rest)', nil, 'foo', 'bar', nil, options) _, _, requirements, _ = m.to_route assert_equal(/.+?/, requirements[:rest]) end diff --git a/actionpack/test/dispatch/middleware_stack/middleware_test.rb b/actionpack/test/dispatch/middleware_stack/middleware_test.rb deleted file mode 100644 index 9607f026db..0000000000 --- a/actionpack/test/dispatch/middleware_stack/middleware_test.rb +++ /dev/null @@ -1,77 +0,0 @@ -require 'abstract_unit' -require 'action_dispatch/middleware/stack' - -module ActionDispatch - class MiddlewareStack - class MiddlewareTest < ActiveSupport::TestCase - class Omg; end - - { - 'concrete' => Omg, - 'anonymous' => Class.new - }.each do |name, klass| - - define_method("test_#{name}_klass") do - mw = Middleware.new klass - assert_equal klass, mw.klass - end - - define_method("test_#{name}_==") do - mw1 = Middleware.new klass - mw2 = Middleware.new klass - assert_equal mw1, mw2 - end - - end - - def test_string_class - mw = Middleware.new Omg.name - assert_equal Omg, mw.klass - end - - def test_double_equal_works_with_classes - k = Class.new - mw = Middleware.new k - assert_operator mw, :==, k - - result = mw != Class.new - assert result, 'middleware should not equal other anon class' - end - - def test_double_equal_works_with_strings - mw = Middleware.new Omg - assert_operator mw, :==, Omg.name - end - - def test_double_equal_normalizes_strings - mw = Middleware.new Omg - assert_operator mw, :==, "::#{Omg.name}" - end - - def test_middleware_loads_classnames_from_cache - mw = Class.new(Middleware) { - attr_accessor :classcache - }.new(Omg.name) - - fake_cache = { mw.name => Omg } - mw.classcache = fake_cache - - assert_equal Omg, mw.klass - - fake_cache[mw.name] = Middleware - assert_equal Middleware, mw.klass - end - - def test_middleware_always_returns_class - mw = Class.new(Middleware) { - attr_accessor :classcache - }.new(Omg) - - fake_cache = { mw.name => Middleware } - mw.classcache = fake_cache - - assert_equal Omg, mw.klass - end - end - end -end diff --git a/actionpack/test/dispatch/middleware_stack_test.rb b/actionpack/test/dispatch/middleware_stack_test.rb index 948a690979..33aa616474 100644 --- a/actionpack/test/dispatch/middleware_stack_test.rb +++ b/actionpack/test/dispatch/middleware_stack_test.rb @@ -4,6 +4,7 @@ class MiddlewareStackTest < ActiveSupport::TestCase class FooMiddleware; end class BarMiddleware; end class BazMiddleware; end + class HiyaMiddleware; end class BlockMiddleware attr_reader :block def initialize(&block) @@ -17,6 +18,20 @@ class MiddlewareStackTest < ActiveSupport::TestCase @stack.use BarMiddleware end + def test_delete_with_string_is_deprecated + assert_deprecated do + assert_difference "@stack.size", -1 do + @stack.delete FooMiddleware.name + end + end + end + + def test_delete_works + assert_difference "@stack.size", -1 do + @stack.delete FooMiddleware + end + end + test "use should push middleware as class onto the stack" do assert_difference "@stack.size" do @stack.use BazMiddleware @@ -25,17 +40,21 @@ class MiddlewareStackTest < ActiveSupport::TestCase end test "use should push middleware as a string onto the stack" do - assert_difference "@stack.size" do - @stack.use "MiddlewareStackTest::BazMiddleware" + assert_deprecated do + assert_difference "@stack.size" do + @stack.use "MiddlewareStackTest::BazMiddleware" + end + assert_equal BazMiddleware, @stack.last.klass end - assert_equal BazMiddleware, @stack.last.klass end test "use should push middleware as a symbol onto the stack" do - assert_difference "@stack.size" do - @stack.use :"MiddlewareStackTest::BazMiddleware" + assert_deprecated do + assert_difference "@stack.size" do + @stack.use :"MiddlewareStackTest::BazMiddleware" + end + assert_equal BazMiddleware, @stack.last.klass end - assert_equal BazMiddleware, @stack.last.klass end test "use should push middleware class with arguments onto the stack" do @@ -88,30 +107,28 @@ class MiddlewareStackTest < ActiveSupport::TestCase end test "unshift adds a new middleware at the beginning of the stack" do - @stack.unshift :"MiddlewareStackTest::BazMiddleware" - assert_equal BazMiddleware, @stack.first.klass + assert_deprecated do + @stack.unshift :"MiddlewareStackTest::BazMiddleware" + assert_equal BazMiddleware, @stack.first.klass + end end test "raise an error on invalid index" do assert_raise RuntimeError do - @stack.insert("HiyaMiddleware", BazMiddleware) + @stack.insert(HiyaMiddleware, BazMiddleware) end assert_raise RuntimeError do - @stack.insert_after("HiyaMiddleware", BazMiddleware) + @stack.insert_after(HiyaMiddleware, BazMiddleware) end end test "lazy evaluates middleware class" do - assert_difference "@stack.size" do - @stack.use "MiddlewareStackTest::BazMiddleware" + assert_deprecated do + assert_difference "@stack.size" do + @stack.use "MiddlewareStackTest::BazMiddleware" + end + assert_equal BazMiddleware, @stack.last.klass end - assert_equal BazMiddleware, @stack.last.klass - end - - test "lazy compares so unloaded constants are not loaded" do - @stack.use "UnknownMiddleware" - @stack.use :"MiddlewareStackTest::BazMiddleware" - assert @stack.include?("::MiddlewareStackTest::BazMiddleware") end end diff --git a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb index 939a771c65..b36fbd3c76 100644 --- a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb @@ -63,6 +63,17 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest assert_equal 'contents', file.read end + test "parses utf8 filename with percent character" do + params = parse_multipart('utf8_filename') + assert_equal %w(file foo), params.keys.sort + assert_equal 'bar', params['foo'] + + file = params['file'] + assert_equal 'ファイル%名.txt', file.original_filename + assert_equal "text/plain", file.content_type + assert_equal 'contents', file.read + end + test "parses boundary problem file" do params = parse_multipart('boundary_problem_file') assert_equal %w(file foo), params.keys.sort diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index b18a9ab647..280b258da6 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -168,12 +168,10 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_session_singleton_resource_for_api_app - self.class.stub_controllers do |_| - config = ActionDispatch::Routing::RouteSet::Config.new - config.api_only = true - - routes = ActionDispatch::Routing::RouteSet.new(config) + config = ActionDispatch::Routing::RouteSet::Config.new + config.api_only = true + self.class.stub_controllers(config) do |routes| routes.draw do resource :session do get :create @@ -550,11 +548,10 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_projects_for_api_app - self.class.stub_controllers do |_| - config = ActionDispatch::Routing::RouteSet::Config.new - config.api_only = true + config = ActionDispatch::Routing::RouteSet::Config.new + config.api_only = true - routes = ActionDispatch::Routing::RouteSet.new(config) + self.class.stub_controllers(config) do |routes| routes.draw do resources :projects, controller: :project end diff --git a/actionpack/test/dispatch/session/cache_store_test.rb b/actionpack/test/dispatch/session/cache_store_test.rb index e6a70358c8..dbb996973d 100644 --- a/actionpack/test/dispatch/session/cache_store_test.rb +++ b/actionpack/test/dispatch/session/cache_store_test.rb @@ -170,7 +170,7 @@ class CacheStoreTest < ActionDispatch::IntegrationTest @app = self.class.build_app(set) do |middleware| @cache = ActiveSupport::Cache::MemoryStore.new middleware.use ActionDispatch::Session::CacheStore, :key => '_session_id', :cache => @cache - middleware.delete "ActionDispatch::ShowExceptions" + middleware.delete ActionDispatch::ShowExceptions end yield diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index 715eb90566..e432c65c62 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -348,7 +348,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest @app = self.class.build_app(set) do |middleware| middleware.use ActionDispatch::Session::CookieStore, options - middleware.delete "ActionDispatch::ShowExceptions" + middleware.delete ActionDispatch::ShowExceptions end yield diff --git a/actionpack/test/dispatch/session/mem_cache_store_test.rb b/actionpack/test/dispatch/session/mem_cache_store_test.rb index 6e9107ecbf..3fed9bad4f 100644 --- a/actionpack/test/dispatch/session/mem_cache_store_test.rb +++ b/actionpack/test/dispatch/session/mem_cache_store_test.rb @@ -192,7 +192,7 @@ class MemCacheStoreTest < ActionDispatch::IntegrationTest @app = self.class.build_app(set) do |middleware| middleware.use ActionDispatch::Session::MemCacheStore, :key => '_session_id', :namespace => "mem_cache_store_test:#{SecureRandom.hex(10)}" - middleware.delete "ActionDispatch::ShowExceptions" + middleware.delete ActionDispatch::ShowExceptions end yield diff --git a/actionpack/test/dispatch/session/test_session_test.rb b/actionpack/test/dispatch/session/test_session_test.rb index d30461a623..59c030176b 100644 --- a/actionpack/test/dispatch/session/test_session_test.rb +++ b/actionpack/test/dispatch/session/test_session_test.rb @@ -40,4 +40,14 @@ class ActionController::TestSessionTest < ActiveSupport::TestCase assert_equal %w(one two), session.keys assert_equal %w(1 2), session.values end + + def test_fetch_returns_default + session = ActionController::TestSession.new(one: '1') + assert_equal('2', session.fetch(:two, '2')) + end + + def test_fetch_returns_block_value + session = ActionController::TestSession.new(one: '1') + assert_equal(2, session.fetch('2') { |key| key.to_i }) + end end diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb index 95971b3a0e..13dec8b618 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -156,7 +156,7 @@ module StaticTests def test_does_not_modify_path_info file_name = "/gzip/application-a71b3024f80aea3181c09774ca17e712.js" - env = {'PATH_INFO' => file_name, 'HTTP_ACCEPT_ENCODING' => 'gzip'} + env = {'PATH_INFO' => file_name, 'HTTP_ACCEPT_ENCODING' => 'gzip', "REQUEST_METHOD" => 'POST'} @app.call(env) assert_equal file_name, env['PATH_INFO'] end diff --git a/actionpack/test/fixtures/multipart/utf8_filename b/actionpack/test/fixtures/multipart/utf8_filename new file mode 100644 index 0000000000..60738d53b0 --- /dev/null +++ b/actionpack/test/fixtures/multipart/utf8_filename @@ -0,0 +1,10 @@ +--AaB03x
+Content-Disposition: form-data; name="foo"
+
+bar
+--AaB03x
+Content-Disposition: form-data; name="file"; filename="ファイル%名.txt"
+Content-Type: text/plain
+
+contents
+--AaB03x--
|