diff options
Diffstat (limited to 'actionpack/lib')
93 files changed, 1843 insertions, 860 deletions
diff --git a/actionpack/lib/abstract_controller/layouts.rb b/actionpack/lib/abstract_controller/layouts.rb index 73799e8085..91864f2a35 100644 --- a/actionpack/lib/abstract_controller/layouts.rb +++ b/actionpack/lib/abstract_controller/layouts.rb @@ -209,7 +209,7 @@ module AbstractController _write_layout_method end - delegate :_layout_conditions, :to => "self.class" + delegate :_layout_conditions, to: :class module ClassMethods def inherited(klass) # :nodoc: diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 4b984d0558..f8e4cb4384 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -97,15 +97,23 @@ module AbstractController self.response_body = render_to_body(options) end - # Raw rendering of a template to a string. Just convert the results of - # render_response into a String. + # Raw rendering of a template to a string. + # + # It is similar to render, except that it does not + # set the response_body and it should be guaranteed + # to always return a string. + # + # If a component extends the semantics of response_body + # (as Action Controller extends it to be anything that + # responds to the method each), this method needs to + # overriden in order to still return a string. # :api: plugin def render_to_string(*args, &block) options = _normalize_render(*args, &block) render_to_body(options) end - # Raw rendering of a template to a Rack-compatible body. + # Raw rendering of a template. # :api: plugin def render_to_body(options = {}) _process_options(options) diff --git a/actionpack/lib/abstract_controller/translation.rb b/actionpack/lib/abstract_controller/translation.rb index b6c484d188..02028d8e05 100644 --- a/actionpack/lib/abstract_controller/translation.rb +++ b/actionpack/lib/abstract_controller/translation.rb @@ -1,9 +1,17 @@ module AbstractController module Translation + # Delegates to <tt>I18n.translate</tt>. Also aliased as <tt>t</tt>. + # + # When the given key starts with a period, it will be scoped by the current + # controller and action. So if you call <tt>translate(".foo")</tt> from + # <tt>PeopleController#index</tt>, it will convert the call to + # <tt>I18n.translate("people.index.foo")</tt>. This makes it less repetitive + # to translate many keys within the same controller / action and gives you a + # simple framework for scoping them consistently. def translate(*args) key = args.first if key.is_a?(String) && (key[0] == '.') - key = "#{ controller_path.gsub('/', '.') }.#{ action_name }#{ key }" + key = "#{ controller_path.tr('/', '.') }.#{ action_name }#{ key }" args[0] = key end @@ -11,6 +19,7 @@ module AbstractController end alias :t :translate + # Delegates to <tt>I18n.localize</tt>. Also aliased as <tt>l</tt>. def localize(*args) I18n.localize(*args) end diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 1a13d7af29..9cacb3862b 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -42,7 +42,6 @@ module ActionController autoload :Integration, 'action_controller/deprecated/integration_test' autoload :IntegrationTest, 'action_controller/deprecated/integration_test' - autoload :PerformanceTest, 'action_controller/deprecated/performance_test' autoload :Routing, 'action_controller/deprecated' autoload :TestCase, 'action_controller/test_case' autoload :TemplateAssertions, 'action_controller/test_case' diff --git a/actionpack/lib/action_controller/caching.rb b/actionpack/lib/action_controller/caching.rb index 2892e093af..ea33d975ef 100644 --- a/actionpack/lib/action_controller/caching.rb +++ b/actionpack/lib/action_controller/caching.rb @@ -70,10 +70,20 @@ module ActionController config_accessor :perform_caching self.perform_caching = true if perform_caching.nil? + + class_attribute :_view_cache_dependencies + self._view_cache_dependencies = [] + helper_method :view_cache_dependencies if respond_to?(:helper_method) + end + + module ClassMethods + def view_cache_dependency(&dependency) + self._view_cache_dependencies += [dependency] + end end - def caching_allowed? - request.get? && response.status == 200 + def view_cache_dependencies + self.class._view_cache_dependencies.map { |dep| instance_exec(&dep) }.compact end protected diff --git a/actionpack/lib/action_controller/deprecated/performance_test.rb b/actionpack/lib/action_controller/deprecated/performance_test.rb deleted file mode 100644 index c7ba5a2fe7..0000000000 --- a/actionpack/lib/action_controller/deprecated/performance_test.rb +++ /dev/null @@ -1,3 +0,0 @@ -ActionController::PerformanceTest = ActionDispatch::PerformanceTest - -ActiveSupport::Deprecation.warn 'ActionController::PerformanceTest is deprecated and will be removed, use ActionDispatch::PerformanceTest instead.' diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 832dec7b2a..143b3e0cbd 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -1,3 +1,4 @@ +require 'active_support/core_ext/array/extract_options' require 'action_dispatch/middleware/stack' module ActionController diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index 3f9b382a11..6e0cd51d8b 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -1,4 +1,4 @@ -require 'active_support/core_ext/class/attribute' +require 'active_support/core_ext/hash/keys' module ActionController module ConditionalGet diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 896238b7dc..e295002b16 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -228,7 +228,7 @@ module ActionController end def decode_credentials(header) - HashWithIndifferentAccess[header.to_s.gsub(/^Digest\s+/,'').split(',').map do |pair| + ActiveSupport::HashWithIndifferentAccess[header.to_s.gsub(/^Digest\s+/, '').split(',').map do |pair| key, value = pair.split('=', 2) [key.strip, value.to_s.gsub(/^"|"$/,'').delete('\'')] end] diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 6bf306ac5b..834d44f045 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -1,3 +1,4 @@ +require 'active_support/core_ext/array/extract_options' require 'abstract_controller/collector' module ActionController #:nodoc: @@ -82,7 +83,7 @@ module ActionController #:nodoc: # (by name) if it does not already exist, without web-services, it might look like this: # # def create - # @company = Company.find_or_create_by_name(params[:company][:name]) + # @company = Company.find_or_create_by(name: params[:company][:name]) # @person = @company.people.create(params[:person]) # # redirect_to(person_list_url) @@ -92,7 +93,7 @@ module ActionController #:nodoc: # # def create # company = params[:person].delete(:company) - # @company = Company.find_or_create_by_name(company[:name]) + # @company = Company.find_or_create_by(name: company[:name]) # @person = @company.people.create(params[:person]) # # respond_to do |format| @@ -120,7 +121,7 @@ module ActionController #:nodoc: # Note, however, the extra bit at the top of that action: # # company = params[:person].delete(:company) - # @company = Company.find_or_create_by_name(company[:name]) + # @company = Company.find_or_create_by(name: company[:name]) # # This is because the incoming XML document (if a web-service request is in process) can only contain a # single root-node. So, we have to rearrange things so that the request looks like this (url-encoded): @@ -227,7 +228,7 @@ module ActionController #:nodoc: # i.e. its +show+ action. # 2. If there are validation errors, the response # renders a default action, which is <tt>:new</tt> for a - # +post+ request or <tt>:edit</tt> for +put+. + # +post+ request or <tt>:edit</tt> for +patch+ or +put+. # Thus an example like this - # # respond_to :html, :xml @@ -320,7 +321,7 @@ module ActionController #:nodoc: # 2. <tt>:action</tt> - overwrites the default render action used after an # unsuccessful html +post+ request. def respond_with(*resources, &block) - raise "In order to use respond_with, first you need to declare the formats your " << + raise "In order to use respond_with, first you need to declare the formats your " \ "controller responds to in the class level" if self.class.mimes_for_respond_to.empty? if collector = retrieve_collector_from_mimes(&block) @@ -419,7 +420,7 @@ module ActionController #:nodoc: end def response - @responses[format] || @responses[Mime::ALL] + @responses.fetch(format, @responses[Mime::ALL]) end def negotiate_format(request) diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 091facfd8d..e9031f3fac 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -32,7 +32,7 @@ module ActionController # redirect_to :back # redirect_to proc { edit_post_url(@post) } # - # The redirection happens as a "302 Moved" header unless otherwise specified. + # The redirection happens as a "302 Found" header unless otherwise specified. # # redirect_to post_url(@post), status: :found # redirect_to action: 'atom', status: :moved_permanently @@ -65,7 +65,6 @@ module ActionController def redirect_to(options = {}, response_status = {}) #:doc: raise ActionControllerError.new("Cannot redirect to nil!") unless options raise AbstractController::DoubleRenderError if response_body - logger.debug { "Redirected by #{caller(1).first rescue "unknown"}" } if logger self.status = _extract_redirect_to_status(options, response_status) self.location = _compute_redirect_to_location(options) @@ -89,7 +88,7 @@ module ActionController # letters, digits, and the plus ("+"), period ("."), or hyphen ("-") # characters; and is terminated by a colon (":"). # The protocol relative scheme starts with a double slash "//" - when %r{^(\w[\w+.-]*:|//).*} + when %r{\A(\w[\w+.-]*:|//).*} options when String request.protocol + request.host_with_port + options diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index c5db0cb0d4..d275a854fd 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -50,6 +50,10 @@ module ActionController #:nodoc: config_accessor :request_forgery_protection_token self.request_forgery_protection_token ||= :authenticity_token + # Holds the class which implements the request forgery protection. + config_accessor :forgery_protection_strategy + self.forgery_protection_strategy = nil + # Controls whether request forgery protection is turned on or not. Turned off by default only in test mode. config_accessor :allow_forgery_protection self.allow_forgery_protection = true if allow_forgery_protection.nil? @@ -82,14 +86,14 @@ module ActionController #:nodoc: # * <tt>:reset_session</tt> - Resets the session. # * <tt>:null_session</tt> - Provides an empty session during request but doesn't reset it completely. Used as default if <tt>:with</tt> option is not specified. def protect_from_forgery(options = {}) - include protection_method_module(options[:with] || :null_session) + self.forgery_protection_strategy = protection_method_class(options[:with] || :null_session) self.request_forgery_protection_token ||= :authenticity_token prepend_before_action :verify_authenticity_token, options end private - def protection_method_module(name) + def protection_method_class(name) ActionController::RequestForgeryProtection::ProtectionMethods.const_get(name.to_s.classify) rescue NameError raise ArgumentError, 'Invalid request forgery protection method, use :null_session, :exception, or :reset_session' @@ -97,20 +101,26 @@ module ActionController #:nodoc: end module ProtectionMethods - module NullSession - protected + class NullSession + def initialize(controller) + @controller = controller + end # This is the method that defines the application behavior when a request is found to be unverified. def handle_unverified_request - request.session = NullSessionHash.new + request = @controller.request + 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) end + protected + class NullSessionHash < Rack::Session::Abstract::SessionHash #:nodoc: - def initialize - super(nil, nil) + def initialize(env) + super(nil, env) + @data = {} @loaded = true end @@ -125,7 +135,7 @@ module ActionController #:nodoc: host = request.host secure = request.ssl? - new(key_generator, host, secure) + new(key_generator, host, secure, options_for_env({})) end def write(*) @@ -134,16 +144,20 @@ module ActionController #:nodoc: end end - module ResetSession - protected + class ResetSession + def initialize(controller) + @controller = controller + end def handle_unverified_request - reset_session + @controller.reset_session end end - module Exception - protected + class Exception + def initialize(controller) + @controller = controller + end def handle_unverified_request raise ActionController::InvalidAuthenticityToken @@ -152,6 +166,10 @@ module ActionController #:nodoc: end protected + def handle_unverified_request + forgery_protection_strategy.new(self).handle_unverified_request + end + # The actual before_action that is used. Modify this to change how you handle unverified requests. def verify_authenticity_token unless verified_request? @@ -162,11 +180,11 @@ module ActionController #:nodoc: # Returns true or false if a request is verified. Checks: # - # * is it a GET request? Gets should be safe and idempotent + # * is it a GET or HEAD request? Gets should be safe and idempotent # * Does the form_authenticity_token match the given token value from the params? # * Does the X-CSRF-Token header match the form_authenticity_token def verified_request? - !protect_against_forgery? || request.get? || + !protect_against_forgery? || request.get? || request.head? || form_authenticity_token == params[request_forgery_protection_token] || form_authenticity_token == request.headers['X-CSRF-Token'] end diff --git a/actionpack/lib/action_controller/metal/streaming.rb b/actionpack/lib/action_controller/metal/streaming.rb index 0b3c438ec2..73e9b5660d 100644 --- a/actionpack/lib/action_controller/metal/streaming.rb +++ b/actionpack/lib/action_controller/metal/streaming.rb @@ -26,7 +26,7 @@ module ActionController #:nodoc: # # class PostsController # def index - # @posts = Post.scoped + # @posts = Post.all # render stream: true # end # end @@ -51,9 +51,9 @@ module ActionController #:nodoc: # # def dashboard # # Allow lazy execution of the queries - # @posts = Post.scoped - # @pages = Page.scoped - # @articles = Article.scoped + # @posts = Post.all + # @pages = Page.all + # @articles = Article.all # render stream: true # end # diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 8faa5f8a13..e4dcd3213f 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -1,7 +1,7 @@ -require 'active_support/concern' require 'active_support/core_ext/hash/indifferent_access' require 'active_support/core_ext/array/wrap' require 'active_support/rescuable' +require 'action_dispatch/http/upload' module ActionController # Raised when a required parameter is missing. @@ -20,6 +20,20 @@ module ActionController end end + # Raised when a supplied parameter is not expected. + # + # params = ActionController::Parameters.new(a: "123", b: "456") + # params.permit(:c) + # # => ActionController::UnpermittedParameters: found unexpected keys: a, b + class UnpermittedParameters < IndexError + attr_reader :params # :nodoc: + + def initialize(params) # :nodoc: + @params = params + super("found unpermitted parameters: #{params.join(", ")}") + end + end + # == Action Controller \Parameters # # Allows to choose which attributes should be whitelisted for mass updating @@ -41,13 +55,18 @@ module ActionController # permitted.class # => ActionController::Parameters # permitted.permitted? # => true # - # Person.first.update_attributes!(permitted) + # Person.first.update!(permitted) # # => #<Person id: 1, name: "Francesco", age: 22, role: "user"> # - # It provides a +permit_all_parameters+ option that controls the top-level - # behaviour of new instances. If it's +true+, all the parameters will be - # permitted by default. The default value for +permit_all_parameters+ - # option is +false+. + # It provides two options that controls the top-level behavior of new instances: + # + # * +permit_all_parameters+ - If it's +true+, all the parameters will be + # permitted by default. The default is +false+. + # * +action_on_unpermitted_parameters+ - Allow to control the behavior when parameters + # that are not explicitly permitted are found. The values can be <tt>:log</tt> to + # write a message on the logger or <tt>:raise</tt> to raise + # ActionController::UnpermittedParameters exception. The default value is <tt>:log</tt> + # in test and development environments, +false+ otherwise. # # params = ActionController::Parameters.new # params.permitted? # => false @@ -57,6 +76,16 @@ module ActionController # params = ActionController::Parameters.new # params.permitted? # => true # + # params = ActionController::Parameters.new(a: "123", b: "456") + # params.permit(:c) + # # => {} + # + # ActionController::Parameters.action_on_unpermitted_parameters = :raise + # + # params = ActionController::Parameters.new(a: "123", b: "456") + # params.permit(:c) + # # => ActionController::UnpermittedParameters: found unpermitted keys: a, b + # # <tt>ActionController::Parameters</tt> is inherited from # <tt>ActiveSupport::HashWithIndifferentAccess</tt>, this means # that you can fetch values using either <tt>:key</tt> or <tt>"key"</tt>. @@ -66,6 +95,11 @@ module ActionController # params["key"] # => "value" class Parameters < ActiveSupport::HashWithIndifferentAccess cattr_accessor :permit_all_parameters, instance_accessor: false + cattr_accessor :action_on_unpermitted_parameters, instance_accessor: false + + # Never raise an UnpermittedParameters exception because of these params + # are present. They are added by Rails and it's of no concern. + NEVER_UNPERMITTED_PARAMS = %w( controller action ) # Returns a new instance of <tt>ActionController::Parameters</tt>. # Also, sets the +permitted+ attribute to the default value of @@ -151,6 +185,21 @@ module ActionController # permitted.has_key?(:age) # => true # permitted.has_key?(:role) # => false # + # Only permitted scalars pass the filter. For example, given + # + # params.permit(:name) + # + # +:name+ passes it is a key of +params+ whose associated value is of type + # +String+, +Symbol+, +NilClass+, +Numeric+, +TrueClass+, +FalseClass+, + # +Date+, +Time+, +DateTime+, +StringIO+, +IO+, + # +ActionDispatch::Http::UploadedFile+ or +Rack::Test::UploadedFile+. + # Otherwise, the key +:name+ is filtered out. + # + # You may declare that the parameter should be an array of permitted scalars + # by mapping it to an empty array: + # + # params.permit(tags: []) + # # You can also use +permit+ on nested parameters, like: # # params = ActionController::Parameters.new({ @@ -197,32 +246,15 @@ module ActionController filters.flatten.each do |filter| case filter - when Symbol, String then - if has_key?(filter) - _value = self[filter] - params[filter] = _value unless Hash === _value - end - keys.grep(/\A#{Regexp.escape(filter)}\(\d+[if]?\)\z/) { |key| params[key] = self[key] } + when Symbol, String + permitted_scalar_filter(params, filter) when Hash then - filter = filter.with_indifferent_access - - self.slice(*filter.keys).each do |key, values| - return unless values - - key = key.to_sym - - params[key] = each_element(values) do |value| - # filters are a Hash, so we expect value to be a Hash too - next if filter.is_a?(Hash) && !value.is_a?(Hash) - - value = self.class.new(value) if !value.respond_to?(:permit) - - value.permit(*Array.wrap(filter[key])) - end - end + hash_filter(params, filter) end end + unpermitted_parameters!(params) if self.class.action_on_unpermitted_parameters + params.permit! end @@ -301,6 +333,100 @@ module ActionController yield object end end + + def unpermitted_parameters!(params) + unpermitted_keys = unpermitted_keys(params) + if unpermitted_keys.any? + case self.class.action_on_unpermitted_parameters + when :log + ActionController::Base.logger.debug "Unpermitted parameters: #{unpermitted_keys.join(", ")}" + when :raise + raise ActionController::UnpermittedParameters.new(unpermitted_keys) + end + end + end + + def unpermitted_keys(params) + self.keys - params.keys - NEVER_UNPERMITTED_PARAMS + end + + # + # --- Filtering ---------------------------------------------------------- + # + + # This is a white list of permitted scalar types that includes the ones + # supported in XML and JSON requests. + # + # This list is in particular used to filter ordinary requests, String goes + # as first element to quickly short-circuit the common case. + # + # If you modify this collection please update the API of +permit+ above. + PERMITTED_SCALAR_TYPES = [ + String, + Symbol, + NilClass, + Numeric, + TrueClass, + FalseClass, + Date, + Time, + # DateTimes are Dates, we document the type but avoid the redundant check. + StringIO, + IO, + ActionDispatch::Http::UploadedFile, + Rack::Test::UploadedFile, + ] + + def permitted_scalar?(value) + PERMITTED_SCALAR_TYPES.any? {|type| value.is_a?(type)} + end + + def permitted_scalar_filter(params, key) + if has_key?(key) && permitted_scalar?(self[key]) + params[key] = self[key] + end + + keys.grep(/\A#{Regexp.escape(key)}\(\d+[if]?\)\z/) do |k| + if permitted_scalar?(self[k]) + params[k] = self[k] + end + end + end + + def array_of_permitted_scalars?(value) + if value.is_a?(Array) + value.all? {|element| permitted_scalar?(element)} + end + end + + def array_of_permitted_scalars_filter(params, key) + if has_key?(key) && array_of_permitted_scalars?(self[key]) + params[key] = self[key] + end + end + + EMPTY_ARRAY = [] + def hash_filter(params, filter) + filter = filter.with_indifferent_access + + # Slicing filters out non-declared keys. + slice(*filter.keys).each do |key, value| + return unless value + + if filter[key] == EMPTY_ARRAY + # Declaration { comment_ids: [] }. + array_of_permitted_scalars_filter(params, key) + else + # Declaration { user: :name } or { user: [:name, :age, { adress: ... }] }. + params[key] = each_element(value) do |element| + if element.is_a?(Hash) + element = self.class.new(element) unless element.respond_to?(:permit) + element.permit(*Array.wrap(filter[key])) + end + end + end + end + end end # == Strong \Parameters @@ -329,7 +455,7 @@ module ActionController # # into a 400 Bad Request reply. # def update # redirect_to current_account.people.find(params[:id]).tap { |person| - # person.update_attributes!(person_params) + # person.update!(person_params) # } # end # @@ -374,12 +500,6 @@ module ActionController extend ActiveSupport::Concern include ActiveSupport::Rescuable - included do - rescue_from(ActionController::ParameterMissing) do |parameter_missing_exception| - render text: "Required parameter missing: #{parameter_missing_exception.param}", status: :bad_request - end - end - # Returns a new ActionController::Parameters object that # has been instantiated with the <tt>request.parameters</tt>. def params diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb index 3e44155f73..5379547c57 100644 --- a/actionpack/lib/action_controller/railtie.rb +++ b/actionpack/lib/action_controller/railtie.rb @@ -20,22 +20,27 @@ module ActionController end initializer "action_controller.parameters_config" do |app| - ActionController::Parameters.permit_all_parameters = app.config.action_controller.delete(:permit_all_parameters) { false } + options = app.config.action_controller + + ActionController::Parameters.permit_all_parameters = options.delete(:permit_all_parameters) { false } + ActionController::Parameters.action_on_unpermitted_parameters = options.delete(:action_on_unpermitted_parameters) do + (Rails.env.test? || Rails.env.development?) ? :log : false + end end initializer "action_controller.set_configs" do |app| paths = app.config.paths options = app.config.action_controller - options.logger ||= Rails.logger - options.cache_store ||= Rails.cache + options.logger ||= Rails.logger + options.cache_store ||= Rails.cache - options.javascripts_dir ||= paths["public/javascripts"].first - options.stylesheets_dir ||= paths["public/stylesheets"].first + options.javascripts_dir ||= paths["public/javascripts"].first + options.stylesheets_dir ||= paths["public/stylesheets"].first # Ensure readers methods get compiled - options.asset_host ||= app.config.asset_host - options.relative_url_root ||= app.config.relative_url_root + options.asset_host ||= app.config.asset_host + options.relative_url_root ||= app.config.relative_url_root ActiveSupport.on_load(:action_controller) do include app.routes.mounted_helpers diff --git a/actionpack/lib/action_controller/record_identifier.rb b/actionpack/lib/action_controller/record_identifier.rb index b49128c184..d598bac467 100644 --- a/actionpack/lib/action_controller/record_identifier.rb +++ b/actionpack/lib/action_controller/record_identifier.rb @@ -1,19 +1,30 @@ -require 'active_support/deprecation' require 'action_view/record_identifier' module ActionController module RecordIdentifier - MESSAGE = 'method will no longer be included by default in controllers since Rails 4.1. ' + - 'If you would like to use it in controllers, please include ' + - 'ActionView::RecodIdentifier module.' + MODULE_MESSAGE = 'Calling ActionController::RecordIdentifier.%s is deprecated and ' \ + 'will be removed in Rails 4.1, please call using ActionView::RecordIdentifier instead.' + INSTANCE_MESSAGE = '%s method will no longer be included by default in controllers ' \ + 'since Rails 4.1. If you would like to use it in controllers, please include ' \ + 'ActionView::RecordIdentifier module.' def dom_id(record, prefix = nil) - ActiveSupport::Deprecation.warn('dom_id ' + MESSAGE) + ActiveSupport::Deprecation.warn(INSTANCE_MESSAGE % 'dom_id') ActionView::RecordIdentifier.dom_id(record, prefix) end def dom_class(record, prefix = nil) - ActiveSupport::Deprecation.warn('dom_class ' + MESSAGE) + ActiveSupport::Deprecation.warn(INSTANCE_MESSAGE % 'dom_class') + ActionView::RecordIdentifier.dom_class(record, prefix) + end + + def self.dom_id(record, prefix = nil) + ActiveSupport::Deprecation.warn(MODULE_MESSAGE % 'dom_id') + ActionView::RecordIdentifier.dom_id(record, prefix) + end + + def self.dom_class(record, prefix = nil) + ActiveSupport::Deprecation.warn(MODULE_MESSAGE % 'dom_class') ActionView::RecordIdentifier.dom_class(record, prefix) end end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 331d15d403..e12bf0a1c6 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -1,6 +1,7 @@ require 'rack/session/abstract/id' require 'active_support/core_ext/object/to_query' require 'active_support/core_ext/module/anonymous' +require 'active_support/core_ext/hash/keys' module ActionController module TemplateAssertions @@ -15,6 +16,7 @@ module ActionController @_partials = Hash.new(0) @_templates = Hash.new(0) @_layouts = Hash.new(0) + @_files = Hash.new(0) ActiveSupport::Notifications.subscribe("render_template.action_view") do |name, start, finish, id, payload| path = payload[:layout] @@ -38,6 +40,16 @@ module ActionController @_templates[path] += 1 end + + ActiveSupport::Notifications.subscribe("!render_template.action_view") do |name, start, finish, id, payload| + path = payload[:identifier] + next if payload[:virtual_path] # files don't have virtual path + if path + @_files[path] += 1 + @_files[path.split("/").last] += 1 + end + + end end def teardown_subscriptions @@ -81,8 +93,7 @@ module ActionController # # assert that the "_customer" partial was rendered with a specific object # assert_template partial: '_customer', locals: { customer: @customer } def assert_template(options = {}, message = nil) - # Force body to be read in case the - # template is being streamed + # Force body to be read in case the template is being streamed. response.body case options @@ -106,7 +117,7 @@ module ActionController end assert matches_template, msg when Hash - options.assert_valid_keys(:layout, :partial, :locals, :count) + options.assert_valid_keys(:layout, :partial, :locals, :count, :file) if options.key?(:layout) expected_layout = options[:layout] @@ -123,10 +134,18 @@ module ActionController end end + if options[:file] + assert_includes @_files.keys, options[:file] + end + if expected_partial = options[:partial] if expected_locals = options[:locals] if defined?(@_rendered_views) - view = expected_partial.to_s.sub(/^_/,'') + view = expected_partial.to_s.sub(/^_/, '').sub(/\/_(?=[^\/]+\z)/, '/') + + partial_was_not_rendered_msg = "expected %s to be rendered but it was not." % view + assert_includes @_rendered_views.rendered_views, view, partial_was_not_rendered_msg + msg = 'expecting %s to be rendered with %s but was with %s' % [expected_partial, expected_locals, @_rendered_views.locals_for(view)] @@ -236,18 +255,39 @@ module ActionController end end + # Methods #destroy and #load! are overridden to avoid calling methods on the + # @store object, which does not exist for the TestSession class. class TestSession < Rack::Session::Abstract::SessionHash #:nodoc: DEFAULT_OPTIONS = Rack::Session::Abstract::ID::DEFAULT_OPTIONS def initialize(session = {}) super(nil, nil) - replace(session.stringify_keys) + @id = SecureRandom.hex(16) + @data = stringify_keys(session) @loaded = true end def exists? true end + + def keys + @data.keys + end + + def values + @data.values + end + + def destroy + clear + end + + private + + def load! + @id + end end # Superclass for ActionController functional tests. Functional tests allow you to diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index b35761fb4a..618e2f3033 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -47,7 +47,6 @@ module ActionDispatch autoload_under 'middleware' do autoload :RequestId - autoload :BestStandardsSupport autoload :Callbacks autoload :Cookies autoload :DebugExceptions @@ -97,7 +96,6 @@ module ActionDispatch autoload :Assertions autoload :Integration autoload :IntegrationTest, 'action_dispatch/testing/integration' - autoload :PerformanceTest autoload :TestProcess autoload :TestRequest autoload :TestResponse diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb index 02ab49b44e..289e204ac8 100644 --- a/actionpack/lib/action_dispatch/http/filter_parameters.rb +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/hash/keys' require 'active_support/core_ext/object/duplicable' +require 'action_dispatch/http/parameter_filter' module ActionDispatch module Http diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index 57660e93c4..40bb060d52 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -121,8 +121,8 @@ module ActionDispatch BROWSER_LIKE_ACCEPTS = /,\s*\*\/\*|\*\/\*\s*,/ def valid_accept_header - (xhr? && (accept || content_mime_type)) || - (accept && accept !~ BROWSER_LIKE_ACCEPTS) + (xhr? && (accept.present? || content_mime_type)) || + (accept.present? && accept !~ BROWSER_LIKE_ACCEPTS) end def use_accept_header diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index 6610315da7..446862aad0 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -55,7 +55,7 @@ module ActionDispatch # should really prevent that from happening def encode_params(params) if params.is_a?(String) - return params.force_encoding("UTF-8").encode! + return params.force_encoding(Encoding::UTF_8).encode! elsif !params.is_a?(Hash) return params end @@ -72,7 +72,7 @@ module ActionDispatch end end - # Convert nested Hash to HashWithIndifferentAccess + # Convert nested Hash to ActiveSupport::HashWithIndifferentAccess def normalize_parameters(value) case value when Hash diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index d60c8775af..7b04d6e851 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -1,12 +1,16 @@ -require 'tempfile' require 'stringio' -require 'strscan' -require 'active_support/core_ext/hash/indifferent_access' -require 'active_support/core_ext/string/access' require 'active_support/inflector' require 'action_dispatch/http/headers' require 'action_controller/metal/exceptions' +require 'rack/request' +require 'action_dispatch/http/cache' +require 'action_dispatch/http/mime_negotiation' +require 'action_dispatch/http/parameters' +require 'action_dispatch/http/filter_parameters' +require 'action_dispatch/http/upload' +require 'action_dispatch/http/url' +require 'active_support/core_ext/array/conversions' module ActionDispatch class Request < Rack::Request @@ -280,15 +284,14 @@ module ActionDispatch LOCALHOST =~ remote_addr && LOCALHOST =~ remote_ip end - protected - # Remove nils from the params hash def deep_munge(hash) - hash.each_value do |v| + hash.each do |k, v| case v when Array v.grep(Hash) { |x| deep_munge(x) } v.compact! + hash[k] = nil if v.empty? when Hash deep_munge(v) end @@ -297,6 +300,8 @@ module ActionDispatch hash end + protected + def parse_query(qs) deep_munge(super) end diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 91cf4784db..06e936cdb0 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -1,4 +1,3 @@ -require 'digest/md5' require 'active_support/core_ext/class/attribute_accessors' require 'monitor' @@ -137,6 +136,7 @@ module ActionDispatch # :nodoc: @committed end + # Sets the HTTP status code. def status=(status) @status = Rack::Utils.status_code(status) end @@ -145,23 +145,31 @@ module ActionDispatch # :nodoc: @content_type = content_type.to_s end - # The response code of the request + # The response code of the request. def response_code @status end - # Returns a String to ensure compatibility with Net::HTTPResponse + # Returns a string to ensure compatibility with <tt>Net::HTTPResponse</tt>. def code @status.to_s end + # Returns the corresponding message for the current HTTP status code: + # + # response.status = 200 + # response.message # => "OK" + # + # response.status = 404 + # response.message # => "Not Found" + # def message Rack::Utils::HTTP_STATUS_CODES[@status] end alias_method :status_message, :message def respond_to?(method) - if method.to_sym == :to_path + if method.to_s == 'to_path' stream.respond_to?(:to_path) else super @@ -172,6 +180,8 @@ module ActionDispatch # :nodoc: stream.to_path end + # Returns the content of the response as a string. This contains the contents + # of any calls to <tt>render</tt>. def body strings = [] each { |part| strings << part.to_s } @@ -180,6 +190,7 @@ module ActionDispatch # :nodoc: EMPTY = " " + # Allows you to manually set or override the response body. def body=(body) @blank = true if body == EMPTY diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb index 79437d6e85..67cb7fbcb5 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -19,7 +19,7 @@ module ActionDispatch # its interface is available directly. attr_accessor :tempfile - # TODO. + # A string with the headers of the multipart request. attr_accessor :headers def initialize(hash) # :nodoc: @@ -70,12 +70,12 @@ module ActionDispatch def encode_filename(filename) # Encode the filename in the utf8 encoding, unless it is nil - filename.force_encoding("UTF-8").encode! if filename + filename.force_encoding(Encoding::UTF_8).encode! if filename end end module Upload # :nodoc: - # Convert nested Hash to HashWithIndifferentAccess and replace + # Convert nested Hash to ActiveSupport::HashWithIndifferentAccess and replace # file upload hash with UploadedFile objects def normalize_parameters(value) if Hash === value && value.has_key?(:tempfile) diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index bced7d84c0..97ac462411 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -1,3 +1,6 @@ +require 'active_support/core_ext/module/attribute_accessors' +require 'active_support/core_ext/hash/slice' + module ActionDispatch module Http module URL @@ -32,7 +35,15 @@ module ActionDispatch params.reject! { |_,v| v.to_param.nil? } result = build_host_url(options) - result << (options[:trailing_slash] ? path.sub(/\?|\z/) { "/" + $& } : path) + if options[:trailing_slash] + if path.include?('?') + result << path.sub(/\?/, '/\&') + else + result << path.sub(/[^\/]\z|\A\z/, '\&/') + end + else + result << path + end result << "?#{params.to_query}" unless params.empty? result << "##{Journey::Router::Utils.escape_fragment(options[:anchor].to_param.to_s)}" if options[:anchor] result diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb index 4a344f71af..82c55660ea 100644 --- a/actionpack/lib/action_dispatch/journey/formatter.rb +++ b/actionpack/lib/action_dispatch/journey/formatter.rb @@ -1,3 +1,5 @@ +require 'action_controller/metal/exceptions' + module ActionDispatch module Journey # The Formatter class is used for formatting URLs. For example, parameters @@ -27,7 +29,10 @@ module ActionDispatch return [route.format(parameterized_parts), params] end - raise Router::RoutingError.new "missing required keys: #{missing_keys}" + message = "No route matches #{constraints.inspect}" + message << " missing required keys: #{missing_keys.inspect}" if name + + raise ActionController::UrlGenerationError, message end def clear @@ -37,19 +42,16 @@ module ActionDispatch private def extract_parameterized_parts(route, options, recall, parameterize = nil) - constraints = recall.merge(options) - data = constraints.dup + parameterized_parts = recall.merge(options) keys_to_keep = route.parts.reverse.drop_while { |part| !options.key?(part) || (options[part] || recall[part]).nil? } | route.required_parts - (data.keys - keys_to_keep).each do |bad_key| - data.delete(bad_key) + (parameterized_parts.keys - keys_to_keep).each do |bad_key| + parameterized_parts.delete(bad_key) end - parameterized_parts = data.dup - if parameterize parameterized_parts.each do |k, v| parameterized_parts[k] = parameterize.call(k, v) diff --git a/actionpack/lib/action_dispatch/journey/path/pattern.rb b/actionpack/lib/action_dispatch/journey/path/pattern.rb index 4a571ec546..d37aa1fbe5 100644 --- a/actionpack/lib/action_dispatch/journey/path/pattern.rb +++ b/actionpack/lib/action_dispatch/journey/path/pattern.rb @@ -20,7 +20,7 @@ module ActionDispatch @separators = strexp.separators.join @anchored = strexp.anchor else - raise "wtf bro: #{strexp}" + raise ArgumentError, "Bad expression: #{strexp}" end @names = nil diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index d18efd863a..6fda085681 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -1,7 +1,7 @@ module ActionDispatch module Journey # :nodoc: class Route # :nodoc: - attr_reader :app, :path, :verb, :defaults, :ip, :name + attr_reader :app, :path, :defaults, :name attr_reader :constraints alias :conditions :constraints @@ -12,15 +12,11 @@ module ActionDispatch # +path+ is a path constraint. # +constraints+ is a hash of constraints to be applied to this route. def initialize(name, app, path, constraints, defaults = {}) - constraints = constraints.dup @name = name @app = app @path = path - @verb = constraints[:request_method] || // - @ip = constraints.delete(:ip) || // @constraints = constraints - @constraints.keep_if { |_,v| Regexp === v || String === v } @defaults = defaults @required_defaults = nil @required_parts = nil @@ -30,11 +26,11 @@ module ActionDispatch end def ast - return @decorated_ast if @decorated_ast - - @decorated_ast = path.ast - @decorated_ast.grep(Nodes::Terminal).each { |n| n.memo = self } - @decorated_ast + @decorated_ast ||= begin + decorated_ast = path.ast + decorated_ast.grep(Nodes::Terminal).each { |n| n.memo = self } + decorated_ast + end end def requirements # :nodoc: @@ -45,11 +41,11 @@ module ActionDispatch end def segments - @path.names + path.names end def required_keys - path.required_names.map { |x| x.to_sym } + required_defaults.keys + required_parts + required_defaults.keys end def score(constraints) @@ -75,6 +71,10 @@ module ActionDispatch Visitors::Formatter.new(path_options).accept(path.spec) end + def optimized_path + Visitors::OptimizedPath.new.accept(path.spec) + end + def optional_parts path.optional_names.map { |n| n.to_sym } end @@ -83,12 +83,38 @@ module ActionDispatch @required_parts ||= path.required_names.map { |n| n.to_sym } end + def required_default?(key) + (constraints[:required_defaults] || []).include?(key) + end + def required_defaults - @required_defaults ||= begin - matches = parts - @defaults.dup.delete_if { |k,_| matches.include?(k) } + @required_defaults ||= @defaults.dup.delete_if do |k,_| + parts.include?(k) || !required_default?(k) end end + + def matches?(request) + constraints.all? do |method, value| + next true unless request.respond_to?(method) + + case value + when Regexp, String + value === request.send(method).to_s + when Array + value.include?(request.send(method)) + else + value === request.send(method) + end + end + end + + def ip + constraints[:ip] || // + end + + def verb + constraints[:request_method] || // + end end end end diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index 1fc45a2109..31868b1814 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -131,11 +131,7 @@ module ActionDispatch } routes.concat get_routes_as_head(routes) - routes.sort_by!(&:precedence).select! { |r| - r.constraints.all? { |k, v| v === req.send(k) } && - r.verb === req.request_method - } - routes.reject! { |r| req.ip && !(r.ip === req.ip) } + routes.sort_by!(&:precedence).select! { |r| r.matches?(req) } routes.map! { |r| match_data = r.path.match(req.path_info) diff --git a/actionpack/lib/action_dispatch/journey/routes.rb b/actionpack/lib/action_dispatch/journey/routes.rb index 32829a1f20..a99d6d0d6a 100644 --- a/actionpack/lib/action_dispatch/journey/routes.rb +++ b/actionpack/lib/action_dispatch/journey/routes.rb @@ -16,12 +16,12 @@ module ActionDispatch end def length - @routes.length + routes.length end alias :size :length def last - @routes.last + routes.last end def each(&block) @@ -33,24 +33,23 @@ module ActionDispatch end def partitioned_routes - @partitioned_routes ||= routes.partition { |r| - r.path.anchored && r.ast.grep(Nodes::Symbol).all? { |n| n.default_regexp? } - } + @partitioned_routes ||= routes.partition do |r| + r.path.anchored && r.ast.grep(Nodes::Symbol).all?(&:default_regexp?) + end end def ast - return @ast if @ast - return if partitioned_routes.first.empty? - - asts = partitioned_routes.first.map { |r| r.ast } - @ast = Nodes::Or.new(asts) + @ast ||= begin + asts = partitioned_routes.first.map(&:ast) + Nodes::Or.new(asts) unless asts.empty? + end end def simulator - return @simulator if @simulator - - gtg = GTG::Builder.new(ast).transition_table - @simulator = GTG::Simulator.new(gtg) + @simulator ||= begin + gtg = GTG::Builder.new(ast).transition_table + GTG::Simulator.new(gtg) + end end # Add a route to the routing table. diff --git a/actionpack/lib/action_dispatch/journey/visitors.rb b/actionpack/lib/action_dispatch/journey/visitors.rb index 46bd58c178..2964d80d9f 100644 --- a/actionpack/lib/action_dispatch/journey/visitors.rb +++ b/actionpack/lib/action_dispatch/journey/visitors.rb @@ -74,6 +74,14 @@ module ActionDispatch end end + class OptimizedPath < String # :nodoc: + private + + def visit_GROUP(node) + "" + end + end + # Used for formatting urls (url_for) class Formatter < Visitor # :nodoc: attr_reader :options, :consumed diff --git a/actionpack/lib/action_dispatch/middleware/best_standards_support.rb b/actionpack/lib/action_dispatch/middleware/best_standards_support.rb deleted file mode 100644 index d338996240..0000000000 --- a/actionpack/lib/action_dispatch/middleware/best_standards_support.rb +++ /dev/null @@ -1,28 +0,0 @@ -module ActionDispatch - class BestStandardsSupport - def initialize(app, type = true) - @app = app - - @header = case type - when true - "IE=Edge,chrome=1" - when :builtin - "IE=Edge" - when false - nil - end - end - - def call(env) - status, headers, body = @app.call(env) - - if headers["X-UA-Compatible"] && @header - headers["X-UA-Compatible"] << "," << @header.to_s - else - headers["X-UA-Compatible"] = @header - end - - [status, headers, body] - end - end -end diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 121a11c8e1..36a0db6e61 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/hash/keys' require 'active_support/core_ext/module/attribute_accessors' +require 'active_support/key_generator' require 'active_support/message_verifier' module ActionDispatch @@ -15,7 +16,7 @@ module ActionDispatch # being written will be sent out with the response. Reading a cookie does not get # the cookie object itself back, just the value it holds. # - # Examples for writing: + # Examples of writing: # # # Sets a simple session cookie. # # This cookie will be deleted when the user's browser is closed. @@ -38,7 +39,7 @@ module ActionDispatch # # You can also chain these methods: # cookies.permanent.signed[:login] = "XJ-122" # - # Examples for reading: + # Examples of reading: # # cookies[:user_name] # => "david" # cookies.size # => 2 @@ -110,13 +111,17 @@ 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] || '', + token_key: env[TOKEN_KEY] } + end + def self.build(request) env = request.env key_generator = env[GENERATOR_KEY] - options = { signed_cookie_salt: env[SIGNED_COOKIE_SALT], - encrypted_cookie_salt: env[ENCRYPTED_COOKIE_SALT], - encrypted_signed_cookie_salt: env[ENCRYPTED_SIGNED_COOKIE_SALT], - token_key: env[TOKEN_KEY] } + options = options_for_env env host = request.host secure = request.ssl? @@ -145,6 +150,10 @@ module ActionDispatch @cookies[name.to_s] end + def fetch(name, *args, &block) + @cookies.fetch(name.to_s, *args, &block) + end + def key?(name) @cookies.key?(name.to_s) end @@ -196,7 +205,7 @@ module ActionDispatch end # Removes the cookie on the client machine by setting the value to an empty string - # and setting its expiration date into the past. Like <tt>[]=</tt>, you can pass in + # and the expiration date in the past. Like <tt>[]=</tt>, you can pass in # an options hash to delete cookies with extra data such as a <tt>:path</tt>. def delete(key, options = {}) return unless @cookies.has_key? key.to_s @@ -401,7 +410,7 @@ module ActionDispatch @encryptor.decrypt_and_verify(encrypted_message) end rescue ActiveSupport::MessageVerifier::InvalidSignature, - ActiveSupport::MessageVerifier::InvalidMessage + ActiveSupport::MessageEncryptor::InvalidMessage nil end diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 6705e531cb..64230ff1ae 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -2,7 +2,6 @@ require 'action_dispatch/http/request' require 'action_dispatch/middleware/exception_wrapper' require 'action_dispatch/routing/inspector' - module ActionDispatch # This middleware is responsible for logging exceptions and # showing a debugging page in case the request is local. @@ -15,18 +14,17 @@ module ActionDispatch end def call(env) - begin - response = (_, headers, body = @app.call(env)) - - if headers['X-Cascade'] == 'pass' - body.close if body.respond_to?(:close) - raise ActionController::RoutingError, "No route matches [#{env['REQUEST_METHOD']}] #{env['PATH_INFO'].inspect}" - end - rescue Exception => exception - raise exception if env['action_dispatch.show_exceptions'] == false + _, headers, body = response = @app.call(env) + + if headers['X-Cascade'] == 'pass' + body.close if body.respond_to?(:close) + raise ActionController::RoutingError, "No route matches [#{env['REQUEST_METHOD']}] #{env['PATH_INFO'].inspect}" end - exception ? render_exception(env, exception) : response + response + rescue Exception => exception + raise exception if env['action_dispatch.show_exceptions'] == false + render_exception(env, exception) end private @@ -42,9 +40,11 @@ module ActionDispatch :application_trace => wrapper.application_trace, :framework_trace => wrapper.framework_trace, :full_trace => wrapper.full_trace, - :routes => formatted_routes(exception) + :routes_inspector => routes_inspector(exception), + :source_extract => wrapper.source_extract, + :line_number => wrapper.line_number, + :file => wrapper.file ) - file = "rescues/#{wrapper.rescue_template}" body = template.render(:template => file, :layout => 'rescues/layout') render(wrapper.status_code, body) @@ -82,11 +82,9 @@ module ActionDispatch @stderr_logger ||= ActiveSupport::Logger.new($stderr) end - def formatted_routes(exception) - return false unless @routes_app.respond_to?(:routes) - if exception.is_a?(ActionController::RoutingError) || exception.is_a?(ActionView::Template::Error) - inspector = ActionDispatch::Routing::RoutesInspector.new - inspector.collect_routes(@routes_app.routes.routes) + def routes_inspector(exception) + if @routes_app.respond_to?(:routes) && (exception.is_a?(ActionController::RoutingError) || exception.is_a?(ActionView::Template::Error)) + ActionDispatch::Routing::RoutesInspector.new(@routes_app.routes.routes) end end end diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index ae38c56a67..7489ce8028 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -1,5 +1,4 @@ require 'action_controller/metal/exceptions' -require 'active_support/core_ext/exception' require 'active_support/core_ext/class/attribute_accessors' module ActionDispatch @@ -13,7 +12,8 @@ module ActionDispatch 'ActionController::NotImplemented' => :not_implemented, 'ActionController::UnknownFormat' => :not_acceptable, 'ActionController::InvalidAuthenticityToken' => :unprocessable_entity, - 'ActionController::BadRequest' => :bad_request + 'ActionController::BadRequest' => :bad_request, + 'ActionController::ParameterMissing' => :bad_request ) cattr_accessor :rescue_templates @@ -25,7 +25,7 @@ module ActionDispatch 'ActionView::Template::Error' => 'template_error' ) - attr_reader :env, :exception + attr_reader :env, :exception, :line_number, :file def initialize(env, exception) @env = env @@ -56,6 +56,15 @@ module ActionDispatch Rack::Utils.status_code(@@rescue_responses[class_name]) end + def source_extract + if application_trace && trace = application_trace.first + file, line, _ = trace.split(":") + @file = file + @line_number = line.to_i + source_fragment(@file, @line_number) + end + end + private def original_exception(exception) @@ -81,5 +90,17 @@ module ActionDispatch 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) + if File.exists?(full_path) + File.open(full_path, "r") do |file| + start = [line - 3, 0].max + lines = file.each_line.drop(start).take(6) + Hash[*(start+1..(lines.count+start)).zip(lines).flatten] + end + end + end end end diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index f24e9b8e18..7e56feb90a 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -59,12 +59,12 @@ module ActionDispatch @flash[k] end - # Convenience accessor for flash.now[:alert]= + # Convenience accessor for <tt>flash.now[:alert]=</tt>. def alert=(message) self[:alert] = message end - # Convenience accessor for flash.now[:notice]= + # Convenience accessor for <tt>flash.now[:notice]=</tt>. def notice=(message) self[:notice] = message end @@ -82,7 +82,7 @@ module ActionDispatch else new end - + flash.tap(&:sweep) end @@ -169,6 +169,14 @@ module ActionDispatch # vanish when the current action is done. # # Entries set via <tt>now</tt> are accessed the same way as standard entries: <tt>flash['my-key']</tt>. + # + # Also, brings two convenience accessors: + # + # flash.now.alert = "Beware now!" + # # Equivalent to flash.now[:alert] = "Beware now!" + # + # flash.now.notice = "Good luck now!" + # # Equivalent to flash.now[:notice] = "Good luck now!" def now @now ||= FlashNow.new(self) end @@ -199,22 +207,22 @@ module ActionDispatch @discard.replace @flashes.keys end - # Convenience accessor for flash[:alert] + # Convenience accessor for <tt>flash[:alert]</tt>. def alert self[:alert] end - # Convenience accessor for flash[:alert]= + # Convenience accessor for <tt>flash[:alert]=</tt>. def alert=(message) self[:alert] = message end - # Convenience accessor for flash[:notice] + # Convenience accessor for <tt>flash[:notice]</tt>. def notice self[:notice] end - # Convenience accessor for flash[:notice]= + # Convenience accessor for <tt>flash[:notice]=</tt>. def notice=(message) self[:notice] = message end diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index 2c98ca03a8..0fa1e9b859 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -13,10 +13,7 @@ module ActionDispatch end end - DEFAULT_PARSERS = { - Mime::XML => :xml_simple, - Mime::JSON => :json - } + DEFAULT_PARSERS = { Mime::JSON => :json } def initialize(app, parsers = {}) @app, @parsers = app, DEFAULT_PARSERS.merge(parsers) @@ -36,45 +33,26 @@ module ActionDispatch return false if request.content_length.zero? - mime_type = content_type_from_legacy_post_data_format_header(env) || - request.content_mime_type - - strategy = @parsers[mime_type] + strategy = @parsers[request.content_mime_type] return false unless strategy case strategy when Proc strategy.call(request.raw_post) - when :xml_simple, :xml_node - data = Hash.from_xml(request.raw_post) || {} - data.with_indifferent_access - when :yaml - YAML.load(request.raw_post) when :json - data = ActiveSupport::JSON.decode(request.raw_post) + data = ActiveSupport::JSON.decode(request.body) data = {:_json => data} unless data.is_a?(Hash) - data.with_indifferent_access + request.deep_munge(data).with_indifferent_access else false end - rescue Exception => e # YAML, XML or Ruby code block errors + rescue Exception => e # JSON or Ruby code block errors logger(env).debug "Error occurred while parsing request parameters.\nContents:\n\n#{request.raw_post}" raise ParseError.new(e.message, e) end - def content_type_from_legacy_post_data_format_header(env) - if x_post_format = env['HTTP_X_POST_DATA_FORMAT'] - case x_post_format.to_s.downcase - when 'yaml' then return Mime::YAML - when 'xml' then return Mime::XML - end - end - - nil - end - def logger(env) env['action_dispatch.logger'] || ActiveSupport::Logger.new($stderr) end diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 5abf8f2802..93a2b52996 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -1,23 +1,59 @@ module ActionDispatch + # This middleware calculates the IP address of the remote client that is + # making the request. It does this by checking various headers that could + # contain the address, and then picking the last-set address that is not + # on the list of trusted IPs. This follows the precedent set by e.g. + # {the Tomcat server}[https://issues.apache.org/bugzilla/show_bug.cgi?id=50453], + # with {reasoning explained at length}[http://blog.gingerlime.com/2012/rails-ip-spoofing-vulnerabilities-and-protection] + # by @gingerlime. A more detailed explanation of the algorithm is given + # at GetIp#calculate_ip. + # + # Some Rack servers concatenate repeated headers, like {HTTP RFC 2616}[http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2] + # requires. Some Rack servers simply drop preceding headers, and only report + # the value that was {given in the last header}[http://andre.arko.net/2011/12/26/repeated-headers-and-ruby-web-servers]. + # If you are behind multiple proxy servers (like Nginx to HAProxy to Unicorn) + # then you should test your Rack server to make sure your data is good. + # + # IF YOU DON'T USE A PROXY, THIS MAKES YOU VULNERABLE TO IP SPOOFING. + # This middleware assumes that there is at least one proxy sitting around + # and setting headers with the client's remote IP address. If you don't use + # a proxy, because you are hosted on e.g. Heroku without SSL, any client can + # claim to have any IP address by setting the X-Forwarded-For header. If you + # care about that, then you need to explicitly drop or ignore those headers + # sometime before this middleware runs. class RemoteIp - class IpSpoofAttackError < StandardError ; end + class IpSpoofAttackError < StandardError; end - # IP addresses that are "trusted proxies" that can be stripped from - # the comma-delimited list in the X-Forwarded-For header. See also: - # http://en.wikipedia.org/wiki/Private_network#Private_IPv4_address_spaces - # http://en.wikipedia.org/wiki/Private_network#Private_IPv6_addresses. + # The default trusted IPs list simply includes IP addresses that are + # guaranteed by the IP specification to be private addresses. Those will + # not be the ultimate client IP in production, and so are discarded. See + # http://en.wikipedia.org/wiki/Private_network for details. TRUSTED_PROXIES = %r{ - ^127\.0\.0\.1$ | # localhost - ^::1$ | - ^(10 | # private IP 10.x.x.x - 172\.(1[6-9]|2[0-9]|3[0-1]) | # private IP in the range 172.16.0.0 .. 172.31.255.255 - 192\.168 | # private IP 192.168.x.x - fc00:: # private IP fc00 - )\. + ^127\.0\.0\.1$ | # localhost IPv4 + ^::1$ | # localhost IPv6 + ^fc00: | # private IPv6 range fc00 + ^10\. | # private IPv4 range 10.x.x.x + ^172\.(1[6-9]|2[0-9]|3[0-1])\.| # private IPv4 range 172.16.0.0 .. 172.31.255.255 + ^192\.168\. # private IPv4 range 192.168.x.x }x attr_reader :check_ip, :proxies + # Create a new +RemoteIp+ middleware instance. + # + # The +check_ip_spoofing+ option is on by default. When on, an exception + # is raised if it looks like the client is trying to lie about its own IP + # address. It makes sense to turn off this check on sites aimed at non-IP + # clients (like WAP devices), or behind proxies that set headers in an + # incorrect or confusing way (like AWS ELB). + # + # The +custom_trusted+ argument can take a regex, which will be used + # instead of +TRUSTED_PROXIES+, or a string, which will be used in addition + # to +TRUSTED_PROXIES+. Any proxy setup will put the value you want in the + # middle (or at the beginning) of the X-Forwarded-For list, with your proxy + # servers after it. If your proxies aren't removed, pass them in via the + # +custom_trusted+ parameter. That way, the middleware will ignore those + # IP addresses, and return the one that you want. def initialize(app, check_ip_spoofing = true, custom_proxies = nil) @app = app @check_ip = check_ip_spoofing @@ -31,15 +67,23 @@ module ActionDispatch end end + # Since the IP address may not be needed, we store the object here + # without calculating the IP to keep from slowing down the majority of + # 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, self) @app.call(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 - # IP v4 and v6 (with compression) validation regexp - # https://gist.github.com/1289635 + # This constant contains a regular expression that validates every known + # form of IP v4 and v6 address, with or without abbreviations, adapted + # from {this gist}[https://gist.github.com/gazay/1289635]. VALID_IP = %r{ (^(25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[0-9]{1,2})(\.(25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[0-9]{1,2})){3}$) | # ip v4 (^( @@ -63,62 +107,78 @@ module ActionDispatch }x def initialize(env, middleware) - @env = env - @middleware = middleware - @ip = nil + @env = env + @check_ip = middleware.check_ip + @proxies = middleware.proxies end - # Determines originating IP address. REMOTE_ADDR is the standard - # but will be wrong if the user is behind a proxy. Proxies will set - # HTTP_CLIENT_IP and/or HTTP_X_FORWARDED_FOR, so we prioritize those. - # HTTP_X_FORWARDED_FOR may be a comma-delimited list in the case of - # multiple chained proxies. The first address which is in this list - # if it's not a known proxy will be the originating IP. - # Format of HTTP_X_FORWARDED_FOR: - # client_ip, proxy_ip1, proxy_ip2... - # http://en.wikipedia.org/wiki/X-Forwarded-For + # Sort through the various IP address headers, looking for the IP most + # likely to be the address of the actual remote client making this + # request. + # + # REMOTE_ADDR will be correct if the request is made directly against the + # Ruby process, on e.g. Heroku. When the request is proxied by another + # server like HAProxy or Nginx, the IP address that made the original + # request will be put in an X-Forwarded-For header. If there are multiple + # proxies, that header may contain a list of IPs. Other proxy services + # set the Client-Ip header instead, so we check that too. + # + # As discussed in {this post about Rails IP Spoofing}[http://blog.gingerlime.com/2012/rails-ip-spoofing-vulnerabilities-and-protection/], + # while the first IP in the list is likely to be the "originating" IP, + # it could also have been set by the client maliciously. + # + # In order to find the first address that is (probably) accurate, we + # take the list of IPs, remove known and trusted proxies, and then take + # the last address left, which was presumably set by one of those proxies. def calculate_ip - client_ip = @env['HTTP_CLIENT_IP'] - forwarded_ip = ips_from('HTTP_X_FORWARDED_FOR').first - remote_addrs = ips_from('REMOTE_ADDR') - - check_ip = client_ip && @middleware.check_ip - if check_ip && forwarded_ip != client_ip + # Set by the Rack web server, this is a single value. + remote_addr = ips_from('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-Ip+ and +X-Forwarded-For+ should not, generally, both be set. + # If they are both set, it means that this request passed through two + # proxies with incompatible IP header conventions, and there is no way + # for us to determine which header is the right one after the fact. + # Since we have no idea, we give up and explode. + should_check_ip = @check_ip && client_ips.last + 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}" \ + raise IpSpoofAttackError, "IP spoofing attack?! " + + "HTTP_CLIENT_IP=#{@env['HTTP_CLIENT_IP'].inspect} " + "HTTP_X_FORWARDED_FOR=#{@env['HTTP_X_FORWARDED_FOR'].inspect}" end - client_ips = remove_proxies [client_ip, forwarded_ip, remote_addrs].flatten - if client_ips.present? - client_ips.first - else - # If there is no client ip we can return first valid proxy ip from REMOTE_ADDR - remote_addrs.find { |ip| valid_ip? ip } - end + # We assume these things about the IP headers: + # + # - X-Forwarded-For will be a list of IPs, one per proxy, or blank + # - Client-Ip is propagated from the outermost proxy, or is blank + # - REMOTE_ADDR will be the IP that made the request to Rack + ips = [forwarded_ips, client_ips, remote_addr].flatten.compact + + # If every single IP option is in the trusted list, just return REMOTE_ADDR + filter_proxies(ips).first || remote_addr end + # Memoizes the value returned by #calculate_ip and returns it for + # ActionDispatch::Request to use. def to_s @ip ||= calculate_ip end - private + protected def ips_from(header) - @env[header] ? @env[header].strip.split(/[,\s]+/) : [] - end - - def valid_ip?(ip) - ip =~ VALID_IP - end - - def not_a_proxy?(ip) - ip !~ @middleware.proxies + # Split the comma-separated list into an array of strings + ips = @env[header] ? @env[header].strip.split(/[,\s]+/) : [] + # Only return IPs that are valid according to the regex + ips.select{ |ip| ip =~ VALID_IP } end - def remove_proxies(ips) - ips.select { |ip| valid_ip?(ip) && not_a_proxy?(ip) } + def filter_proxies(ips) + ips.reject { |ip| ip =~ @proxies } end end diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index 7c12590c49..84df55fd5a 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -26,7 +26,7 @@ module ActionDispatch def generate_sid sid = SecureRandom.hex(16) - sid.encode!('UTF-8') + sid.encode!(Encoding::UTF_8) sid end diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index ce5f89ee5b..1e6ed624b0 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -21,15 +21,12 @@ module ActionDispatch # # Session options: # - # * <tt>:secret</tt>: An application-wide key string or block returning a - # string called per generated digest. The block is called with the - # CGI::Session instance as an argument. It's important that the secret - # is not vulnerable to a dictionary attack. Therefore, you should choose - # a secret consisting of random numbers and letters and more than 30 - # characters. + # * <tt>:secret</tt>: An application-wide key string. It's important that + # the secret is not vulnerable to a dictionary attack. Therefore, you + # should choose a secret consisting of random numbers and letters and + # more than 30 characters. # # secret: '449fe2e7daee471bffae2fd8dc02313d' - # secret: Proc.new { User.current_user.secret_key } # # * <tt>:digest</tt>: The message digest algorithm used to verify session # integrity defaults to 'SHA1' but may be any digest provided by OpenSSL, @@ -39,21 +36,38 @@ module ActionDispatch # "rake secret" and set the key in config/initializers/secret_token.rb. # # Note that changing digest or secret invalidates all existing sessions! - class CookieStore < Rack::Session::Cookie + class CookieStore < Rack::Session::Abstract::ID include Compatibility include StaleSessionCheck include SessionObject - # Override rack's method + def initialize(app, options={}) + super(app, options.merge!(:cookie_only => true)) + end + def destroy_session(env, session_id, options) - new_sid = super + new_sid = generate_sid unless options[:drop] # Reset hash and Assign the new session id env["action_dispatch.request.unsigned_session_cookie"] = new_sid ? { "session_id" => new_sid } : {} new_sid end + def load_session(env) + stale_session_check! do + data = unpacked_cookie_data(env) + data = persistent_session_id!(data) + [data["session_id"], data] + end + end + private + def extract_session_id(env) + stale_session_check! do + unpacked_cookie_data(env)["session_id"] + end + end + def unpacked_cookie_data(env) env["action_dispatch.request.unsigned_session_cookie"] ||= begin stale_session_check! do @@ -65,6 +79,12 @@ module ActionDispatch end end + def persistent_session_id!(data, sid=nil) + data ||= {} + data["session_id"] ||= sid || generate_sid + data + end + def set_session(env, sid, session_data, options) session_data["session_id"] = sid session_data diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index 2b37a8d026..fcc5bc12c4 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -16,9 +16,9 @@ module ActionDispatch # catches the exceptions and returns a FAILSAFE_RESPONSE. class ShowExceptions FAILSAFE_RESPONSE = [500, { 'Content-Type' => 'text/plain' }, - ["500 Internal Server Error\n" << - "If you are the administrator of this website, then please read this web " << - "application's log file and/or the web server's log file to find out what " << + ["500 Internal Server Error\n" \ + "If you are the administrator of this website, then please read this web " \ + "application's log file and/or the web server's log file to find out what " \ "went wrong."]] def initialize(app, exceptions_app) @@ -27,13 +27,10 @@ module ActionDispatch end def call(env) - begin - response = @app.call(env) - rescue Exception => exception - raise exception if env['action_dispatch.show_exceptions'] == false - end - - response || render_exception(env, exception) + @app.call(env) + rescue Exception => exception + raise exception if env['action_dispatch.show_exceptions'] == false + render_exception(env, exception) end private diff --git a/actionpack/lib/action_dispatch/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb index 9098f4e170..9e03cbf2b7 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -45,7 +45,7 @@ module ActionDispatch # http://tools.ietf.org/html/draft-hodges-strict-transport-sec-02 def hsts_headers if @hsts - value = "max-age=#{@hsts[:expires]}" + value = "max-age=#{@hsts[:expires].to_i}" value += "; includeSubDomains" if @hsts[:subdomains] { 'Strict-Transport-Security' => value } else diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index e3b15b43b9..c6a7d9c415 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -6,7 +6,8 @@ module ActionDispatch def initialize(root, cache_control) @root = root.chomp('/') @compiled_root = /^#{Regexp.escape(root)}/ - @file_server = ::Rack::File.new(@root, cache_control) + headers = cache_control && { 'Cache-Control' => cache_control } + @file_server = ::Rack::File.new(@root, headers) end def match?(path) diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb index 823f5d25b6..550f4dbd0d 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb @@ -1,8 +1,8 @@ <% unless @exception.blamed_files.blank? %> <% if (hide = @exception.blamed_files.length > 8) %> - <a href="#" onclick="document.getElementById('blame_trace').style.display='block'; return false;">Show blamed files</a> + <a href="#" onclick="return toggleTrace()">Toggle blamed files</a> <% end %> - <pre id="blame_trace" <%='style="display:none"' if hide %>><code><%=h @exception.describe_blame %></code></pre> + <pre id="blame_trace" <%='style="display:none"' if hide %>><code><%= @exception.describe_blame %></code></pre> <% end %> <% @@ -18,14 +18,17 @@ %> <h2 style="margin-top: 30px">Request</h2> -<p><b>Parameters</b>: <pre><%=h request_dump %></pre></p> +<p><b>Parameters</b>:</p> <pre><%= request_dump %></pre> -<p><a href="#" onclick="document.getElementById('session_dump').style.display='block'; return false;">Show session dump</a></p> -<div id="session_dump" style="display:none"><pre><%= debug_hash @request.session %></pre></div> - -<p><a href="#" onclick="document.getElementById('env_dump').style.display='block'; return false;">Show env dump</a></p> -<div id="env_dump" style="display:none"><pre><%= debug_hash @request.env.slice(*@request.class::ENV_METHODS) %></pre></div> +<div class="details"> + <div class="summary"><a href="#" onclick="return toggleSessionDump()">Toggle session dump</a></div> + <div id="session_dump" style="display:none"><pre><%= debug_hash @request.session %></pre></div> +</div> +<div class="details"> + <div class="summary"><a href="#" onclick="return toggleEnvDump()">Toggle env dump</a></div> + <div id="env_dump" style="display:none"><pre><%= debug_hash @request.env.slice(*@request.class::ENV_METHODS) %></pre></div> +</div> <h2 style="margin-top: 30px">Response</h2> -<p><b>Headers</b>: <pre><%=h defined?(@response) ? @response.headers.inspect.gsub(',', ",\n") : 'None' %></pre></p> +<p><b>Headers</b>:</p> <pre><%= defined?(@response) ? @response.headers.inspect.gsub(',', ",\n") : 'None' %></pre> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb new file mode 100644 index 0000000000..38429cb78e --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb @@ -0,0 +1,25 @@ +<% if @source_extract %> +<div class="source"> +<div class="info"> + Extracted source (around line <strong>#<%= @line_number %></strong>): +</div> +<div class="data"> + <table cellpadding="0" cellspacing="0" class="lines"> + <tr> + <td> + <pre class="line_numbers"> + <% @source_extract.keys.each do |line_number| %> +<span><%= line_number -%></span> + <% end %> + </pre> + </td> +<td width="100%"> +<pre> +<% @source_extract.each do |line, source| -%><div class="line<%= " active" if line == @line_number -%>"><%= source -%></div><% end -%> +</pre> +</td> + </tr> + </table> +</div> +</div> +<% end %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb index 8771b5fd6d..9d947aea40 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb @@ -12,15 +12,15 @@ <div id="traces"> <% names.each do |name| %> <% - show = "document.getElementById('#{name.gsub(/\s/, '-')}').style.display='block';" - hide = (names - [name]).collect {|hide_name| "document.getElementById('#{hide_name.gsub(/\s/, '-')}').style.display='none';"} + show = "show('#{name.gsub(/\s/, '-')}');" + hide = (names - [name]).collect {|hide_name| "hide('#{hide_name.gsub(/\s/, '-')}');"} %> <a href="#" onclick="<%= hide.join %><%= show %>; return false;"><%= name %></a> <%= '|' unless names.last == name %> <% end %> <% traces.each do |name, trace| %> <div id="<%= name.gsub(/\s/, '-') %>" style="display: <%= (name == "Application Trace") ? 'block' : 'none' %>;"> - <pre><code><%=h trace.join "\n" %></code></pre> + <pre><code><%= trace.join "\n" %></code></pre> </div> <% end %> </div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb index c5043c5e7b..57a2940802 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb @@ -1,10 +1,16 @@ -<h1> - <%=h @exception.class.to_s %> - <% if @request.parameters['controller'] %> - in <%=h @request.parameters['controller'].camelize %>Controller<% if @request.parameters['action'] %>#<%=h @request.parameters['action'] %><% end %> - <% end %> -</h1> -<pre><%=h @exception.message %></pre> +<header> + <h1> + <%= @exception.class.to_s %> + <% if @request.parameters['controller'] %> + in <%= @request.parameters['controller'].camelize %>Controller<% if @request.parameters['action'] %>#<%= @request.parameters['action'] %><% end %> + <% end %> + </h1> +</header> -<%= render template: "rescues/_trace" %> -<%= render template: "rescues/_request_and_response" %> +<div id="container"> + <h2><%= @exception.message %></h2> + + <%= render template: "rescues/_source" %> + <%= render template: "rescues/_trace" %> + <%= render template: "rescues/_request_and_response" %> +</div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb index 1a308707d1..891c87ac27 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb @@ -4,7 +4,11 @@ <meta charset="utf-8" /> <title>Action Controller: Exception caught</title> <style> - body { background-color: #fff; color: #333; } + body { + background-color: #FAFAFA; + color: #333; + margin: 0px; + } body, p, ol, ul, td { font-family: helvetica, verdana, arial, sans-serif; @@ -13,16 +17,128 @@ } pre { - background-color: #eee; - padding: 10px; font-size: 11px; white-space: pre-wrap; } - a { color: #000; } + pre.box { + border: 1px solid #EEE; + padding: 10px; + margin: 0px; + width: 958px; + } + + header { + color: #F0F0F0; + background: #C52F24; + padding: 0.5em 1.5em; + } + + h2 { + color: #C52F24; + line-height: 25px; + } + + .details { + border: 1px solid #D0D0D0; + border-radius: 4px; + margin: 1em 0px; + display: block; + width: 978px; + } + + .summary { + padding: 8px 15px; + border-bottom: 1px solid #D0D0D0; + display: block; + } + + .details pre { + margin: 5px; + border: none; + } + + #container { + box-sizing: border-box; + width: 100%; + padding: 0 1.5em; + } + + .source * { + margin: 0px; + padding: 0px; + } + + .source { + border: 1px solid #D9D9D9; + background: #ECECEC; + width: 978px; + } + + .source pre { + padding: 10px 0px; + border: none; + } + + .source .data { + font-size: 80%; + overflow: auto; + background-color: #FFF; + } + + .info { + padding: 0.5em; + } + + .source .data .line_numbers { + background-color: #ECECEC; + color: #AAA; + padding: 1em .5em; + border-right: 1px solid #DDD; + text-align: right; + } + + .line { + padding-left: 10px; + } + + .line:hover { + background-color: #F6F6F6; + } + + .line.active { + background-color: #FFCCCC; + } + + a { color: #980905; } a:visited { color: #666; } - a:hover { color: #fff; background-color:#000; } + a:hover { color: #C52F24; } + + <%= yield :style %> </style> + + <script> + var toggle = function(id) { + var s = document.getElementById(id).style; + s.display = s.display == 'none' ? 'block' : 'none'; + return false; + } + var show = function(id) { + document.getElementById(id).style.display = 'block'; + } + var hide = function(id) { + document.getElementById(id).style.display = 'none'; + } + var toggleTrace = function() { + return toggle('blame_trace'); + } + var toggleSessionDump = function() { + return toggle('session_dump'); + } + var toggleEnvDump = function() { + return toggle('env_dump'); + } + </script> </head> <body> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb index dbfdf76947..ca14215946 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb @@ -1,2 +1,7 @@ -<h1>Template is missing</h1> -<p><%=h @exception.message %></p> +<header> + <h1>Template is missing</h1> +</header> + +<div id="container"> + <h2><%= @exception.message %></h2> +</div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb index 6c903d6a17..cd3daff065 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb @@ -1,25 +1,30 @@ -<h1>Routing Error</h1> -<p><pre><%=h @exception.message %></pre></p> -<% unless @exception.failures.empty? %> - <p> - <h2>Failure reasons:</h2> - <ol> - <% @exception.failures.each do |route, reason| %> - <li><code><%=h route.inspect.gsub('\\', '') %></code> failed because <%=h reason.downcase %></li> - <% end %> - </ol> - </p> -<% end %> -<%= render template: "rescues/_trace" %> +<header> + <h1>Routing Error</h1> +</header> +<div id="container"> + <h2><%= @exception.message %></h2> + <% unless @exception.failures.empty? %> + <p> + <h2>Failure reasons:</h2> + <ol> + <% @exception.failures.each do |route, reason| %> + <li><code><%= route.inspect.delete('\\') %></code> failed because <%= reason.downcase %></li> + <% end %> + </ol> + </p> + <% end %> -<h2> - Routes -</h2> + <%= render template: "rescues/_trace" %> -<p> - Routes match in priority from top to bottom -</p> + <% if @routes_inspector %> + <h2> + Routes + </h2> -<%= render layout: "routes/route_wrapper" do %> - <%= render partial: "routes/route", collection: @routes %> -<% end %> + <p> + Routes match in priority from top to bottom + </p> + + <%= @routes_inspector.format(ActionDispatch::Routing::HtmlTableFormatter.new(self)) %> + <% end %> +</div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb index a1b377f68c..63216ef7c5 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb @@ -1,17 +1,43 @@ -<h1> - <%=h @exception.original_exception.class.to_s %> in - <%=h @request.parameters["controller"].capitalize if @request.parameters["controller"]%>#<%=h @request.parameters["action"] %> -</h1> +<% @source_extract = @exception.source_extract(0, :html) %> +<header> + <h1> + <%= @exception.original_exception.class.to_s %> in + <%= @request.parameters["controller"].capitalize if @request.parameters["controller"]%>#<%= @request.parameters["action"] %> + </h1> +</header> -<p> - Showing <i><%=h @exception.file_name %></i> where line <b>#<%=h @exception.line_number %></b> raised: - <pre><code><%=h @exception.message %></code></pre> -</p> +<div id="container"> + <p> + Showing <i><%= @exception.file_name %></i> where line <b>#<%= @exception.line_number %></b> raised: + </p> + <pre><code><%= @exception.message %></code></pre> -<p>Extracted source (around line <b>#<%=h @exception.line_number %></b>): -<pre><code><%=h @exception.source_extract %></code></pre></p> + <div class="source"> + <div class="info"> + <p>Extracted source (around line <strong>#<%= @exception.line_number %></strong>):</p> + </div> + <div class="data"> + <table cellpadding="0" cellspacing="0" class="lines"> + <tr> + <td> + <pre class="line_numbers"> + <% @source_extract.keys.each do |line_number| %> +<span><%= line_number -%></span> + <% end %> + </pre> + </td> +<td width="100%"> +<pre> +<% @source_extract.each do |line, source| -%><div class="line<%= " active" if line == @exception.line_number -%>"><%= source -%></div><% end -%> +</pre> +</td> + </tr> + </table> +</div> +</div> -<p><%=h @exception.sub_template_message %></p> + <p><%= @exception.sub_template_message %></p> -<%= render template: "rescues/_trace" %> -<%= render template: "rescues/_request_and_response" %> + <%= render template: "rescues/_trace" %> + <%= render template: "rescues/_request_and_response" %> +</div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb index 683379da10..c1fbf67eed 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb @@ -1,2 +1,6 @@ -<h1>Unknown action</h1> -<p><%=h @exception.message %></p> +<header> + <h1>Unknown action</h1> +</header> +<div id="container"> + <h2><%= @exception.message %></h2> +</div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/routes/_route.html.erb b/actionpack/lib/action_dispatch/middleware/templates/routes/_route.html.erb index 400ae97d22..24e44f31ac 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/routes/_route.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/routes/_route.html.erb @@ -7,7 +7,7 @@ <td data-route-verb='<%= route[:verb] %>'> <%= route[:verb] %> </td> - <td data-route-path='<%= route[:path] %>'> + <td data-route-path='<%= route[:path] %>' data-regexp='<%= route[:regexp] %>'> <%= route[:path] %> </td> <td data-route-reqs='<%= route[:reqs] %>'> diff --git a/actionpack/lib/action_dispatch/middleware/templates/routes/_route_wrapper.html.erb b/actionpack/lib/action_dispatch/middleware/templates/routes/_route_wrapper.html.erb deleted file mode 100644 index dc17cb77ef..0000000000 --- a/actionpack/lib/action_dispatch/middleware/templates/routes/_route_wrapper.html.erb +++ /dev/null @@ -1,56 +0,0 @@ -<style type='text/css'> - #route_table td {padding: 0 30px;} - #route_table {margin: 0 auto 0;} -</style> - -<table id='route_table' class='route_table'> - <thead> - <tr> - <th>Helper<br /> - <%= link_to "Path", "#", 'data-route-helper' => '_path', - title: "Returns a relative path (without the http or domain)" %> / - <%= link_to "Url", "#", 'data-route-helper' => '_url', - title: "Returns an absolute url (with the http and domain)" %> - </th> - <th>HTTP Verb</th> - <th>Path</th> - <th>Controller#Action</th> - </tr> - </thead> - <tbody> - <%= yield %> - </tbody> -</table> - -<script type='text/javascript'> - function each(elems, func) { - if (!elems instanceof Array) { elems = [elems]; } - for (var i = elems.length; i--; ) { - func(elems[i]); - } - } - - function setValOn(elems, val) { - each(elems, function(elem) { - elem.innerHTML = val; - }); - } - - function onClick(elems, func) { - each(elems, function(elem) { - elem.onclick = func; - }); - } - - // Enables functionality to toggle between `_path` and `_url` helper suffixes - function setupRouteToggleHelperLinks() { - var toggleLinks = document.querySelectorAll('#route_table [data-route-helper]'); - onClick(toggleLinks, function(){ - var helperTxt = this.getAttribute("data-route-helper"); - var helperElems = document.querySelectorAll('[data-route-name] span.helper'); - setValOn(helperElems, helperTxt); - }); - } - - setupRouteToggleHelperLinks(); -</script> diff --git a/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb b/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb new file mode 100644 index 0000000000..95461fa693 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb @@ -0,0 +1,144 @@ +<% content_for :style do %> + #route_table { + margin: 0 auto 0; + border-collapse: collapse; + } + + #route_table td { + padding: 0 30px; + } + + #route_table tr.bottom th { + padding-bottom: 10px; + line-height: 15px; + } + + #route_table .matched_paths { + background-color: LightGoldenRodYellow; + } + + #route_table .matched_paths { + border-bottom: solid 3px SlateGrey; + } + + #path_search { + width: 80%; + font-size: inherit; + } +<% end %> + +<table id='route_table' class='route_table'> + <thead> + <tr> + <th>Helper</th> + <th>HTTP Verb</th> + <th>Path</th> + <th>Controller#Action</th> + </tr> + <tr class='bottom'> + <th><%# Helper %> + <%= link_to "Path", "#", 'data-route-helper' => '_path', + title: "Returns a relative path (without the http or domain)" %> / + <%= link_to "Url", "#", 'data-route-helper' => '_url', + title: "Returns an absolute url (with the http and domain)" %> + </th> + <th><%# HTTP Verb %> + </th> + <th><%# Path %> + <%= search_field(:path, nil, id: 'path_search', placeholder: "Path Match") %> + </th> + <th><%# Controller#action %> + </th> + </tr> + </thead> + <tbody class='matched_paths' id='matched_paths'> + </tbody> + <tbody> + <%= yield %> + </tbody> +</table> + +<script type='text/javascript'> + function each(elems, func) { + if (!elems instanceof Array) { elems = [elems]; } + for (var i = 0, len = elems.length; i < len; i++) { + func(elems[i]); + } + } + + function setValOn(elems, val) { + each(elems, function(elem) { + elem.innerHTML = val; + }); + } + + function onClick(elems, func) { + each(elems, function(elem) { + elem.onclick = func; + }); + } + + // Enables functionality to toggle between `_path` and `_url` helper suffixes + function setupRouteToggleHelperLinks() { + var toggleLinks = document.querySelectorAll('#route_table [data-route-helper]'); + onClick(toggleLinks, function(){ + var helperTxt = this.getAttribute("data-route-helper"), + helperElems = document.querySelectorAll('[data-route-name] span.helper'); + setValOn(helperElems, helperTxt); + }); + } + + // takes an array of elements with a data-regexp attribute and + // passes their their parent <tr> into the callback function + // if the regexp matchs a given path + function eachElemsForPath(elems, path, func) { + each(elems, function(e){ + var reg = e.getAttribute("data-regexp"); + if (path.match(RegExp(reg))) { + func(e.parentNode.cloneNode(true)); + } + }) + } + + // Ensure path always starts with a slash "/" and remove params or fragments + function sanitizePath(path) { + var path = path.charAt(0) == '/' ? path : "/" + path; + return path.replace(/\#.*|\?.*/, ''); + } + + // Enables path search functionality + function setupMatchPaths() { + var regexpElems = document.querySelectorAll('#route_table [data-regexp]'), + pathElem = document.querySelector('#path_search'), + selectedSection = document.querySelector('#matched_paths'), + noMatchText = '<tr><th colspan="4">None</th></tr>'; + + + // Remove matches if no path is present + pathElem.onblur = function(e) { + if (pathElem.value === "") selectedSection.innerHTML = ""; + } + + // On key press perform a search for matching paths + pathElem.onkeyup = function(e){ + var path = sanitizePath(pathElem.value), + defaultText = '<tr><th colspan="4">Paths Matching (' + path + '):</th></tr>'; + + // Clear out results section + selectedSection.innerHTML= defaultText; + + // Display matches if they exist + eachElemsForPath(regexpElems, path, function(e){ + selectedSection.appendChild(e); + }); + + // If no match present, tell the user + if (selectedSection.innerHTML === defaultText) { + selectedSection.innerHTML = selectedSection.innerHTML + noMatchText; + } + } + } + + setupMatchPaths(); + setupRouteToggleHelperLinks(); +</script> diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index 5a835ae439..edf37bb9a5 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -6,7 +6,6 @@ module ActionDispatch config.action_dispatch.x_sendfile_header = nil config.action_dispatch.ip_spoofing_check = true config.action_dispatch.show_exceptions = true - config.action_dispatch.best_standards_support = true config.action_dispatch.tld_length = 1 config.action_dispatch.ignore_accept_header = false config.action_dispatch.rescue_templates = { } @@ -21,7 +20,8 @@ module ActionDispatch config.action_dispatch.default_headers = { 'X-Frame-Options' => 'SAMEORIGIN', 'X-XSS-Protection' => '1; mode=block', - 'X-Content-Type-Options' => 'nosniff' + 'X-Content-Type-Options' => 'nosniff', + 'X-UA-Compatible' => 'chrome=1' } config.eager_load_namespaces << ActionDispatch diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb index a05a23d953..7bc812fd22 100644 --- a/actionpack/lib/action_dispatch/request/session.rb +++ b/actionpack/lib/action_dispatch/request/session.rb @@ -63,6 +63,10 @@ module ActionDispatch @exists = nil # we haven't checked yet end + def id + options[:id] + end + def options Options.find @env end diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index 73b2b4ac1d..d251de33df 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -1,4 +1,5 @@ require 'delegate' +require 'active_support/core_ext/string/strip' module ActionDispatch module Routing @@ -34,6 +35,23 @@ module ActionDispatch super.to_s end + def regexp + __getobj__.path.to_regexp + end + + def json_regexp + str = regexp.inspect. + sub('\\A' , '^'). + sub('\\Z' , '$'). + sub('\\z' , '$'). + sub(/^\// , ''). + sub(/\/[a-z]*$/ , ''). + gsub(/\(\?#.+\)/ , ''). + gsub(/\(\?-\w+:/ , '('). + gsub(/\s/ , '') + Regexp.new(str).source + end + def reqs @reqs ||= begin reqs = endpoint @@ -61,32 +79,58 @@ module ActionDispatch ## # This class is just used for displaying route information when someone - # executes `rake routes`. People should not use this class. + # executes `rake routes` or looks at the RoutingError page. + # People should not use this class. class RoutesInspector # :nodoc: - def initialize - @engines = Hash.new + def initialize(routes) + @engines = {} + @routes = routes end - def format(all_routes, filter = nil) - if filter - all_routes = all_routes.select{ |route| route.defaults[:controller] == filter } + def format(formatter, filter = nil) + routes_to_display = filter_routes(filter) + + routes = collect_routes(routes_to_display) + + if routes.none? + formatter.no_routes + return formatter.result + end + + formatter.header routes + formatter.section routes + + @engines.each do |name, engine_routes| + formatter.section_title "Routes for #{name}" + formatter.section engine_routes end - routes = collect_routes(all_routes) + formatter.result + end + + private - formatted_routes(routes) + - formatted_routes_for_engines + def filter_routes(filter) + if filter + @routes.select { |route| route.defaults[:controller] == filter } + else + @routes + end end def collect_routes(routes) - routes = routes.collect do |route| + routes.collect do |route| RouteWrapper.new(route) end.reject do |route| route.internal? end.collect do |route| collect_engine_routes(route) - { name: route.name, verb: route.verb, path: route.path, reqs: route.reqs } + { name: route.name, + verb: route.verb, + path: route.path, + reqs: route.reqs, + regexp: route.json_regexp } end end @@ -100,21 +144,96 @@ module ActionDispatch @engines[name] = collect_routes(routes.routes) end end + end + + class ConsoleFormatter + def initialize + @buffer = [] + end + + def result + @buffer.join("\n") + end - def formatted_routes_for_engines - @engines.map do |name, routes| - ["\nRoutes for #{name}:"] + formatted_routes(routes) - end.flatten + def section_title(title) + @buffer << "\n#{title}:" end - def formatted_routes(routes) - name_width = routes.map{ |r| r[:name].length }.max - verb_width = routes.map{ |r| r[:verb].length }.max - path_width = routes.map{ |r| r[:path].length }.max + def section(routes) + @buffer << draw_section(routes) + end + + def header(routes) + @buffer << draw_header(routes) + end - routes.map do |r| - "#{r[:name].rjust(name_width)} #{r[:verb].ljust(verb_width)} #{r[:path].ljust(path_width)} #{r[:reqs]}" + def no_routes + @buffer << <<-MESSAGE.strip_heredoc + You don't have any routes defined! + + Please add some routes in config/routes.rb. + + For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html. + MESSAGE + end + + private + def draw_section(routes) + name_width, verb_width, path_width = widths(routes) + + routes.map do |r| + "#{r[:name].rjust(name_width)} #{r[:verb].ljust(verb_width)} #{r[:path].ljust(path_width)} #{r[:reqs]}" + end end + + def draw_header(routes) + name_width, verb_width, path_width = widths(routes) + + "#{"Prefix".rjust(name_width)} #{"Verb".ljust(verb_width)} #{"URI Pattern".ljust(path_width)} Controller#Action" + end + + def widths(routes) + [routes.map { |r| r[:name].length }.max, + routes.map { |r| r[:verb].length }.max, + routes.map { |r| r[:path].length }.max] + end + end + + class HtmlTableFormatter + def initialize(view) + @view = view + @buffer = [] + end + + def section_title(title) + @buffer << %(<tr><th colspan="4">#{title}</th></tr>) + end + + def section(routes) + @buffer << @view.render(partial: "routes/route", collection: routes) + end + + # the header is part of the HTML page, so we don't construct it here. + def header(routes) + end + + def no_routes + @buffer << <<-MESSAGE.strip_heredoc + <p>You don't have any routes defined!</p> + <ul> + <li>Please add some routes in <tt>config/routes.rb</tt>.</li> + <li> + For more information about routes, please see the Rails guide + <a href="http://guides.rubyonrails.org/routing.html">Rails Routing from the Outside In</a>. + </li> + </ul> + MESSAGE + end + + def result + @view.raw @view.render(layout: "routes/table") { + @view.raw @buffer.join("\n") + } end end end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 0dc6403ec0..c5f2b33602 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -2,12 +2,15 @@ require 'active_support/core_ext/hash/except' require 'active_support/core_ext/hash/reverse_merge' require 'active_support/core_ext/hash/slice' require 'active_support/core_ext/enumerable' +require 'active_support/core_ext/array/extract_options' require 'active_support/inflector' require 'action_dispatch/routing/redirection' module ActionDispatch module Routing class Mapper + URL_OPTIONS = [:protocol, :subdomain, :domain, :host, :port] + class Constraints #:nodoc: def self.new(app, constraints, request = Rack::Request) if constraints.any? @@ -26,15 +29,10 @@ module ActionDispatch def matches?(env) req = @request.new(env) - @constraints.each { |constraint| - if constraint.respond_to?(:matches?) && !constraint.matches?(req) - return false - elsif constraint.respond_to?(:call) && !constraint.call(*constraint_args(constraint, req)) - return false - end - } - - return true + @constraints.all? do |constraint| + (constraint.respond_to?(:matches?) && constraint.matches?(req)) || + (constraint.respond_to?(:call) && constraint.call(*constraint_args(constraint, req))) + end ensure req.reset_parameters end @@ -50,73 +48,60 @@ module ActionDispatch end class Mapping #:nodoc: - IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix] + IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix, :format] ANCHOR_CHARACTERS_REGEX = %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z} - SHORTHAND_REGEX = %r{/[\w/]+$} WILDCARD_PATH = %r{\*([^/\)]+)\)?$} - def initialize(set, scope, path, options) - @set, @scope = set, scope - @segment_keys = nil - @options = (@scope[:options] || {}).merge(options) - @path = normalize_path(path) - normalize_options! + attr_reader :scope, :path, :options, :requirements, :conditions, :defaults - via_all = @options.delete(:via) if @options[:via] == :all + def initialize(set, scope, path, options) + @set, @scope, @path, @options = set, scope, path, options + @requirements, @conditions, @defaults = {}, {}, {} - if !via_all && request_method_condition.empty? - msg = "You should not use the `match` method in your router without specifying an HTTP method.\n" \ - "If you want to expose your action to GET, use `get` in the router:\n\n" \ - " Instead of: match \"controller#action\"\n" \ - " Do: get \"controller#action\"" - raise msg - end + normalize_path! + normalize_options! + normalize_requirements! + normalize_conditions! + normalize_defaults! end def to_route - [ app, conditions, requirements, defaults, @options[:as], @options[:anchor] ] + [ app, conditions, requirements, defaults, options[:as], options[:anchor] ] end private - def normalize_options! - path_without_format = @path.sub(/\(\.:format\)$/, '') + def normalize_path! + raise ArgumentError, "path is required" if @path.blank? + @path = Mapper.normalize_path(@path) - if using_match_shorthand?(path_without_format, @options) - to_shorthand = @options[:to].blank? - @options[:to] ||= path_without_format.gsub(/\(.*\)/, "")[1..-1].sub(%r{/([^/]*)$}, '#\1') - end - - @options.merge!(default_controller_and_action(to_shorthand)) - - requirements.each do |name, requirement| - # segment_keys.include?(k.to_s) || k == :controller - next unless Regexp === requirement && !constraints[name] - - if requirement.source =~ ANCHOR_CHARACTERS_REGEX - raise ArgumentError, "Regexp anchor characters are not allowed in routing requirements: #{requirement.inspect}" - end - if requirement.multiline? - raise ArgumentError, "Regexp multiline option not allowed in routing requirements: #{requirement.inspect}" - end + if required_format? + @path = "#{@path}.:format" + elsif optional_format? + @path = "#{@path}(.:format)" end + end - if @options[:constraints].is_a?(Hash) - (@options[:defaults] ||= {}).reverse_merge!(defaults_from_constraints(@options[:constraints])) - end + def required_format? + options[:format] == true end - # match "account/overview" - def using_match_shorthand?(path, options) - path && (options[:to] || options[:action]).nil? && path =~ SHORTHAND_REGEX + def optional_format? + options[:format] != false && !path.include?(':format') && !path.end_with?('/') end - def normalize_path(path) - raise ArgumentError, "path is required" if path.blank? - path = Mapper.normalize_path(path) + def normalize_options! + @options.reverse_merge!(scope[:options]) if scope[:options] + path_without_format = path.sub(/\(\.:format\)$/, '') - if path.match(':controller') - raise ArgumentError, ":controller segment is not allowed within a namespace block" if @scope[:module] + # Add a constraint for wildcard route to make it non-greedy and match the + # optional format part of the route by default + if path_without_format.match(WILDCARD_PATH) && @options[:format] != false + @options[$1.to_sym] ||= /.+?/ + end + + if path_without_format.match(':controller') + raise ArgumentError, ":controller segment is not allowed within a namespace block" if scope[:module] # Add a default constraint for :controller path segments that matches namespaced # controllers with default routes like :controller/:action/:id(.:format), e.g: @@ -125,51 +110,95 @@ module ActionDispatch @options[:controller] ||= /.+?/ end - # Add a constraint for wildcard route to make it non-greedy and match the - # optional format part of the route by default - if path.match(WILDCARD_PATH) && @options[:format] != false - @options[$1.to_sym] ||= /.+?/ + @options.merge!(default_controller_and_action) + end + + def normalize_requirements! + constraints.each do |key, requirement| + next unless segment_keys.include?(key) || key == :controller + verify_regexp_requirement(requirement) if requirement.is_a?(Regexp) + @requirements[key] = requirement end - if @options[:format] == false - @options.delete(:format) - path - elsif path.include?(":format") || path.end_with?('/') - path - elsif @options[:format] == true - "#{path}.:format" - else - "#{path}(.:format)" + if options[:format] == true + @requirements[:format] ||= /.+/ + elsif Regexp === options[:format] + @requirements[:format] = options[:format] + elsif String === options[:format] + @requirements[:format] = Regexp.compile(options[:format]) end end - def app - Constraints.new( - to.respond_to?(:call) ? to : Routing::RouteSet::Dispatcher.new(:defaults => defaults), - blocks, - @set.request_class - ) - end + def verify_regexp_requirement(requirement) + if requirement.source =~ ANCHOR_CHARACTERS_REGEX + raise ArgumentError, "Regexp anchor characters are not allowed in routing requirements: #{requirement.inspect}" + end - def conditions - { :path_info => @path }.merge!(constraints).merge!(request_method_condition) + if requirement.multiline? + raise ArgumentError, "Regexp multiline option is not allowed in routing requirements: #{requirement.inspect}" + end end - def requirements - @requirements ||= (@options[:constraints].is_a?(Hash) ? @options[:constraints] : {}).tap do |requirements| - requirements.reverse_merge!(@scope[:constraints]) if @scope[:constraints] - @options.each { |k, v| requirements[k] ||= v if v.is_a?(Regexp) } + def normalize_defaults! + @defaults.merge!(scope[:defaults]) if scope[:defaults] + @defaults.merge!(options[:defaults]) if options[:defaults] + + options.each do |key, default| + next if Regexp === default || IGNORE_OPTIONS.include?(key) + @defaults[key] = default + end + + if options[:constraints].is_a?(Hash) + options[:constraints].each do |key, default| + next unless URL_OPTIONS.include?(key) && (String === default || Fixnum === default) + @defaults[key] ||= default + end + end + + if Regexp === options[:format] + @defaults[:format] = nil + elsif String === options[:format] + @defaults[:format] = options[:format] end end - def defaults - @defaults ||= (@options[:defaults] || {}).tap do |defaults| - defaults.reverse_merge!(@scope[:defaults]) if @scope[:defaults] - @options.each { |k, v| defaults[k] = v unless v.is_a?(Regexp) || IGNORE_OPTIONS.include?(k.to_sym) } + def normalize_conditions! + @conditions.merge!(:path_info => path) + + constraints.each do |key, condition| + next if segment_keys.include?(key) || key == :controller + @conditions[key] = condition end + + @conditions[:required_defaults] = [] + options.each do |key, required_default| + next if segment_keys.include?(key) || IGNORE_OPTIONS.include?(key) + next if Regexp === required_default + @conditions[:required_defaults] << key + end + + via_all = options.delete(:via) if options[:via] == :all + + if !via_all && options[:via].blank? + msg = "You should not use the `match` method in your router without specifying an HTTP method.\n" \ + "If you want to expose your action to both GET and POST, add `via: [:get, :post]` option.\n" \ + "If you want to expose your action to GET, use `get` in the router:\n" \ + " Instead of: match \"controller#action\"\n" \ + " Do: get \"controller#action\"" + raise msg + end + + if via = options[:via] + list = Array(via).map { |m| m.to_s.dasherize.upcase } + @conditions.merge!(:request_method => list) + end + end + + def app + Constraints.new(endpoint, blocks, @set.request_class) end - def default_controller_and_action(to_shorthand=nil) + def default_controller_and_action if to.respond_to?(:call) { } else @@ -182,7 +211,7 @@ module ActionDispatch controller ||= default_controller action ||= default_action - unless controller.is_a?(Regexp) || to_shorthand + unless controller.is_a?(Regexp) controller = [@scope[:module], controller].compact.join("/").presence end @@ -193,14 +222,20 @@ module ActionDispatch controller = controller.to_s unless controller.is_a?(Regexp) action = action.to_s unless action.is_a?(Regexp) - if controller.blank? && segment_keys.exclude?("controller") + if controller.blank? && segment_keys.exclude?(:controller) raise ArgumentError, "missing :controller" end - if action.blank? && segment_keys.exclude?("action") + if action.blank? && segment_keys.exclude?(:action) raise ArgumentError, "missing :action" end + if controller.is_a?(String) && controller !~ /\A[a-z_0-9\/]*\z/ + message = "'#{controller}' is not a supported controller name. This can lead to potential routing problems." + message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use" + raise ArgumentError, message + end + hash = {} hash[:controller] = controller unless controller.blank? hash[:action] = action unless action.blank? @@ -209,50 +244,55 @@ module ActionDispatch end def blocks - constraints = @options[:constraints] - if constraints.present? && !constraints.is_a?(Hash) - [constraints] + if options[:constraints].present? && !options[:constraints].is_a?(Hash) + [options[:constraints]] else - @scope[:blocks] || [] + scope[:blocks] || [] end end def constraints - @constraints ||= requirements.reject { |k, v| segment_keys.include?(k.to_s) || k == :controller } - end + @constraints ||= {}.tap do |constraints| + constraints.merge!(scope[:constraints]) if scope[:constraints] - def request_method_condition - if via = @options[:via] - list = Array(via).map { |m| m.to_s.dasherize.upcase } - { :request_method => list } - else - { } + options.except(*IGNORE_OPTIONS).each do |key, option| + constraints[key] = option if Regexp === option + end + + constraints.merge!(options[:constraints]) if options[:constraints].is_a?(Hash) end end def segment_keys - return @segment_keys if @segment_keys + @segment_keys ||= path_pattern.names.map{ |s| s.to_sym } + end - @segment_keys = Journey::Path::Pattern.new( - Journey::Router::Strexp.compile(@path, requirements, SEPARATORS) - ).names + def path_pattern + Journey::Path::Pattern.new(strexp) + end + + def strexp + Journey::Router::Strexp.compile(path, requirements, SEPARATORS) + end + + def endpoint + to.respond_to?(:call) ? to : dispatcher + end + + def dispatcher + Routing::RouteSet::Dispatcher.new(:defaults => defaults) end def to - @options[:to] + options[:to] end def default_controller - @options[:controller] || @scope[:controller] + options[:controller] || scope[:controller] end def default_action - @options[:action] || @scope[:action] - end - - def defaults_from_constraints(constraints) - url_keys = [:protocol, :subdomain, :domain, :host, :port] - constraints.select { |k, v| url_keys.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum)) } + options[:action] || scope[:action] end end @@ -284,7 +324,6 @@ module ActionDispatch # because this means it will be matched first. As this is the most popular route # of most Rails applications, this is beneficial. def root(options = {}) - options = { :to => options } if options.is_a?(String) match '/', { :as => :root, :via => :get }.merge!(options) end @@ -381,11 +420,15 @@ module ActionDispatch # end # # [:constraints] - # Constrains parameters with a hash of regular expressions or an - # object that responds to <tt>matches?</tt> + # Constrains parameters with a hash of regular expressions + # or an object that responds to <tt>matches?</tt>. In addition, constraints + # other than path can also be specified with any object + # that responds to <tt>===</tt> (eg. String, Array, Range, etc.). # # match 'path/:id', constraints: { id: /[A-Z]\d{5}/ } # + # match 'json_only', constraints: { format: 'json' } + # # class Blacklist # def matches?(request) request.remote_ip == '1.2.3.4' end # end @@ -646,7 +689,11 @@ module ActionDispatch options[:constraints] ||= {} if options[:constraints].is_a?(Hash) - (options[:defaults] ||= {}).reverse_merge!(defaults_from_constraints(options[:constraints])) + 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) else block, options[:constraints] = options[:constraints], {} end @@ -851,11 +898,6 @@ module ActionDispatch def override_keys(child) #:nodoc: child.key?(:only) || child.key?(:except) ? [:only, :except] : [] end - - def defaults_from_constraints(constraints) - url_keys = [:protocol, :subdomain, :domain, :host, :port] - constraints.select { |k, v| url_keys.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum)) } - end end # Resource routing allows you to quickly declare all of the common routes @@ -1328,6 +1370,11 @@ module ActionDispatch paths = [path] + rest end + path_without_format = path.to_s.sub(/\(\.:format\)$/, '') + if using_match_shorthand?(path_without_format, options) + options[:to] ||= path_without_format.gsub(%r{^/}, "").sub(%r{/([^/]*)$}, '#\1') + end + options[:anchor] = true unless options.key?(:anchor) if options[:on] && !VALID_ON_OPTIONS.include?(options[:on]) @@ -1338,6 +1385,10 @@ module ActionDispatch self end + def using_match_shorthand?(path, options) + path && (options[:to] || options[:action]).nil? && path =~ %r{/[\w/]+$} + end + def decomposed_match(path, options) # :nodoc: if on = options.delete(:on) send(on) { decomposed_match(path, options) } @@ -1355,9 +1406,10 @@ module ActionDispatch def add_route(action, options) # :nodoc: path = path_for_action(action, options.delete(:path)) + action = action.to_s.dup - if action.to_s =~ /^[\w\/]+$/ - options[:action] ||= action unless action.to_s.include?("/") + if action =~ /^[\w\/]+$/ + options[:action] ||= action unless action.include?("/") else action = nil end @@ -1373,7 +1425,15 @@ module ActionDispatch @set.add_route(app, conditions, requirements, defaults, as, anchor) end - def root(options={}) + def root(path, options={}) + if path.is_a?(String) + options[:to] = path + elsif path.is_a?(Hash) and options.empty? + options = path + else + raise ArgumentError, "must be called with a path and/or options" + end + if @scope[:scope_level] == :resources with_scope_level(:root) do scope(parent_resource.path) do diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index b1959e388c..619dd22ec1 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -4,6 +4,7 @@ require 'thread_safe' require 'active_support/core_ext/object/to_query' require 'active_support/core_ext/hash/slice' require 'active_support/core_ext/module/remove_method' +require 'active_support/core_ext/array/extract_options' require 'action_controller/metal/exceptions' module ActionDispatch @@ -30,6 +31,8 @@ module ActionDispatch # If any of the path parameters has a invalid encoding then # raise since it's likely to trigger errors further on. params.each do |key, value| + next unless value.respond_to?(:valid_encoding?) + unless value.valid_encoding? raise ActionController::BadRequest, "Invalid parameter: #{key} => #{value}" end @@ -100,36 +103,16 @@ module ActionDispatch def initialize @routes = {} @helpers = [] - @module = Module.new do - protected - - def handle_positional_args(args, options, segment_keys) - inner_options = args.extract_options! - result = options.dup - - if args.size > 0 - keys = segment_keys - if args.size < keys.size - 1 # take format into account - keys -= self.url_options.keys if self.respond_to?(:url_options) - keys -= options.keys - end - result.merge!(Hash[keys.zip(args)]) - end - - result.merge!(inner_options) - end - end + @module = Module.new end def helper_names - self.module.instance_methods.map(&:to_s) + @helpers.map(&:to_s) end def clear! @helpers.each do |helper| - @module.module_eval do - remove_possible_method helper - end + @module.remove_possible_method helper end @routes.clear @@ -162,68 +145,118 @@ module ActionDispatch routes.length end - private + class UrlHelper # :nodoc: + def self.create(route, options) + if optimize_helper?(route) + OptimizedUrlHelper.new(route, options) + else + new route, options + end + end - def define_named_route_methods(name, route) - define_url_helper route, :"#{name}_path", - route.defaults.merge(:use_route => name, :only_path => true) - define_url_helper route, :"#{name}_url", - route.defaults.merge(:use_route => name, :only_path => false) + def self.optimize_helper?(route) + route.requirements.except(:controller, :action).empty? end - # Create a url helper allowing ordered parameters to be associated - # with corresponding dynamic segments, so you can do: - # - # foo_url(bar, baz, bang) - # - # Instead of: - # - # foo_url(bar: bar, baz: baz, bang: bang) - # - # Also allow options hash, so you can do: - # - # foo_url(bar, baz, bang, sort_by: 'baz') - # - def define_url_helper(route, name, options) - @module.module_eval <<-END_EVAL, __FILE__, __LINE__ + 1 - remove_possible_method :#{name} - def #{name}(*args) - if #{optimize_helper?(route)} && args.size == #{route.required_parts.size} && !args.last.is_a?(Hash) && optimize_routes_generation? - options = #{options.inspect} - options.merge!(url_options) if respond_to?(:url_options) - options[:path] = "#{optimized_helper(route)}" - ActionDispatch::Http::URL.url_for(options) - else - url_for(handle_positional_args(args, #{options.inspect}, #{route.segment_keys.inspect})) - end + class OptimizedUrlHelper < UrlHelper # :nodoc: + attr_reader :arg_size + + def initialize(route, options) + super + @path_parts = @route.required_parts + @arg_size = @path_parts.size + @string_route = @route.optimized_path + end + + def call(t, args) + if args.size == arg_size && !args.last.is_a?(Hash) && optimize_routes_generation?(t) + @options.merge!(t.url_options) if t.respond_to?(:url_options) + @options[:path] = optimized_helper(args) + ActionDispatch::Http::URL.url_for(@options) + else + super end - END_EVAL + end - helpers << name + private + + def optimized_helper(args) + path = @string_route.dup + klass = Journey::Router::Utils + + @path_parts.zip(args) do |part, arg| + # Replace each route parameter + # e.g. :id for regular parameter or *path for globbing + # with ruby string interpolation code + path.gsub!(/(\*|:)#{part}/, klass.escape_fragment(arg.to_param)) + end + path + end + + def optimize_routes_generation?(t) + t.send(:optimize_routes_generation?) + end end - # Clause check about when we need to generate an optimized helper. - def optimize_helper?(route) #:nodoc: - route.requirements.except(:controller, :action).empty? + def initialize(route, options) + @options = options + @segment_keys = route.segment_keys + @route = route end - # Generates the interpolation to be used in the optimized helper. - def optimized_helper(route) - string_route = route.ast.to_s + def call(t, args) + t.url_for(handle_positional_args(t, args, @options, @segment_keys)) + end - while string_route.gsub!(/\([^\)]*\)/, "") - true - end + def handle_positional_args(t, args, options, keys) + inner_options = args.extract_options! + result = options.dup - route.required_parts.each_with_index do |part, i| - # Replace each route parameter - # e.g. :id for regular parameter or *path for globbing - # with ruby string interpolation code - string_route.gsub!(/(\*|:)#{part}/, "\#{Journey::Router::Utils.escape_fragment(args[#{i}].to_param)}") + if args.size > 0 + if args.size < keys.size - 1 # take format into account + keys -= t.url_options.keys if t.respond_to?(:url_options) + keys -= options.keys + end + result.merge!(Hash[keys.zip(args)]) end - string_route + result.merge!(inner_options) end + end + + private + # Create a url helper allowing ordered parameters to be associated + # with corresponding dynamic segments, so you can do: + # + # foo_url(bar, baz, bang) + # + # Instead of: + # + # foo_url(bar: bar, baz: baz, bang: bang) + # + # Also allow options hash, so you can do: + # + # foo_url(bar, baz, bang, sort_by: 'baz') + # + def define_url_helper(route, name, options) + helper = UrlHelper.create(route, options.dup) + + @module.remove_possible_method name + @module.module_eval do + define_method(name) do |*args| + helper.call self, args + end + end + + helpers << name + end + + def define_named_route_methods(name, route) + define_url_helper route, :"#{name}_path", + route.defaults.merge(:use_route => name, :only_path => true) + define_url_helper route, :"#{name}_url", + route.defaults.merge(:use_route => name, :only_path => false) + end end attr_accessor :formatter, :set, :named_routes, :default_scope, :router @@ -421,7 +454,7 @@ module ActionDispatch end conditions.keep_if do |k, _| - k == :action || k == :controller || + k == :action || k == :controller || k == :required_defaults || @request_class.public_method_defined?(k) || path_values.include?(k) end end @@ -527,12 +560,10 @@ module ActionDispatch recall[:action] = options.delete(:action) if options[:action] == 'index' end - # Generates a path from routes, returns [path, params] - # if no path is returned the formatter will raise Journey::Router::RoutingError + # Generates a path from routes, returns [path, params]. + # If no route is generated the formatter will raise ActionController::UrlGenerationError def generate @set.formatter.generate(:path_info, named_route, options, recall, PARAMETERIZE) - rescue Journey::Router::RoutingError => e - raise ActionController::UrlGenerationError, "No route matches #{options.inspect} #{e.message}" end def different_controller? diff --git a/actionpack/lib/action_dispatch/routing/routes_proxy.rb b/actionpack/lib/action_dispatch/routing/routes_proxy.rb index 73af5920ed..e2393d3799 100644 --- a/actionpack/lib/action_dispatch/routing/routes_proxy.rb +++ b/actionpack/lib/action_dispatch/routing/routes_proxy.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/array/extract_options' + module ActionDispatch module Routing class RoutesProxy #:nodoc: diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 79dff7d121..9210bffd1d 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -1,5 +1,6 @@ require 'uri' require 'active_support/core_ext/hash/indifferent_access' +require 'active_support/core_ext/string/access' require 'action_controller/metal/exceptions' module ActionDispatch @@ -208,11 +209,9 @@ module ActionDispatch end def fail_on(exception_class) - begin - yield - rescue exception_class => e - raise MiniTest::Assertion, e.message - end + yield + rescue exception_class => e + raise MiniTest::Assertion, e.message end end end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 1fc5933e98..ed4e88aab6 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -273,7 +273,7 @@ module ActionDispatch if path =~ %r{://} location = URI.parse(path) https! URI::HTTPS === location if location.scheme - host! location.host if location.host + host! "#{location.host}:#{location.port}" if location.host path = location.query ? "#{location.path}?#{location.query}" : location.path end diff --git a/actionpack/lib/action_dispatch/testing/performance_test.rb b/actionpack/lib/action_dispatch/testing/performance_test.rb deleted file mode 100644 index 13fe693c32..0000000000 --- a/actionpack/lib/action_dispatch/testing/performance_test.rb +++ /dev/null @@ -1,10 +0,0 @@ -require 'active_support/testing/performance' - -module ActionDispatch - # An integration test that runs a code profiler on your test methods. - # Profiling output for combinations of each test method, measurement, and - # output format are written to your tmp/performance directory. - class PerformanceTest < ActionDispatch::IntegrationTest - include ActiveSupport::Testing::Performance - end -end diff --git a/actionpack/lib/action_pack/version.rb b/actionpack/lib/action_pack/version.rb index 94cbc8e478..5c87a9cd7c 100644 --- a/actionpack/lib/action_pack/version.rb +++ b/actionpack/lib/action_pack/version.rb @@ -3,7 +3,7 @@ module ActionPack MAJOR = 4 MINOR = 0 TINY = 0 - PRE = "beta" + PRE = "beta1" STRING = [MAJOR, MINOR, TINY, PRE].compact.join('.') end diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 668515df59..08253de3f4 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -6,7 +6,7 @@ require 'action_view/log_subscriber' module ActionView #:nodoc: # = Action View Base # - # Action View templates can be written in several ways. If the template file has a <tt>.erb</tt> extension then it uses a mixture of ERb + # Action View templates can be written in several ways. If the template file has a <tt>.erb</tt> extension then it uses a mixture of ERB # (included in Ruby) and HTML. If the template file has a <tt>.builder</tt> extension then Jim Weirich's Builder::XmlMarkup library is used. # # == ERB diff --git a/actionpack/lib/action_view/dependency_tracker.rb b/actionpack/lib/action_view/dependency_tracker.rb new file mode 100644 index 0000000000..a2a555dfcb --- /dev/null +++ b/actionpack/lib/action_view/dependency_tracker.rb @@ -0,0 +1,93 @@ +require 'thread_safe' + +module ActionView + class DependencyTracker + @trackers = ThreadSafe::Cache.new + + def self.find_dependencies(name, template) + tracker = @trackers[template.handler] + + if tracker.present? + tracker.call(name, template) + else + [] + end + end + + def self.register_tracker(extension, tracker) + handler = Template.handler_for_extension(extension) + @trackers[handler] = tracker + end + + def self.remove_tracker(handler) + @trackers.delete(handler) + end + + class ERBTracker + EXPLICIT_DEPENDENCY = /# Template Dependency: (\S+)/ + + # Matches: + # render partial: "comments/comment", collection: commentable.comments + # render "comments/comments" + # render 'comments/comments' + # render('comments/comments') + # + # render(@topic) => render("topics/topic") + # render(topics) => render("topics/topic") + # render(message.topics) => render("topics/topic") + RENDER_DEPENDENCY = / + render\s* # render, followed by optional whitespace + \(? # start an optional parenthesis for the render call + (partial:|:partial\s+=>)?\s* # naming the partial, used with collection -- 1st capture + ([@a-z"'][@a-z_\/\."']+) # the template name itself -- 2nd capture + /x + + def self.call(name, template) + new(name, template).dependencies + end + + def initialize(name, template) + @name, @template = name, template + end + + def dependencies + render_dependencies + explicit_dependencies + end + + attr_reader :name, :template + private :name, :template + + private + + def source + template.source + end + + def directory + name.split("/")[0..-2].join("/") + end + + def render_dependencies + source.scan(RENDER_DEPENDENCY). + collect(&:second).uniq. + + # render(@topic) => render("topics/topic") + # render(topics) => render("topics/topic") + # render(message.topics) => render("topics/topic") + collect { |name| name.sub(/\A@?([a-z]+\.)*([a-z_]+)\z/) { "#{$2.pluralize}/#{$2.singularize}" } }. + + # render("headline") => render("message/headline") + collect { |name| name.include?("/") ? name : "#{directory}/#{name}" }. + + # replace quotes from string renders + collect { |name| name.gsub(/["']/, "") } + end + + def explicit_dependencies + source.scan(EXPLICIT_DEPENDENCY).flatten.uniq + end + end + + register_tracker :erb, ERBTracker + end +end diff --git a/actionpack/lib/action_view/digestor.rb b/actionpack/lib/action_view/digestor.rb index f3f6b425a8..9324a1ac50 100644 --- a/actionpack/lib/action_view/digestor.rb +++ b/actionpack/lib/action_view/digestor.rb @@ -1,39 +1,23 @@ require 'thread_safe' +require 'action_view/dependency_tracker' module ActionView class Digestor - EXPLICIT_DEPENDENCY = /# Template Dependency: (\S+)/ - - # Matches: - # render partial: "comments/comment", collection: commentable.comments - # render "comments/comments" - # render 'comments/comments' - # render('comments/comments') - # - # render(@topic) => render("topics/topic") - # render(topics) => render("topics/topic") - # render(message.topics) => render("topics/topic") - RENDER_DEPENDENCY = / - render\s* # render, followed by optional whitespace - \(? # start an optional parenthesis for the render call - (partial:|:partial\s+=>)?\s* # naming the partial, used with collection -- 1st capture - ([@a-z"'][@a-z_\/\."']+) # the template name itself -- 2nd capture - /x - cattr_reader(:cache) @@cache = ThreadSafe::Cache.new def self.digest(name, format, finder, options = {}) - @@cache["#{name}.#{format}"] ||= begin + cache_key = [name, format] + Array.wrap(options[:dependencies]) + @@cache[cache_key.join('.')] ||= begin klass = options[:partial] || name.include?("/_") ? PartialDigestor : Digestor - klass.new(name, format, finder).digest + klass.new(name, format, finder, options).digest end end - attr_reader :name, :format, :finder + attr_reader :name, :format, :finder, :options - def initialize(name, format, finder) - @name, @format, @finder = name, format, finder + def initialize(name, format, finder, options={}) + @name, @format, @finder, @options = name, format, finder, options end def digest @@ -46,7 +30,7 @@ module ActionView end def dependencies - render_dependencies + explicit_dependencies + DependencyTracker.find_dependencies(name, template) rescue ActionView::MissingTemplate [] # File doesn't exist, so no dependencies end @@ -68,42 +52,28 @@ module ActionView name.gsub(%r|/_|, "/") end - def directory - name.split("/")[0..-2].join("/") - end - def partial? false end + def template + @template ||= finder.find(logical_name, [], partial?, formats: [ format ]) + end + def source - @source ||= finder.find(logical_name, [], partial?, formats: [ format ]).source + template.source end def dependency_digest - dependencies.collect do |template_name| + template_digests = dependencies.collect do |template_name| Digestor.digest(template_name, format, finder, partial: true) - end.join("-") - end - - def render_dependencies - source.scan(RENDER_DEPENDENCY). - collect(&:second).uniq. - - # render(@topic) => render("topics/topic") - # render(topics) => render("topics/topic") - # render(message.topics) => render("topics/topic") - collect { |name| name.sub(/\A@?([a-z]+\.)*([a-z_]+)\z/) { "#{$2.pluralize}/#{$2.singularize}" } }. - - # render("headline") => render("message/headline") - collect { |name| name.include?("/") ? name : "#{directory}/#{name}" }. + end - # replace quotes from string renders - collect { |name| name.gsub(/["']/, "") } + (template_digests + injected_dependencies).join("-") end - def explicit_dependencies - source.scan(EXPLICIT_DEPENDENCY).flatten.uniq + def injected_dependencies + Array.wrap(options[:dependencies]) end end diff --git a/actionpack/lib/action_view/helpers.rb b/actionpack/lib/action_view/helpers.rb index 269e78a021..8a78685ae1 100644 --- a/actionpack/lib/action_view/helpers.rb +++ b/actionpack/lib/action_view/helpers.rb @@ -1,3 +1,5 @@ +require 'active_support/benchmarkable' + module ActionView #:nodoc: module Helpers #:nodoc: extend ActiveSupport::Autoload @@ -6,7 +8,6 @@ module ActionView #:nodoc: autoload :AssetTagHelper autoload :AssetUrlHelper autoload :AtomFeedHelper - autoload :BenchmarkHelper autoload :CacheHelper autoload :CaptureHelper autoload :ControllerHelper @@ -29,11 +30,11 @@ module ActionView #:nodoc: extend ActiveSupport::Concern + include ActiveSupport::Benchmarkable include ActiveModelHelper include AssetTagHelper include AssetUrlHelper include AtomFeedHelper - include BenchmarkHelper include CacheHelper include CaptureHelper include ControllerHelper diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index 11743e36f2..31e37893c6 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -182,6 +182,8 @@ module ActionView # width="30" and height="45", and "50" becomes width="50" and height="50". # <tt>:size</tt> will be ignored if the value is not in the correct format. # + # ==== Examples + # # image_tag("icon") # # => <img alt="Icon" src="/assets/icon" /> # image_tag("icon.png") @@ -212,10 +214,24 @@ module ActionView end # Returns a string suitable for an html image tag alt attribute. - # +src+ is meant to be an image file path. - # It removes the basename of the file path and the digest, if any. + # The +src+ argument is meant to be an image file path. + # The method removes the basename of the file path and the digest, + # if any. It also removes hyphens and underscores from file names and + # replaces them with spaces, returning a space-separated, titleized + # string. + # + # ==== Examples + # + # image_tag('rails.png') + # # => <img alt="Rails" src="/assets/rails.png" /> + # + # image_tag('hyphenated-file-name.png') + # # => <img alt="Hyphenated file name" src="/assets/hyphenated-file-name.png" /> + # + # image_tag('underscored_file_name.png') + # # => <img alt="Underscored file name" src="/assets/underscored_file_name.png" /> def image_alt(src) - File.basename(src, '.*').sub(/-[[:xdigit:]]{32}\z/, '').capitalize + File.basename(src, '.*').sub(/-[[:xdigit:]]{32}\z/, '').tr('-_', ' ').capitalize end # Returns an html video tag for the +sources+. If +sources+ is a string, diff --git a/actionpack/lib/action_view/helpers/asset_url_helper.rb b/actionpack/lib/action_view/helpers/asset_url_helper.rb index 0affac41e8..71b78cf0b5 100644 --- a/actionpack/lib/action_view/helpers/asset_url_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_url_helper.rb @@ -132,8 +132,7 @@ module ActionView source = compute_asset_path(source, options) end - relative_url_root = (defined?(config.relative_url_root) && config.relative_url_root) || - (respond_to?(:request) && request.try(:script_name)) + relative_url_root = defined?(config.relative_url_root) && config.relative_url_root if relative_url_root source = "#{relative_url_root}#{source}" unless source.starts_with?("#{relative_url_root}/") end diff --git a/actionpack/lib/action_view/helpers/benchmark_helper.rb b/actionpack/lib/action_view/helpers/benchmark_helper.rb deleted file mode 100644 index 87fbf8f1a8..0000000000 --- a/actionpack/lib/action_view/helpers/benchmark_helper.rb +++ /dev/null @@ -1,13 +0,0 @@ -require 'active_support/benchmarkable' - -module ActionView - module Helpers - module BenchmarkHelper #:nodoc: - include ActiveSupport::Benchmarkable - - def benchmark(*) - capture { super } - end - end - end -end diff --git a/actionpack/lib/action_view/helpers/cache_helper.rb b/actionpack/lib/action_view/helpers/cache_helper.rb index 995aa10afb..8fc78ea7fb 100644 --- a/actionpack/lib/action_view/helpers/cache_helper.rb +++ b/actionpack/lib/action_view/helpers/cache_helper.rb @@ -167,7 +167,7 @@ module ActionView if @virtual_path [ *Array(name.is_a?(Hash) ? controller.url_for(name).split("://").last : name), - Digestor.digest(@virtual_path, formats.last.to_sym, lookup_context) + Digestor.digest(@virtual_path, formats.last.to_sym, lookup_context, dependencies: view_cache_dependencies) ] else name diff --git a/actionpack/lib/action_view/helpers/capture_helper.rb b/actionpack/lib/action_view/helpers/capture_helper.rb index 4ec860d69a..1bad82159a 100644 --- a/actionpack/lib/action_view/helpers/capture_helper.rb +++ b/actionpack/lib/action_view/helpers/capture_helper.rb @@ -156,7 +156,7 @@ module ActionView end nil else - @view_flow.get(name) + @view_flow.get(name).presence end end diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb index 1fbf61a5a9..d3953c26b7 100644 --- a/actionpack/lib/action_view/helpers/date_helper.rb +++ b/actionpack/lib/action_view/helpers/date_helper.rb @@ -1,5 +1,6 @@ require 'date' require 'action_view/helpers/tag_helper' +require 'active_support/core_ext/array/extract_options' require 'active_support/core_ext/date/conversions' require 'active_support/core_ext/hash/slice' require 'active_support/core_ext/object/with_options' @@ -12,7 +13,7 @@ module ActionView # elements. All of the select-type methods share a number of common options that are as follows: # # * <tt>:prefix</tt> - overwrites the default prefix of "date" used for the select names. So specifying "birthday" - # would give \birthday[month] instead of \date[month] if passed to the <tt>select_month</tt> method. + # would give \birthday[month] instead of \date[month] if passed to the <tt>select_month</tt> method. # * <tt>:include_blank</tt> - set to true if it should be possible to set an empty date. # * <tt>:discard_type</tt> - set to true if you want to discard the type part of the select name. If set to true, # the <tt>select_month</tt> method would use simply "date" (which can be overwritten using <tt>:prefix</tt>) instead @@ -193,6 +194,7 @@ module ActionView # * <tt>:include_blank</tt> - Include a blank option in every select field so it's possible to set empty # dates. # * <tt>:default</tt> - Set a default date if the affected date isn't set or is nil. + # * <tt>:selected</tt> - Set a date that overrides the actual value. # * <tt>:disabled</tt> - Set to true if you want show the select fields as disabled. # * <tt>:prompt</tt> - Set to true (for a generic prompt), a prompt string or a hash of prompt strings # for <tt>:year</tt>, <tt>:month</tt>, <tt>:day</tt>, <tt>:hour</tt>, <tt>:minute</tt> and <tt>:second</tt>. @@ -234,6 +236,10 @@ module ActionView # # which is initially set to the date 3 days from the current date # date_select("article", "written_on", default: 3.days.from_now) # + # # Generates a date select that when POSTed is stored in the article variable, in the written_on attribute + # # which is set in the form with todays date, regardless of the value in the Active Record object. + # date_select("article", "written_on", selected: Date.today) + # # # Generates a date select that when POSTed is stored in the credit_card variable, in the bill_due attribute # # that will have a default day of 20. # date_select("credit_card", "bill_due", default: { day: 20 }) @@ -636,6 +642,8 @@ module ActionView # <time datetime="2010-11-03">Yesterday</time> # time_tag Date.today, pubdate: true # => # <time datetime="2010-11-04" pubdate="pubdate">November 04, 2010</time> + # time_tag Date.today, datetime: Date.today.strftime('%G-W%V') # => + # <time datetime="2010-W44">November 04, 2010</time> # # <%= time_tag Time.now do %> # <span>Right now</span> @@ -645,7 +653,7 @@ module ActionView options = args.extract_options! format = options.delete(:format) || :long content = args.first || I18n.l(date_or_time, :format => format) - datetime = date_or_time.acts_like?(:time) ? date_or_time.xmlschema : date_or_time.rfc3339 + datetime = date_or_time.acts_like?(:time) ? date_or_time.xmlschema : date_or_time.iso8601 content_tag(:time, content, options.reverse_merge(:datetime => datetime), &block) end @@ -875,6 +883,7 @@ module ActionView def translated_date_order date_order = I18n.translate(:'date.order', :locale => @options[:locale], :default => []) + date_order = date_order.map { |element| element.to_sym } forbidden_elements = date_order - [:year, :month, :day] if forbidden_elements.any? @@ -1034,14 +1043,38 @@ module ActionView end class FormBuilder + # Wraps ActionView::Helpers::DateHelper#date_select for form builders: + # + # <%= form_for @person do |f| %> + # <%= f.date_select :birth_date %> + # <%= f.submit %> + # <% end %> + # + # Please refer to the documentation of the base helper for details. def date_select(method, options = {}, html_options = {}) @template.date_select(@object_name, method, objectify_options(options), html_options) end + # Wraps ActionView::Helpers::DateHelper#time_select for form builders: + # + # <%= form_for @race do |f| %> + # <%= f.time_select :average_lap %> + # <%= f.submit %> + # <% end %> + # + # Please refer to the documentation of the base helper for details. def time_select(method, options = {}, html_options = {}) @template.time_select(@object_name, method, objectify_options(options), html_options) end + # Wraps ActionView::Helpers::DateHelper#datetime_select for form builders: + # + # <%= form_for @person do |f| %> + # <%= f.time_select :last_request_at %> + # <%= f.submit %> + # <% end %> + # + # Please refer to the documentation of the base helper for details. def datetime_select(method, options = {}, html_options = {}) @template.datetime_select(@object_name, method, objectify_options(options), html_options) end diff --git a/actionpack/lib/action_view/helpers/debug_helper.rb b/actionpack/lib/action_view/helpers/debug_helper.rb index d361a69a92..c29c1b1eea 100644 --- a/actionpack/lib/action_view/helpers/debug_helper.rb +++ b/actionpack/lib/action_view/helpers/debug_helper.rb @@ -27,14 +27,12 @@ module ActionView # new_record: true # </pre> def debug(object) - begin - Marshal::dump(object) - object = ERB::Util.html_escape(object.to_yaml).gsub(" ", " ").html_safe - content_tag(:pre, object, :class => "debug_dump") - rescue Exception # errors from Marshal or YAML - # Object couldn't be dumped, perhaps because of singleton methods -- this is the fallback - content_tag(:code, object.to_yaml, :class => "debug_dump") - end + Marshal::dump(object) + object = ERB::Util.html_escape(object.to_yaml).gsub(" ", " ").html_safe + content_tag(:pre, object, :class => "debug_dump") + rescue Exception # errors from Marshal or YAML + # Object couldn't be dumped, perhaps because of singleton methods -- this is the fallback + content_tag(:code, object.inspect, :class => "debug_dump") end end end diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 516492ca30..3dae1fc87a 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -8,7 +8,6 @@ require 'action_view/model_naming' require 'active_support/core_ext/class/attribute_accessors' require 'active_support/core_ext/hash/slice' require 'active_support/core_ext/string/output_safety' -require 'active_support/core_ext/array/extract_options' require 'active_support/core_ext/string/inflections' module ActionView @@ -84,7 +83,7 @@ module ActionView # # <form action="/people/256" class="edit_person" id="edit_person_256" method="post"> # <div style="margin:0;padding:0;display:inline"> - # <input name="_method" type="hidden" value="put" /> + # <input name="_method" type="hidden" value="patch" /> # <input name="authenticity_token" type="hidden" value="NrOp5bsjoLRuK8IW5+dQEYjKGUJDe7TQoZVvq95Wteg=" /> # </div> # <label for="person_first_name">First name</label>: @@ -101,9 +100,9 @@ module ActionView # and generate HTML accordingly. # # The controller would receive the form data again in <tt>params[:person]</tt>, ready to be - # passed to <tt>Person#update_attributes</tt>: + # passed to <tt>Person#update</tt>: # - # if @person.update_attributes(params[:person]) + # if @person.update(params[:person]) # # success # else # # error handling @@ -242,7 +241,7 @@ module ActionView # # is then equivalent to something like: # - # <%= form_for @post, as: :post, url: post_path(@post), method: :put, html: { class: "edit_post", id: "edit_post_45" } do |f| %> + # <%= form_for @post, as: :post, url: post_path(@post), method: :patch, html: { class: "edit_post", id: "edit_post_45" } do |f| %> # ... # <% end %> # @@ -318,7 +317,7 @@ module ActionView # # <form action='http://www.example.com' method='post' data-remote='true'> # <div style='margin:0;padding:0;display:inline'> - # <input name='_method' type='hidden' value='put' /> + # <input name='_method' type='hidden' value='patch' /> # </div> # ... # </form> @@ -336,7 +335,7 @@ module ActionView # # <form action='http://www.example.com' method='post' data-behavior='autosave' name='go'> # <div style='margin:0;padding:0;display:inline'> - # <input name='_method' type='hidden' value='put' /> + # <input name='_method' type='hidden' value='patch' /> # </div> # ... # </form> @@ -388,9 +387,9 @@ module ActionView # In many cases you will want to wrap the above in another helper, so you # could do something like the following: # - # def labelled_form_for(record_or_name_or_array, *args, &proc) + # def labelled_form_for(record_or_name_or_array, *args, &block) # options = args.extract_options! - # form_for(record_or_name_or_array, *(args << options.merge(builder: LabellingFormBuilder)), &proc) + # form_for(record_or_name_or_array, *(args << options.merge(builder: LabellingFormBuilder)), &block) # end # # If you don't need to attach a form to a model instance, then check out @@ -412,10 +411,9 @@ module ActionView # <%= form_for @invoice, url: external_url, authenticity_token: false do |f| # ... # <% end %> - def form_for(record, options = {}, &proc) + def form_for(record, options = {}, &block) raise ArgumentError, "Missing block" unless block_given? - - options[:html] ||= {} + html_options = options[:html] ||= {} case record when String, Symbol @@ -428,17 +426,16 @@ module ActionView apply_form_for_options!(record, object, options) end - options[:html][:data] = options.delete(:data) if options.has_key?(:data) - options[:html][:remote] = options.delete(:remote) if options.has_key?(:remote) - options[:html][:method] = options.delete(:method) if options.has_key?(:method) - options[:html][:authenticity_token] = options.delete(:authenticity_token) + html_options[:data] = options.delete(:data) if options.has_key?(:data) + html_options[:remote] = options.delete(:remote) if options.has_key?(:remote) + html_options[:method] = options.delete(:method) if options.has_key?(:method) + html_options[:authenticity_token] = options.delete(:authenticity_token) - builder = options[:parent_builder] = instantiate_builder(object_name, object, options) - fields_for = fields_for(object_name, object, options, &proc) - default_options = builder.multipart? ? { multipart: true } : {} - default_options.merge!(options.delete(:html)) + builder = instantiate_builder(object_name, object, options) + output = capture(builder, &block) + html_options[:multipart] = builder.multipart? - form_tag(options.delete(:url) || {}, default_options) { fields_for } + form_tag(options[:url] || {}, html_options) { output } end def apply_form_for_options!(record, object, options) #:nodoc: @@ -707,9 +704,7 @@ module ActionView # to prevent fields_for from rendering it automatically. def fields_for(record_name, record_object = nil, options = {}, &block) builder = instantiate_builder(record_name, record_object, options) - output = capture(builder, &block) - output.concat builder.hidden_field(:id) if output && options[:hidden_field_id] && !builder.emitted_hidden_id? - output + capture(builder, &block) end # Returns a label tag tailored for labelling an input field for a specified attribute (identified by +method+) on an object @@ -775,8 +770,8 @@ module ActionView # text_field(:post, :title, class: "create_input") # # => <input type="text" id="post_title" name="post[title]" value="#{@post.title}" class="create_input" /> # - # text_field(:session, :user, onchange: "if $('#session_user').value == 'admin' { alert('Your login can not be admin!'); }") - # # => <input type="text" id="session_user" name="session[user]" value="#{@session.user}" onchange = "if $('#session_user').value == 'admin' { alert('Your login can not be admin!'); }"/> + # text_field(:session, :user, onchange: "if ($('#session_user').val() === 'admin') { alert('Your login can not be admin!'); }") + # # => <input type="text" id="session_user" name="session[user]" value="#{@session.user}" onchange="if ($('#session_user').val() === 'admin') { alert('Your login can not be admin!'); }"/> # # text_field(:snippet, :code, size: 20, class: 'code_input') # # => <input type="text" id="snippet_code" name="snippet[code]" size="20" value="#{@snippet.code}" class="code_input" /> @@ -796,8 +791,8 @@ module ActionView # password_field(:account, :secret, class: "form_input", value: @account.secret) # # => <input type="password" id="account_secret" name="account[secret]" value="#{@account.secret}" class="form_input" /> # - # password_field(:user, :password, onchange: "if $('user[password]').length > 30 { alert('Your password needs to be shorter!'); }") - # # => <input type="password" id="user_password" name="user[password]" onchange = "if $('user[password]').length > 30 { alert('Your password needs to be shorter!'); }"/> + # password_field(:user, :password, onchange: "if ($('#user_password').val().length > 30) { alert('Your password needs to be shorter!'); }") + # # => <input type="password" id="user_password" name="user[password]" onchange="if ($('#user_password').val().length > 30) { alert('Your password needs to be shorter!'); }"/> # # password_field(:account, :pin, size: 20, class: 'form_input') # # => <input type="password" id="account_pin" name="account[pin]" size="20" class="form_input" /> @@ -897,7 +892,7 @@ module ActionView # invoice the user unchecks its check box, no +paid+ parameter is sent. So, # any mass-assignment idiom like # - # @invoice.update_attributes(params[:invoice]) + # @invoice.update(params[:invoice]) # # wouldn't update the flag. # @@ -1174,12 +1169,15 @@ module ActionView attr_accessor :object_name, :object, :options - attr_reader :multipart, :parent_builder, :index + attr_reader :multipart, :index alias :multipart? :multipart def multipart=(multipart) @multipart = multipart - parent_builder.multipart = multipart if parent_builder + + if parent_builder = @options[:parent_builder] + parent_builder.multipart = multipart + end end def self._to_partial_path @@ -1201,7 +1199,6 @@ module ActionView @nested_child_index = {} @object_name, @object, @template, @options = object_name, object, template, options - @parent_builder = options[:parent_builder] @default_options = @options ? @options.slice(:index, :namespace) : {} if @object_name.to_s.match(/\[\]$/) if object ||= @template.instance_variable_get("@#{Regexp.last_match.pre_match}") and object.respond_to?(:to_param) @@ -1478,8 +1475,8 @@ module ActionView def fields_for(record_name, record_object = nil, fields_options = {}, &block) fields_options, record_object = record_object, nil if record_object.is_a?(Hash) && record_object.extractable_options? fields_options[:builder] ||= options[:builder] - fields_options[:parent_builder] = self fields_options[:namespace] = options[:namespace] + fields_options[:parent_builder] = self case record_name when String, Symbol @@ -1569,7 +1566,7 @@ module ActionView # invoice the user unchecks its check box, no +paid+ parameter is sent. So, # any mass-assignment idiom like # - # @invoice.update_attributes(params[:invoice]) + # @invoice.update(params[:invoice]) # # wouldn't update the flag. # @@ -1802,7 +1799,7 @@ module ActionView association = convert_to_model(association) if association.respond_to?(:persisted?) - association = [association] if @object.send(association_name).is_a?(Array) + association = [association] if @object.send(association_name).respond_to?(:to_ary) elsif !association.respond_to?(:to_ary) association = @object.send(association_name) end @@ -1820,13 +1817,17 @@ module ActionView end end - def fields_for_nested_model(name, object, options, block) + def fields_for_nested_model(name, object, fields_options, block) object = convert_to_model(object) + emit_hidden_id = object.persisted? && fields_options.fetch(:include_id) { + options.fetch(:include_id, true) + } - parent_include_id = self.options.fetch(:include_id, true) - include_id = options.fetch(:include_id, parent_include_id) - options[:hidden_field_id] = object.persisted? && include_id - @template.fields_for(name, object, options, &block) + @template.fields_for(name, object, fields_options) do |f| + output = @template.capture(f, &block) + output.concat f.hidden_field(:id) if output && emit_hidden_id && !f.emitted_hidden_id? + output + end end def nested_child_index(name) diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb index 1b5b788a35..377819a80c 100644 --- a/actionpack/lib/action_view/helpers/form_options_helper.rb +++ b/actionpack/lib/action_view/helpers/form_options_helper.rb @@ -2,6 +2,7 @@ require 'cgi' require 'erb' require 'action_view/helpers/form_helper' require 'active_support/core_ext/string/output_safety' +require 'active_support/core_ext/array/extract_options' require 'active_support/core_ext/array/wrap' module ActionView @@ -130,7 +131,7 @@ module ActionView # the user deselects all roles from +role_ids+ multiple select box, no +role_ids+ parameter is sent. So, # any mass-assignment idiom like # - # @user.update_attributes(params[:user]) + # @user.update(params[:user]) # # wouldn't update roles. # @@ -564,14 +565,14 @@ module ActionView if priority_zones if priority_zones.is_a?(Regexp) - priority_zones = zones.select { |z| z =~ priority_zones } + priority_zones = zones.grep(priority_zones) end zone_options.safe_concat options_for_select(convert_zones[priority_zones], selected) zone_options.safe_concat content_tag(:option, '-------------', :value => '', :disabled => 'disabled') zone_options.safe_concat "\n" - zones.reject! { |z| priority_zones.include?(z) } + zones = zones - priority_zones end zone_options.safe_concat options_for_select(convert_zones[zones], selected) @@ -756,28 +757,76 @@ module ActionView end class FormBuilder + # Wraps ActionView::Helpers::FormOptionsHelper#select for form builders: + # + # <%= form_for @post do |f| %> + # <%= f.select :person_id, Person.all.collect {|p| [ p.name, p.id ] }, { include_blank: true }) %> + # <%= f.submit %> + # <% end %> + # + # Please refer to the documentation of the base helper for details. def select(method, choices, options = {}, html_options = {}) @template.select(@object_name, method, choices, objectify_options(options), @default_options.merge(html_options)) end + # Wraps ActionView::Helpers::FormOptionsHelper#collection_select for form builders: + # + # <%= form_for @post do |f| %> + # <%= f.collection_select :person_id, Author.all, :id, :name_with_initial, prompt: true %> + # <%= f.submit %> + # <% end %> + # + # Please refer to the documentation of the base helper for details. def collection_select(method, collection, value_method, text_method, options = {}, html_options = {}) @template.collection_select(@object_name, method, collection, value_method, text_method, objectify_options(options), @default_options.merge(html_options)) end + # Wraps ActionView::Helpers::FormOptionsHelper#grouped_collection_select for form builders: + # + # <%= form_for @city do |f| %> + # <%= f.grouped_collection_select :country_id, :country_id, @continents, :countries, :name, :id, :name %> + # <%= f.submit %> + # <% end %> + # + # Please refer to the documentation of the base helper for details. def grouped_collection_select(method, collection, group_method, group_label_method, option_key_method, option_value_method, options = {}, html_options = {}) @template.grouped_collection_select(@object_name, method, collection, group_method, group_label_method, option_key_method, option_value_method, objectify_options(options), @default_options.merge(html_options)) end + # Wraps ActionView::Helpers::FormOptionsHelper#time_zone_select for form builders: + # + # <%= form_for @user do |f| %> + # <%= f.time_zone_select :time_zone, nil, include_blank: true %> + # <%= f.submit %> + # <% end %> + # + # Please refer to the documentation of the base helper for details. def time_zone_select(method, priority_zones = nil, options = {}, html_options = {}) @template.time_zone_select(@object_name, method, priority_zones, objectify_options(options), @default_options.merge(html_options)) end - def collection_check_boxes(method, collection, value_method, text_method, options = {}, html_options = {}) - @template.collection_check_boxes(@object_name, method, collection, value_method, text_method, objectify_options(options), @default_options.merge(html_options)) + # Wraps ActionView::Helpers::FormOptionsHelper#collection_check_boxes for form builders: + # + # <%= form_for @post do |f| %> + # <%= f.collection_check_boxes :author_ids, Author.all, :id, :name_with_initial %> + # <%= f.submit %> + # <% end %> + # + # Please refer to the documentation of the base helper for details. + def collection_check_boxes(method, collection, value_method, text_method, options = {}, html_options = {}, &block) + @template.collection_check_boxes(@object_name, method, collection, value_method, text_method, objectify_options(options), @default_options.merge(html_options), &block) end - def collection_radio_buttons(method, collection, value_method, text_method, options = {}, html_options = {}) - @template.collection_radio_buttons(@object_name, method, collection, value_method, text_method, objectify_options(options), @default_options.merge(html_options)) + # Wraps ActionView::Helpers::FormOptionsHelper#collection_radio_buttons for form builders: + # + # <%= form_for @post do |f| %> + # <%= f.collection_radio_buttons :author_id, Author.all, :id, :name_with_initial %> + # <%= f.submit %> + # <% end %> + # + # Please refer to the documentation of the base helper for details. + def collection_radio_buttons(method, collection, value_method, text_method, options = {}, html_options = {}, &block) + @template.collection_radio_buttons(@object_name, method, collection, value_method, text_method, objectify_options(options), @default_options.merge(html_options), &block) end end end diff --git a/actionpack/lib/action_view/helpers/form_tag_helper.rb b/actionpack/lib/action_view/helpers/form_tag_helper.rb index ff83ef3ca1..8abd5d6e9c 100644 --- a/actionpack/lib/action_view/helpers/form_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/form_tag_helper.rb @@ -26,14 +26,14 @@ module ActionView # ==== Options # * <tt>:multipart</tt> - If set to true, the enctype is set to "multipart/form-data". # * <tt>:method</tt> - The method to use when submitting the form, usually either "get" or "post". - # If "put", "delete", or another verb is used, a hidden input with name <tt>_method</tt> + # If "patch", "put", "delete", or another verb is used, a hidden input with name <tt>_method</tt> # is added to simulate the verb over post. # * <tt>:authenticity_token</tt> - Authenticity token to use in the form. Use only if you need to # pass custom authenticity token string, or to not add authenticity_token field at all # (by passing <tt>false</tt>). Remote forms may omit the embedded authenticity token # by setting <tt>config.action_view.embed_authenticity_token_in_remote_forms = false</tt>. # This is helpful when you're fragment-caching the form. Remote forms get the - # authenticity from the <tt>meta</tt> tag, so embedding is unnecessary unless you + # authenticity token from the <tt>meta</tt> tag, so embedding is unnecessary unless you # support browsers without JavaScript. # * A list of parameters to feed to the URL the form will be posted to. # * <tt>:remote</tt> - If set to true, will allow the Unobtrusive JavaScript drivers to control the @@ -184,7 +184,7 @@ module ActionView # # => <label for="name">Name</label> # # label_tag 'name', 'Your name' - # # => <label for="name">Your Name</label> + # # => <label for="name">Your name</label> # # label_tag 'name', nil, class: 'small_label' # # => <label for="name" class="small_label">Name</label> @@ -526,19 +526,19 @@ module ActionView # # ==== Examples # image_submit_tag("login.png") - # # => <input src="/images/login.png" type="image" /> + # # => <input alt="Login" src="/images/login.png" type="image" /> # # image_submit_tag("purchase.png", disabled: true) - # # => <input disabled="disabled" src="/images/purchase.png" type="image" /> + # # => <input alt="Purchase" disabled="disabled" src="/images/purchase.png" type="image" /> # - # image_submit_tag("search.png", class: 'search_button') - # # => <input class="search_button" src="/images/search.png" type="image" /> + # image_submit_tag("search.png", class: 'search_button', alt: 'Find') + # # => <input alt="Find" class="search_button" src="/images/search.png" type="image" /> # # image_submit_tag("agree.png", disabled: true, class: "agree_disagree_button") - # # => <input class="agree_disagree_button" disabled="disabled" src="/images/agree.png" type="image" /> + # # => <input alt="Agree" class="agree_disagree_button" disabled="disabled" src="/images/agree.png" type="image" /> # # image_submit_tag("save.png", data: { confirm: "Are you sure?" }) - # # => <input src="/images/save.png" data-confirm="Are you sure?" type="image" /> + # # => <input alt="Save" src="/images/save.png" data-confirm="Are you sure?" type="image" /> def image_submit_tag(source, options = {}) options = options.stringify_keys @@ -550,7 +550,7 @@ module ActionView options["data-confirm"] = confirm end - tag :input, { "type" => "image", "src" => path_to_image(source) }.update(options) + tag :input, { "alt" => image_alt(source), "type" => "image", "src" => path_to_image(source) }.update(options) end # Creates a field set for grouping HTML form elements. @@ -778,7 +778,7 @@ module ActionView # see http://www.w3.org/TR/html4/types.html#type-name def sanitize_to_id(name) - name.to_s.gsub(']','').gsub(/[^-a-zA-Z0-9:.]/, "_") + name.to_s.delete(']').gsub(/[^-a-zA-Z0-9:.]/, "_") end end end diff --git a/actionpack/lib/action_view/helpers/javascript_helper.rb b/actionpack/lib/action_view/helpers/javascript_helper.rb index cfdd7c77d8..878d3e0eda 100644 --- a/actionpack/lib/action_view/helpers/javascript_helper.rb +++ b/actionpack/lib/action_view/helpers/javascript_helper.rb @@ -13,8 +13,8 @@ module ActionView "'" => "\\'" } - JS_ESCAPE_MAP["\342\200\250".force_encoding('UTF-8').encode!] = '
' - JS_ESCAPE_MAP["\342\200\251".force_encoding('UTF-8').encode!] = '
' + JS_ESCAPE_MAP["\342\200\250".force_encoding(Encoding::UTF_8).encode!] = '
' + JS_ESCAPE_MAP["\342\200\251".force_encoding(Encoding::UTF_8).encode!] = '
' # Escapes carriage returns and single and double quotes for JavaScript segments. # diff --git a/actionpack/lib/action_view/helpers/record_tag_helper.rb b/actionpack/lib/action_view/helpers/record_tag_helper.rb index 33194250b7..f767957fa9 100644 --- a/actionpack/lib/action_view/helpers/record_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/record_tag_helper.rb @@ -65,8 +65,8 @@ module ActionView # # produces: # - # <tr id="person_123" class="person">...</tr> - # <tr id="person_124" class="person">...</tr> + # <tr id="person_123" class="person">...</tr> + # <tr id="person_124" class="person">...</tr> # # content_tag_for also accepts a hash of options, which will be converted to # additional HTML attributes. If you specify a <tt>:class</tt> value, it will be combined @@ -92,10 +92,14 @@ module ActionView # for each record. def content_tag_for_single_record(tag_name, record, prefix, options, &block) options = options ? options.dup : {} - options[:class] = "#{dom_class(record, prefix)} #{options[:class]}".rstrip + options[:class] = [ dom_class(record, prefix), options[:class] ].compact options[:id] = dom_id(record, prefix) - content_tag(tag_name, capture(record, &block), options) + if block_given? + content_tag(tag_name, capture(record, &block), options) + else + content_tag(tag_name, "", options) + end end end end diff --git a/actionpack/lib/action_view/helpers/tags/collection_check_boxes.rb b/actionpack/lib/action_view/helpers/tags/collection_check_boxes.rb index d27df45b5a..9655008fe2 100644 --- a/actionpack/lib/action_view/helpers/tags/collection_check_boxes.rb +++ b/actionpack/lib/action_view/helpers/tags/collection_check_boxes.rb @@ -13,13 +13,13 @@ module ActionView end end - def render + def render(&block) rendered_collection = render_collection do |item, value, text, default_html_options| default_html_options[:multiple] = true builder = instantiate_builder(CheckBoxBuilder, item, value, text, default_html_options) if block_given? - yield builder + @template_object.capture(builder, &block) else render_component(builder) end diff --git a/actionpack/lib/action_view/helpers/tags/collection_radio_buttons.rb b/actionpack/lib/action_view/helpers/tags/collection_radio_buttons.rb index 81f2ecb2b3..893f4411e7 100644 --- a/actionpack/lib/action_view/helpers/tags/collection_radio_buttons.rb +++ b/actionpack/lib/action_view/helpers/tags/collection_radio_buttons.rb @@ -13,12 +13,12 @@ module ActionView end end - def render + def render(&block) render_collection do |item, value, text, default_html_options| builder = instantiate_builder(RadioButtonBuilder, item, value, text, default_html_options) if block_given? - yield builder + @template_object.capture(builder, &block) else render_component(builder) end diff --git a/actionpack/lib/action_view/helpers/tags/date_select.rb b/actionpack/lib/action_view/helpers/tags/date_select.rb index 6c400f85cb..734591394b 100644 --- a/actionpack/lib/action_view/helpers/tags/date_select.rb +++ b/actionpack/lib/action_view/helpers/tags/date_select.rb @@ -27,7 +27,7 @@ module ActionView end def datetime_selector(options, html_options) - datetime = value(object) || default_datetime(options) + datetime = options.fetch(:selected) { value(object) || default_datetime(options) } @auto_index ||= nil options = options.dup diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index aeee662071..775d93ed39 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -170,7 +170,7 @@ module ActionView # You can also use custom data attributes using the <tt>:data</tt> option: # # link_to "Visit Other Site", "http://www.rubyonrails.org/", data: { confirm: "Are you sure?" } - # # => <a href="http://www.rubyonrails.org/" data-confirm="Are you sure?"">Visit Other Site</a> + # # => <a href="http://www.rubyonrails.org/" data-confirm="Are you sure?">Visit Other Site</a> def link_to(name = nil, options = nil, html_options = nil, &block) html_options, options = options, name if block_given? options ||= {} @@ -241,13 +241,13 @@ module ActionView # # </div> # # </form>" # - # <%= button_to "New", action: "new", form_class: "new-thing" %> + # <%= button_to "New", { action: "new" }, form_class: "new-thing" %> # # => "<form method="post" action="/controller/new" class="new-thing"> # # <div><input value="New" type="submit" /></div> # # </form>" # # - # <%= button_to "Create", action: "create", remote: true, form: { "data-type" => "json" } %> + # <%= button_to "Create", { action: "create" }, remote: true, form: { "data-type" => "json" } %> # # => "<form method="post" action="/images/create" class="button_to" data-remote="true" data-type="json"> # # <div> # # <input value="Create" type="submit" /> @@ -514,7 +514,7 @@ module ActionView "in a #request method" end - return false unless request.get? + return false unless request.get? || request.head? url_string = url_for(options) diff --git a/actionpack/lib/action_view/record_identifier.rb b/actionpack/lib/action_view/record_identifier.rb index 2953654972..63f645431a 100644 --- a/actionpack/lib/action_view/record_identifier.rb +++ b/actionpack/lib/action_view/record_identifier.rb @@ -17,7 +17,7 @@ module ActionView # # controller # def update # post = Post.find(params[:id]) - # post.update_attributes(params[:post]) + # post.update(params[:post]) # # redirect_to(post) # Calls polymorphic_url(post) which in turn calls post_url(post) # end diff --git a/actionpack/lib/action_view/renderer/partial_renderer.rb b/actionpack/lib/action_view/renderer/partial_renderer.rb index 37f93a13fc..43a88b0623 100644 --- a/actionpack/lib/action_view/renderer/partial_renderer.rb +++ b/actionpack/lib/action_view/renderer/partial_renderer.rb @@ -452,7 +452,7 @@ module ActionView def retrieve_template_keys keys = @locals.keys - keys << @variable + keys << @variable if @object || @collection keys << @variable_counter if @collection keys end diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index aefc42be48..720890eeb9 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -1,6 +1,5 @@ require 'active_support/core_ext/object/try' require 'active_support/core_ext/kernel/singleton_class' -require 'active_support/deprecation' require 'thread' module ActionView @@ -81,8 +80,7 @@ module ActionView # problems with converting the user's data to # the <tt>default_internal</tt>. # - # To do so, simply raise the raise +WrongEncodingError+ - # as follows: + # To do so, simply raise +WrongEncodingError+ as follows: # # raise WrongEncodingError.new( # problematic_string, @@ -140,7 +138,7 @@ module ActionView # we use a bang in this instrumentation because you don't want to # consume this in production. This is only slow if it's being listened to. def render(view, locals, buffer=nil, &block) - ActiveSupport::Notifications.instrument("!render_template.action_view", :virtual_path => @virtual_path) do + ActiveSupport::Notifications.instrument("!render_template.action_view", virtual_path: @virtual_path, identifier: @identifier) do compile!(view) view.send(method_name, locals, buffer, &block) end diff --git a/actionpack/lib/action_view/template/error.rb b/actionpack/lib/action_view/template/error.rb index e00056781d..a89d51221e 100644 --- a/actionpack/lib/action_view/template/error.rb +++ b/actionpack/lib/action_view/template/error.rb @@ -17,7 +17,7 @@ module ActionView end def message - @string.force_encoding("BINARY") + @string.force_encoding(Encoding::ASCII_8BIT) "Your template was not saved as valid #{@encoding}. Please " \ "either specify #{@encoding} as the encoding for your template " \ "in your text editor, or mark the template with its " \ @@ -78,7 +78,7 @@ module ActionView end end - def source_extract(indentation = 0) + def source_extract(indentation = 0, output = :console) return unless num = line_number num = num.to_i @@ -88,13 +88,9 @@ module ActionView end_on_line = [ num + SOURCE_CODE_RADIUS - 1, source_code.length].min indent = end_on_line.to_s.size + indentation - line_counter = start_on_line return unless source_code = source_code[start_on_line..end_on_line] - source_code.sum do |line| - line_counter += 1 - "%#{indent}s: %s\n" % [line_counter, line] - end + formatted_code_for(source_code, start_on_line, indent, output) end def sub_template_of(template_path) @@ -123,6 +119,18 @@ module ActionView 'in ' end + file_name end + + def formatted_code_for(source_code, line_counter, indent, output) + start_value = (output == :html) ? {} : "" + source_code.inject(start_value) do |result, line| + line_counter += 1 + if output == :html + result.update(line_counter.to_s => "%#{indent}s %s\n" % ["", line]) + else + result << "%#{indent}s: %s\n" % [line_counter, line] + end + end + end end end diff --git a/actionpack/lib/action_view/template/handlers/erb.rb b/actionpack/lib/action_view/template/handlers/erb.rb index afbbece90f..5aaafc15c1 100644 --- a/actionpack/lib/action_view/template/handlers/erb.rb +++ b/actionpack/lib/action_view/template/handlers/erb.rb @@ -81,7 +81,7 @@ module ActionView # wrong, we can still find an encoding tag # (<%# encoding %>) inside the String using a regular # expression - template_source = template.source.dup.force_encoding("BINARY") + template_source = template.source.dup.force_encoding(Encoding::ASCII_8BIT) erb = template_source.gsub(ENCODING_TAG, '') encoding = $2 diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index 8b23029bbc..47bd011d5e 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -109,7 +109,7 @@ module ActionView @cache.clear end - # Normalizes the arguments and passes it on to find_template. + # Normalizes the arguments and passes it on to find_templates. def find_all(name, prefix=nil, partial=false, details={}, key=nil, locals=[]) cached(key, [name, prefix, partial], details, locals) do find_templates(name, prefix, partial, details) @@ -118,7 +118,7 @@ module ActionView private - delegate :caching?, :to => "self.class" + delegate :caching?, to: :class # This is what child classes implement. No defaults are needed # because Resolver guarantees that the arguments are present and diff --git a/actionpack/lib/action_view/template/types.rb b/actionpack/lib/action_view/template/types.rb index 7611c9e708..db77cb5d19 100644 --- a/actionpack/lib/action_view/template/types.rb +++ b/actionpack/lib/action_view/template/types.rb @@ -1,6 +1,5 @@ require 'set' require 'active_support/core_ext/class/attribute_accessors' -require 'active_support/core_ext/object/blank' module ActionView class Template diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 4479da5bc4..10b487f37a 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -122,7 +122,7 @@ module ActionView class RenderedViewsCollection def initialize - @rendered_views ||= {} + @rendered_views ||= Hash.new { |hash, key| hash[key] = [] } end def add(view, locals) @@ -134,6 +134,10 @@ module ActionView @rendered_views[view] end + def rendered_views + @rendered_views.keys + end + def view_rendered?(view, expected_locals) locals_for(view).any? do |actual_locals| expected_locals.all? {|key, value| value == actual_locals[key] } @@ -215,6 +219,7 @@ module ActionView :@_routes, :@controller, :@_layouts, + :@_files, :@_rendered_views, :@method_name, :@output_buffer, |