diff options
Diffstat (limited to 'actionpack')
41 files changed, 378 insertions, 124 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 3f6cb5a5b1..e0076225ba 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,13 +1,57 @@ -* Fix handling of empty X_FORWARDED_HOST header in raw_host_with_port +* Fix rake routes not showing the right format when + nesting multiple routes. - Previously, an empty X_FORWARDED_HOST header would cause - Actiondispatch::Http:URL.raw_host_with_port to return nil, causing - Actiondispatch::Http:URL.host to raise a NoMethodError. + See #18373. + + *Ravil Bayramgalin* + +* Add ability to override default form builder for a controller. + + class AdminController < ApplicationController + default_form_builder AdminFormBuilder + end + + *Kevin McPhillips* + +* For actions with no corresponding templates, render `head :no_content` + instead of raising an error. This allows for slimmer API controller + methods that simply work, without needing further instructions. + + See #19036. + + *Stephen Bussey* + +* Provide friendlier access to request variants. + + request.variant = :phone + request.variant.phone? # true + request.variant.tablet? # false + + request.variant = [:phone, :tablet] + request.variant.phone? # true + request.variant.desktop? # false + request.variant.any?(:phone, :desktop) # true + request.variant.any?(:desktop, :watch) # false + + *George Claghorn* + +* Fix regression where a gzip file response would have a Content-type, + even when it was a 304 status code. + + See #19271. + + *Kohei Suzuki* + +* Fix handling of empty `X_FORWARDED_HOST` header in `raw_host_with_port`. + + Previously, an empty `X_FORWARDED_HOST` header would cause + `Actiondispatch::Http:URL.raw_host_with_port` to return `nil`, causing + `Actiondispatch::Http:URL.host` to raise a `NoMethodError`. *Adam Forsyth* * Drop request class from RouteSet constructor. - + If you would like to use a custom request class, please subclass and implement the `request_class` method. diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index b6b70a027c..1bba9df969 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -7,7 +7,7 @@ Gem::Specification.new do |s| s.summary = 'Web-flow and rendering framework putting the VC in MVC (part of Rails).' s.description = 'Web apps on Rails. Simple, battle-tested conventions for building and testing MVC web applications. Works with any Rack-compatible server.' - s.required_ruby_version = '>= 2.2.1' + s.required_ruby_version = '>= 2.2.2' s.license = 'MIT' diff --git a/actionpack/lib/abstract_controller/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb index 59ffb0a19e..13795f0dd8 100644 --- a/actionpack/lib/abstract_controller/callbacks.rb +++ b/actionpack/lib/abstract_controller/callbacks.rb @@ -62,9 +62,9 @@ module AbstractController # using #skip_action_callback def skip_action_callback(*names) ActiveSupport::Deprecation.warn('`skip_action_callback` is deprecated and will be removed in the next major version of Rails. Please use skip_before_action, skip_after_action or skip_around_action instead.') - skip_before_action(*names) - skip_after_action(*names) - skip_around_action(*names) + skip_before_action(*names, raise: false) + skip_after_action(*names, raise: false) + skip_around_action(*names, raise: false) end def skip_filter(*names) diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 7667e469d3..a1893ce920 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -12,6 +12,7 @@ module ActionController autoload :Metal autoload :Middleware autoload :Renderer + autoload :FormBuilder autoload_under "metal" do autoload :Compatibility diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index e6038396f9..bfae372f53 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -221,6 +221,7 @@ module ActionController Cookies, Flash, + FormBuilder, RequestForgeryProtection, ForceSSL, Streaming, diff --git a/actionpack/lib/action_controller/form_builder.rb b/actionpack/lib/action_controller/form_builder.rb new file mode 100644 index 0000000000..f2656ca894 --- /dev/null +++ b/actionpack/lib/action_controller/form_builder.rb @@ -0,0 +1,48 @@ +module ActionController + # Override the default form builder for all views rendered by this + # controller and any of its descendants. Accepts a subclass of + # +ActionView::Helpers::FormBuilder+. + # + # For example, given a form builder: + # + # class AdminFormBuilder < ActionView::Helpers::FormBuilder + # def special_field(name) + # end + # end + # + # The controller specifies a form builder as its default: + # + # class AdminAreaController < ApplicationController + # default_form_builder AdminFormBuilder + # end + # + # Then in the view any form using +form_for+ will be an instance of the + # specified form builder: + # + # <%= form_for(@instance) do |builder| %> + # <%= builder.special_field(:name) %> + # <% end %> + module FormBuilder + extend ActiveSupport::Concern + + included do + class_attribute :_default_form_builder, instance_accessor: false + end + + module ClassMethods + # Set the form builder to be used as the default for all forms + # in the views rendered by this controller and its subclasses. + # + # ==== Parameters + # * <tt>builder</tt> - Default form builder, an instance of +ActionView::Helpers::FormBuilder+ + def default_form_builder(builder) + self._default_form_builder = builder + end + end + + # Default form builder for the controller + def default_form_builder + self.class._default_form_builder + end + end +end diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 2273406948..909ed19a49 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -118,7 +118,7 @@ module ActionController end def authentication_request(controller, realm) - controller.headers["WWW-Authenticate"] = %(Basic realm="#{realm.gsub(/"/, "")}") + controller.headers["WWW-Authenticate"] = %(Basic realm="#{realm.tr('"'.freeze, "".freeze)}") controller.status = 401 controller.response_body = "HTTP Basic: Access denied.\n" end @@ -499,7 +499,7 @@ module ActionController # # Returns nothing. def authentication_request(controller, realm) - controller.headers["WWW-Authenticate"] = %(Token realm="#{realm.gsub(/"/, "")}") + controller.headers["WWW-Authenticate"] = %(Token realm="#{realm.tr('"'.freeze, "".freeze)}") controller.__send__ :render, :text => "HTTP Token: Access denied.\n", :status => :unauthorized end end diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index ae04b53825..1573ea7099 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -7,7 +7,12 @@ module ActionController end def default_render(*args) - render(*args) + if template_exists?(action_name.to_s, _prefixes, variants: request.variant) + render(*args) + else + logger.info "No template found for #{self.class.name}\##{action_name}, rendering head :no_content" if logger + head :no_content + end end def method_for_action(action_name) diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 7590fb6843..58150cd9a9 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -102,7 +102,7 @@ module ActionController end end - message = json.gsub(/\n/, "\ndata: ") + message = json.gsub("\n".freeze, "\ndata: ".freeze) @stream.write "data: #{message}\n\n" end end diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 7dae171215..fab1be3459 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -288,16 +288,17 @@ module ActionController #:nodoc: end def variant - if @variant.nil? + if @variant.empty? @variants[:none] || @variants[:any] - elsif (@variants.keys & @variant).any? - @variant.each do |v| - return @variants[v] if @variants.key?(v) - end else - @variants[:any] + @variants[variant_key] end end + + private + def variant_key + @variant.find { |variant| @variants.key?(variant) } || :any + end end end end diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 367b736035..31c8856437 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -13,9 +13,14 @@ module ActionController #:nodoc: # by including a token in the rendered HTML for your application. This token is # stored as a random string in the session, to which an attacker does not have # access. When a request reaches your application, \Rails verifies the received - # token with the token in the session. Only HTML and JavaScript requests are checked, - # so this will not protect your XML API (presumably you'll have a different - # authentication scheme there anyway). + # token with the token in the session. All requests are checked except GET requests + # as these should be idempotent. Keep in mind that all session-oriented requests + # should be CSRF protected, including JavaScript and HTML requests. + # + # Since HTML and JavaScript requests are typically made from the browser, we + # need to ensure to verify request authenticity for the web browser. We can + # use session-oriented authentication for these types requests, by using + # the `protect_form_forgery` method in our controllers. # # GET requests are not protected since they don't have side effects like writing # to the database and don't leak sensitive information. JavaScript requests are @@ -26,15 +31,21 @@ module ActionController #:nodoc: # Ajax) requests are allowed to make GET requests for JavaScript responses. # # It's important to remember that XML or JSON requests are also affected and if - # you're building an API you'll need something like: + # you're building an API you should change forgery protection method in + # <tt>ApplicationController</tt> (by default: <tt>:exception</tt>): # # class ApplicationController < ActionController::Base # protect_from_forgery unless: -> { request.format.json? } # end # - # CSRF protection is turned on with the <tt>protect_from_forgery</tt> method, - # which checks the token and resets the session if it doesn't match what was expected. - # A call to this method is generated for new \Rails applications by default. + # CSRF protection is turned on with the <tt>protect_from_forgery</tt> method. + # By default <tt>protect_from_forgery</tt> protects your session with + # <tt>:null_session</tt> method, which provides an empty session + # during request. + # + # We may want to disable CSRF protection for APIs since they are typically + # designed to be state-less. That is, the requestion API client will handle + # the session for you instead of Rails. # # The token parameter is named <tt>authenticity_token</tt> by default. The name and # value of this token must be added to every layout that renders forms by including @@ -86,10 +97,10 @@ module ActionController #:nodoc: # Valid Options: # # * <tt>:only/:except</tt> - Only apply forgery protection to a subset of actions. Like <tt>only: [ :create, :create_all ]</tt>. - # * <tt>:if/:unless</tt> - Turn off the forgery protection entirely depending on the passed proc or method reference. + # * <tt>:if/:unless</tt> - Turn off the forgery protection entirely depending on the passed Proc or method reference. # * <tt>:prepend</tt> - By default, the verification of the authentication token is added to the front of the # callback chain. If you need to make the verification depend on other callbacks, like authentication methods - # (say cookies vs oauth), this might not work for you. Pass <tt>prepend: false</tt> to just add the + # (say cookies vs OAuth), this might not work for you. Pass <tt>prepend: false</tt> to just add the # verification callback in the position of the protect_from_forgery call. This means any callbacks added # before are run first. # * <tt>:with</tt> - Set the method to handle unverified request. diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index f19c4201ba..c98e937423 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -117,7 +117,7 @@ module ActionController self.always_permitted_parameters = %w( controller action ) def self.const_missing(const_name) - super unless const_name == :NEVER_UNPERMITTED_PARAMS + return super unless const_name == :NEVER_UNPERMITTED_PARAMS ActiveSupport::Deprecation.warn(<<-MSG.squish) `ActionController::Parameters::NEVER_UNPERMITTED_PARAMS` has been deprecated. Use `ActionController::Parameters.always_permitted_parameters` instead. @@ -268,7 +268,7 @@ module ActionController # # params.permit(:name) # - # +:name+ passes it is a key of +params+ whose associated value is of type + # +:name+ passes if 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+. diff --git a/actionpack/lib/action_controller/metal/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb index fbaa90d521..5a0e5c62e4 100644 --- a/actionpack/lib/action_controller/metal/url_for.rb +++ b/actionpack/lib/action_controller/metal/url_for.rb @@ -5,9 +5,9 @@ module ActionController # In addition to <tt>AbstractController::UrlFor</tt>, this module accesses the HTTP layer to define # url options like the +host+. In order to do so, this module requires the host class # to implement +env+ which needs to be Rack-compatible and +request+ - # which is either instance of +ActionDispatch::Request+ or an object - # that responds to <tt>host</tt>, <tt>optional_port</tt>, <tt>protocol</tt> and - # <tt>symbolized_path_parameter</tt> methods. + # which is either an instance of +ActionDispatch::Request+ or an object + # that responds to the +host+, +optional_port+, +protocol+ and + # +symbolized_path_parameter+ methods. # # class RootUrl # include ActionController::UrlFor diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 33c24999f9..ca7ba90c40 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -604,7 +604,7 @@ module ActionController def process(action, *args) check_required_ivars - if kwarg_request?(*args) + if kwarg_request?(args) parameters, session, body, flash, http_method, format, xhr = args[0].values_at(:params, :session, :body, :flash, :method, :format, :xhr) else http_method, parameters, session, flash = args @@ -745,7 +745,7 @@ module ActionController private def process_with_kwargs(http_method, action, *args) - if kwarg_request?(*args) + if kwarg_request?(args) args.first.merge!(method: http_method) process(action, *args) else @@ -757,7 +757,7 @@ module ActionController end REQUEST_KWARGS = %i(params session flash method body xhr) - def kwarg_request?(*args) + def kwarg_request?(args) args[0].respond_to?(:keys) && ( (args[0].key?(:format) && args[0].keys.size == 1) || args[0].keys.any? { |k| REQUEST_KWARGS.include?(k) } diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index 53a98c5d0a..ff336b7354 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -10,8 +10,6 @@ module ActionDispatch self.ignore_accept_header = false end - attr_reader :variant - # The MIME type of the HTTP request, such as Mime::XML. # # For backward compatibility, the post \format is extracted from the @@ -75,18 +73,22 @@ module ActionDispatch # Sets the \variant for template. def variant=(variant) - if variant.is_a?(Symbol) - @variant = [variant] - elsif variant.nil? || variant.is_a?(Array) && variant.any? && variant.all?{ |v| v.is_a?(Symbol) } - @variant = variant + variant = Array(variant) + + if variant.all? { |v| v.is_a?(Symbol) } + @variant = ActiveSupport::ArrayInquirer.new(variant) else - raise ArgumentError, "request.variant must be set to a Symbol or an Array of Symbols, not a #{variant.class}. " \ + raise ArgumentError, "request.variant must be set to a Symbol or an Array of Symbols. " \ "For security reasons, never directly set the variant to a user-provided value, " \ "like params[:variant].to_sym. Check user-provided value against a whitelist first, " \ "then set the variant: request.variant = :tablet if params[:variant] == 'tablet'" end end + def variant + @variant ||= ActiveSupport::ArrayInquirer.new + end + # Sets the \format by string extension, which can be used to force custom formats # that are not controlled by the extension. # diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 732ee67268..a1f84e5ace 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -340,7 +340,7 @@ module ActionDispatch end protected - def parse_query(qs) + def parse_query(*) Utils.deep_munge(super) end diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb index 992c1a9efe..c0566c6fc9 100644 --- a/actionpack/lib/action_dispatch/journey/formatter.rb +++ b/actionpack/lib/action_dispatch/journey/formatter.rb @@ -39,7 +39,7 @@ module ActionDispatch return [route.format(parameterized_parts), params] end - message = "No route matches #{Hash[constraints.sort].inspect}" + message = "No route matches #{Hash[constraints.sort_by{|k,v| k.to_s}].inspect}" message << " missing required keys: #{missing_keys.sort.inspect}" unless missing_keys.empty? raise ActionController::UrlGenerationError, message diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index 4d5c18984a..4698ff8cc7 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -36,7 +36,7 @@ module ActionDispatch def requirements # :nodoc: # needed for rails `rake routes` - path.requirements.merge(@defaults).delete_if { |_,v| + @defaults.merge(path.requirements).delete_if { |_,v| /.+?/ == v } end diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index b7687ca100..dd1f140051 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -79,6 +79,9 @@ module ActionDispatch # domain: %w(.example.com .example.org) # Allow the cookie # # for concrete domain names. # + # * <tt>:tld_length</tt> - When using <tt>:domain => :all</tt>, this option can be used to explicitly + # set the TLD length when using a short (<= 3 character) domain that is being interpreted as part of a TLD. + # For example, to share cookies between user1.lvh.me and user2.lvh.me, set <tt>:tld_length</tt> to 1. # * <tt>:expires</tt> - The time at which this cookie expires, as a \Time object. # * <tt>:secure</tt> - Whether this cookie is only transmitted to HTTPS servers. # Default is +false+. @@ -181,7 +184,7 @@ module ActionDispatch # to the Message{Encryptor,Verifier} allows us to handle the # (de)serialization step within the cookie jar, which gives us the # opportunity to detect and migrate legacy cookies. - module VerifyAndUpgradeLegacySignedMessage + module VerifyAndUpgradeLegacySignedMessage # :nodoc: def initialize(*args) super @legacy_verifier = ActiveSupport::MessageVerifier.new(@options[:secret_token], serializer: ActiveSupport::MessageEncryptor::NullSerializer) @@ -392,7 +395,7 @@ module ActionDispatch end end - class JsonSerializer + class JsonSerializer # :nodoc: def self.load(value) ActiveSupport::JSON.decode(value) end @@ -402,7 +405,7 @@ module ActionDispatch end end - module SerializedCookieJars + module SerializedCookieJars # :nodoc: MARSHAL_SIGNATURE = "\x04\x08".freeze protected @@ -454,12 +457,16 @@ module ActionDispatch @verifier = ActiveSupport::MessageVerifier.new(secret, digest: digest, serializer: ActiveSupport::MessageEncryptor::NullSerializer) end + # Returns the value of the cookie by +name+ if it is untampered, + # returns +nil+ otherwise or if no such cookie exists. def [](name) if signed_message = @parent_jar[name] deserialize name, verify(signed_message) end end + # Signs and sets the cookie named +name+. The second argument may be the cookie's + # value or a hash of options as documented above. def []=(name, options) if options.is_a?(Hash) options.symbolize_keys! @@ -482,8 +489,8 @@ module ActionDispatch # UpgradeLegacySignedCookieJar is used instead of SignedCookieJar if # secrets.secret_token and secrets.secret_key_base are both set. It reads - # legacy cookies signed with the old dummy key generator and re-saves - # them using the new key generator to provide a smooth upgrade path. + # legacy cookies signed with the old dummy key generator and signs and + # re-saves them using the new key generator to provide a smooth upgrade path. class UpgradeLegacySignedCookieJar < SignedCookieJar #:nodoc: include VerifyAndUpgradeLegacySignedMessage @@ -511,12 +518,16 @@ module ActionDispatch @encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, digest: digest, serializer: ActiveSupport::MessageEncryptor::NullSerializer) end + # Returns the value of the cookie by +name+ if it is untampered, + # returns +nil+ otherwise or if no such cookie exists. def [](name) if encrypted_message = @parent_jar[name] deserialize name, decrypt_and_verify(encrypted_message) end end + # Encrypts and sets the cookie named +name+. The second argument may be the cookie's + # value or a hash of options as documented above. def []=(name, options) if options.is_a?(Hash) options.symbolize_keys! diff --git a/actionpack/lib/action_dispatch/middleware/public_exceptions.rb b/actionpack/lib/action_dispatch/middleware/public_exceptions.rb index 040cb215b7..7cde76b30e 100644 --- a/actionpack/lib/action_dispatch/middleware/public_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/public_exceptions.rb @@ -17,10 +17,10 @@ module ActionDispatch end def call(env) - status = env["PATH_INFO"][1..-1] + status = env["PATH_INFO"][1..-1].to_i request = ActionDispatch::Request.new(env) content_type = request.formats.first - body = { :status => status, :error => Rack::Utils::HTTP_STATUS_CODES.fetch(status.to_i, Rack::Utils::HTTP_STATUS_CODES[500]) } + body = { :status => status, :error => Rack::Utils::HTTP_STATUS_CODES.fetch(status, Rack::Utils::HTTP_STATUS_CODES[500]) } render(status, content_type, body) end diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index fdd1bc4e69..c47e5d5245 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -3,15 +3,15 @@ require 'active_support/core_ext/uri' module ActionDispatch # This middleware returns a file's contents from disk in the body response. - # When initialized it can accept an optional 'Cache-Control' header which + # When initialized, it can accept an optional 'Cache-Control' header, which # will be set when a response containing a file's contents is delivered. # # This middleware will render the file specified in `env["PATH_INFO"]` - # where the base path is in the +root+ directory. For example if the +root+ - # is set to `public/` then a request with `env["PATH_INFO"]` of - # `assets/application.js` will return a response with contents of a file + # where the base path is in the +root+ directory. For example, if the +root+ + # is set to `public/`, then a request with `env["PATH_INFO"]` of + # `assets/application.js` will return a response with the contents of a file # located at `public/assets/application.js` if the file exists. If the file - # does not exist a 404 "File not Found" response will be returned. + # does not exist, a 404 "File not Found" response will be returned. class FileHandler def initialize(root, cache_control) @root = root.chomp('/') @@ -20,6 +20,13 @@ module ActionDispatch @file_server = ::Rack::File.new(@root, headers) end + + # Takes a path to a file. If the file is found, has valid encoding, and has + # correct read permissions, the return value is a URI-escaped string + # representing the filename. Otherwise, false is returned. + # + # Used by the `Static` class to check the existence of a valid file + # in the server's `public/` directory. (See Static#call) def match?(path) path = URI.parser.unescape(path) return false unless path.valid_encoding? @@ -28,7 +35,7 @@ module ActionDispatch paths = [path, "#{path}#{ext}", "#{path}/index#{ext}"] if match = paths.detect { |p| - path = File.join(@root, p) + path = File.join(@root, p.force_encoding('UTF-8')) begin File.file?(path) && File.readable?(path) rescue SystemCallError @@ -88,7 +95,7 @@ module ActionDispatch end # This middleware will attempt to return the contents of a file's body from - # disk in the response. If a file is not found on disk, the request will be + # disk in the response. If a file is not found on disk, the request will be # delegated to the application stack. This middleware is commonly initialized # to serve assets from a server's `public/` directory. # diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb index 973627f106..9a1a05e971 100644 --- a/actionpack/lib/action_dispatch/request/session.rb +++ b/actionpack/lib/action_dispatch/request/session.rb @@ -9,7 +9,8 @@ module ActionDispatch # Singleton object used to determine if an optional param wasn't specified Unspecified = Object.new - + + # Creates a session hash, merging the properties of the previous session if any def self.create(store, env, default_options) session_was = find env session = Request::Session.new(store, env) diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index c513737fc2..48c10a7d4c 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -45,7 +45,7 @@ module ActionDispatch end def internal? - controller.to_s =~ %r{\Arails/(info|mailers|welcome)} || path =~ %r{\A#{Rails.application.config.assets.prefix}\z} + controller.to_s =~ %r{\Arails/(info|mailers|welcome)} end def engine? diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 34b5b48f3a..49009a45cc 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1678,7 +1678,7 @@ module ActionDispatch end def shallow_nesting_depth #:nodoc: - @nesting.select(&:shallow?).size + @nesting.count(&:shallow?) end def param_constraint? #:nodoc: diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 0f3734dd74..d0d8ded515 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -226,7 +226,7 @@ module ActionDispatch params = parameterize_args(args) { |missing_key| missing_keys << missing_key } - constraints = Hash[@route.requirements.merge(params).sort] + constraints = Hash[@route.requirements.merge(params).sort_by{|k,v| k.to_s}] message = "No route matches #{constraints.inspect}" message << " missing required keys: #{missing_keys.sort.inspect}" diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 9390e2937a..b1bd6ae6d5 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -2,6 +2,7 @@ require 'stringio' require 'uri' require 'active_support/core_ext/kernel/singleton_class' require 'active_support/core_ext/object/try' +require 'active_support/core_ext/string/strip' require 'rack/test' require 'minitest' @@ -80,7 +81,7 @@ module ActionDispatch # # xhr :get, '/feed', params: { since: 201501011400 } def xml_http_request(request_method, path, *args) - if kwarg_request?(*args) + if kwarg_request?(args) params, headers, env = args.first.values_at(:params, :headers, :env) else params = args[0] @@ -290,7 +291,7 @@ module ActionDispatch end def process_with_kwargs(http_method, path, *args) - if kwarg_request?(*args) + if kwarg_request?(args) process(http_method, path, *args) else non_kwarg_request_warning if args.present? @@ -299,7 +300,7 @@ module ActionDispatch end REQUEST_KWARGS = %i(params headers env xhr) - def kwarg_request?(*args) + def kwarg_request?(args) args[0].respond_to?(:keys) && args[0].keys.any? { |k| REQUEST_KWARGS.include?(k) } end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 62ff1be5c9..c1be2c9afe 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -14,7 +14,11 @@ silence_warnings do end require 'drb' -require 'drb/unix' +begin + require 'drb/unix' +rescue LoadError + puts "'drb/unix' is not available" +end require 'tempfile' PROCESS_COUNT = (ENV['N'] || 4).to_i diff --git a/actionpack/test/controller/force_ssl_test.rb b/actionpack/test/controller/force_ssl_test.rb index 04222745d9..5639abdc56 100644 --- a/actionpack/test/controller/force_ssl_test.rb +++ b/actionpack/test/controller/force_ssl_test.rb @@ -315,7 +315,7 @@ class RedirectToSSLTest < ActionController::TestCase assert_equal "https://secure.cheeseburger.host/redirect_to_ssl/cheeseburger", redirect_to_url end - def test_banana_does_not_redirect_if_already_https + def test_cheeseburgers_does_not_redirect_if_already_https request.env['HTTPS'] = 'on' get :cheeseburger assert_response 200 diff --git a/actionpack/test/controller/form_builder_test.rb b/actionpack/test/controller/form_builder_test.rb new file mode 100644 index 0000000000..99eeaf9ab6 --- /dev/null +++ b/actionpack/test/controller/form_builder_test.rb @@ -0,0 +1,17 @@ +require 'abstract_unit' + +class FormBuilderController < ActionController::Base + class SpecializedFormBuilder < ActionView::Helpers::FormBuilder ; end + + default_form_builder SpecializedFormBuilder +end + +class ControllerFormBuilderTest < ActiveSupport::TestCase + setup do + @controller = FormBuilderController.new + end + + def test_default_form_builder_assigned + assert_equal FormBuilderController::SpecializedFormBuilder, @controller.default_form_builder + end +end diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index 1f5f66dc80..7aef8a50ce 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -1,4 +1,5 @@ require 'abstract_unit' +require "active_support/log_subscriber/test_helper" class RespondToController < ActionController::Base layout :set_layout @@ -608,19 +609,29 @@ class RespondToControllerTest < ActionController::TestCase end def test_invalid_variant + logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new + old_logger, ActionController::Base.logger = ActionController::Base.logger, logger + @request.variant = :invalid - assert_raises(ActionView::MissingTemplate) do - get :variant_with_implicit_rendering - end + get :variant_with_implicit_rendering + assert_response :no_content + assert_equal 1, logger.logged(:info).select{ |s| s =~ /No template found/ }.size, "Implicit head :no_content not logged" + ensure + ActionController::Base.logger = old_logger end def test_variant_not_set_regular_template_missing - assert_raises(ActionView::MissingTemplate) do - get :variant_with_implicit_rendering - end + get :variant_with_implicit_rendering + assert_response :no_content end def test_variant_with_implicit_rendering + @request.variant = :implicit + get :variant_with_implicit_rendering + assert_response :no_content + end + + def test_variant_with_implicit_template_rendering @request.variant = :mobile get :variant_with_implicit_rendering assert_equal "text/html", @response.content_type diff --git a/actionpack/test/controller/parameters/always_permitted_parameters_test.rb b/actionpack/test/controller/parameters/always_permitted_parameters_test.rb index 059f310d49..59be08db54 100644 --- a/actionpack/test/controller/parameters/always_permitted_parameters_test.rb +++ b/actionpack/test/controller/parameters/always_permitted_parameters_test.rb @@ -1,5 +1,6 @@ require 'abstract_unit' require 'action_controller/metal/strong_parameters' +require 'minitest/mock' class AlwaysPermittedParametersTest < ActiveSupport::TestCase def setup @@ -14,7 +15,13 @@ class AlwaysPermittedParametersTest < ActiveSupport::TestCase test "shows deprecations warning on NEVER_UNPERMITTED_PARAMS" do assert_deprecated do - ActionController::Parameters::NEVER_UNPERMITTED_PARAMS + ActionController::Parameters::NEVER_UNPERMITTED_PARAMS + end + end + + test "returns super on missing constant other than NEVER_UNPERMITTED_PARAMS" do + ActionController::Parameters.superclass.stub :const_missing, "super" do + assert_equal "super", ActionController::Parameters::NON_EXISTING_CONSTANT end end diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 103ca9c776..ef30f1ea0f 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -1,8 +1,5 @@ require 'abstract_unit' -class WorkshopsController < ActionController::Base -end - class RedirectController < ActionController::Base # empty method not used anywhere to ensure methods like # `status` and `location` aren't called on `redirect_to` calls @@ -63,7 +60,7 @@ class RedirectController < ActionController::Base end def redirect_to_url_with_unescaped_query_string - redirect_to "http://dev.rubyonrails.org/query?status=new" + redirect_to "http://example.com/query?status=new" end def redirect_to_url_with_complex_scheme @@ -233,7 +230,7 @@ class RedirectTest < ActionController::TestCase def test_redirect_to_url_with_unescaped_query_string get :redirect_to_url_with_unescaped_query_string assert_response :redirect - assert_redirected_to "http://dev.rubyonrails.org/query?status=new" + assert_redirected_to "http://example.com/query?status=new" end def test_redirect_to_url_with_complex_scheme diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 8887f291cf..f8cf79a257 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -12,14 +12,6 @@ module RequestForgeryProtectionActions render :inline => "<%= button_to('New', '/') %>" end - def external_form - render :inline => "<%= form_tag('http://farfar.away/form', :authenticity_token => 'external_token') {} %>" - end - - def external_form_without_protection - render :inline => "<%= form_tag('http://farfar.away/form', :authenticity_token => false) {} %>" - end - def unsafe render :text => 'pwn' end @@ -28,14 +20,6 @@ module RequestForgeryProtectionActions render :inline => "<%= csrf_meta_tags %>" end - def external_form_for - render :inline => "<%= form_for(:some_resource, :authenticity_token => 'external_token') {} %>" - end - - def form_for_without_protection - render :inline => "<%= form_for(:some_resource, :authenticity_token => false ) {} %>" - end - def form_for_remote render :inline => "<%= form_for(:some_resource, :remote => true ) {} %>" end @@ -70,7 +54,6 @@ module RequestForgeryProtectionActions negotiate_same_origin end - def rescue_action(e) raise e end end # sample controllers diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index 2d08987ca6..9bbfb74e72 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -8,8 +8,6 @@ class MilestonesController < ActionController::Base alias_method :show, :index end -ROUTING = ActionDispatch::Routing - # See RFC 3986, section 3.3 for allowed path characters. class UriReservedCharactersRoutingTest < ActiveSupport::TestCase include RoutingTestHelpers @@ -871,7 +869,7 @@ class RouteSetTest < ActiveSupport::TestCase def default_route_set @default_route_set ||= begin - set = ROUTING::RouteSet.new + set = ActionDispatch::Routing::RouteSet.new set.draw do get '/:controller(/:action(/:id))' end @@ -1748,13 +1746,13 @@ class RouteSetTest < ActiveSupport::TestCase include ActionDispatch::RoutingVerbs - class TestSet < ROUTING::RouteSet + class TestSet < ActionDispatch::Routing::RouteSet def initialize(block) @block = block super() end - class Dispatcher < ROUTING::RouteSet::Dispatcher + class Dispatcher < ActionDispatch::Routing::RouteSet::Dispatcher def initialize(defaults, set, block) super(defaults) @block = block diff --git a/actionpack/test/controller/show_exceptions_test.rb b/actionpack/test/controller/show_exceptions_test.rb index fba5ebba15..786dc15444 100644 --- a/actionpack/test/controller/show_exceptions_test.rb +++ b/actionpack/test/controller/show_exceptions_test.rb @@ -75,7 +75,7 @@ module ShowExceptions get "/", headers: { 'HTTP_ACCEPT' => 'application/json' } assert_response :internal_server_error assert_equal 'application/json', response.content_type.to_s - assert_equal({ :status => '500', :error => 'Internal Server Error' }.to_json, response.body) + assert_equal({ :status => 500, :error => 'Internal Server Error' }.to_json, response.body) end def test_render_xml_exception @@ -83,7 +83,7 @@ module ShowExceptions get "/", headers: { 'HTTP_ACCEPT' => 'application/xml' } assert_response :internal_server_error assert_equal 'application/xml', response.content_type.to_s - assert_equal({ :status => '500', :error => 'Internal Server Error' }.to_xml, response.body) + assert_equal({ :status => 500, :error => 'Internal Server Error' }.to_xml, response.body) end def test_render_fallback_exception diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 61cc4dcd7e..f208cfda89 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -1128,35 +1128,47 @@ class RequestEtag < BaseRequestTest end class RequestVariant < BaseRequestTest - test "setting variant" do - request = stub_request + def setup + super + @request = stub_request + end - request.variant = :mobile - assert_equal [:mobile], request.variant + test 'setting variant to a symbol' do + @request.variant = :phone - request.variant = [:phone, :tablet] - assert_equal [:phone, :tablet], request.variant + assert @request.variant.phone? + assert_not @request.variant.tablet? + assert @request.variant.any?(:phone, :tablet) + assert_not @request.variant.any?(:tablet, :desktop) + end - assert_raise ArgumentError do - request.variant = [:phone, "tablet"] - end + test 'setting variant to an array of symbols' do + @request.variant = [:phone, :tablet] - assert_raise ArgumentError do - request.variant = "yolo" - end + assert @request.variant.phone? + assert @request.variant.tablet? + assert_not @request.variant.desktop? + assert @request.variant.any?(:tablet, :desktop) + assert_not @request.variant.any?(:desktop, :watch) end - test "reset variant" do - request = stub_request + test 'clearing variant' do + @request.variant = nil - request.variant = nil - assert_equal nil, request.variant + assert @request.variant.empty? + assert_not @request.variant.phone? + assert_not @request.variant.any?(:phone, :tablet) end - test "setting variant with non symbol value" do - request = stub_request + test 'setting variant to a non-symbol value' do + assert_raise ArgumentError do + @request.variant = 'phone' + end + end + + test 'setting variant to an array containing a non-symbol value' do assert_raise ArgumentError do - request.variant = "mobile" + @request.variant = [:phone, 'tablet'] end end end diff --git a/actionpack/test/dispatch/routing/inspector_test.rb b/actionpack/test/dispatch/routing/inspector_test.rb index 3df022c64b..4047214843 100644 --- a/actionpack/test/dispatch/routing/inspector_test.rb +++ b/actionpack/test/dispatch/routing/inspector_test.rb @@ -313,6 +313,22 @@ module ActionDispatch assert_equal ["Prefix Verb URI Pattern Controller#Action", " GET /:controller(/:action) (?-mix:api\\/[^\\/]+)#:action"], output end + + def test_inspect_routes_shows_resources_route_when_assets_disabled + @set = ActionDispatch::Routing::RouteSet.new + app = ActiveSupport::OrderedOptions.new + + Rails.stubs(:application).returns(app) + + output = draw do + get '/cart', to: 'cart#show' + end + + assert_equal [ + "Prefix Verb URI Pattern Controller#Action", + " cart GET /cart(.:format) cart#show" + ], output + end end end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 55fc160ac8..62c99a2edc 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -4476,6 +4476,19 @@ class TestUrlGenerationErrors < ActionDispatch::IntegrationTest error = assert_raises(ActionController::UrlGenerationError, message){ product_path(id: nil) } assert_equal message, error.message end + + test "url helpers raise message with mixed parameters when generation fails " do + url, missing = { action: 'show', controller: 'products', id: nil, "id"=>"url-tested"}, [:id] + message = "No route matches #{url.inspect} missing required keys: #{missing.inspect}" + + # Optimized url helper + error = assert_raises(ActionController::UrlGenerationError){ product_path(nil, 'id'=>'url-tested') } + assert_equal message, error.message + + # Non-optimized url helper + error = assert_raises(ActionController::UrlGenerationError, message){ product_path(id: nil, 'id'=>'url-tested') } + assert_equal message, error.message + end end class TestDefaultUrlOptions < ActionDispatch::IntegrationTest diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb index 288a2084f6..93e5c85a97 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -2,6 +2,20 @@ require 'abstract_unit' require 'zlib' module StaticTests + def setup + silence_warnings do + @default_internal_encoding = Encoding.default_internal + @default_external_encoding = Encoding.default_external + end + end + + def teardown + silence_warnings do + Encoding.default_internal = @default_internal_encoding + Encoding.default_external = @default_external_encoding + end + end + def test_serves_dynamic_content assert_equal "Hello, World!", get("/nofile").body end @@ -10,6 +24,18 @@ module StaticTests assert_equal "Hello, World!", get("/doorkeeper%E3E4").body end + def test_handles_urls_with_ascii_8bit + assert_equal "Hello, World!", get("/doorkeeper%E3E4".force_encoding('ASCII-8BIT')).body + end + + def test_handles_urls_with_ascii_8bit_on_win_31j + silence_warnings do + Encoding.default_internal = "Windows-31J" + Encoding.default_external = "Windows-31J" + end + assert_equal "Hello, World!", get("/doorkeeper%E3E4".force_encoding('ASCII-8BIT')).body + end + def test_sets_cache_control response = get("/index.html") assert_html "/index.html", response @@ -208,6 +234,7 @@ class StaticTest < ActiveSupport::TestCase } def setup + super @root = "#{FIXTURE_LOAD_PATH}/public" @app = ActionDispatch::Static.new(DummyApp, @root, "public, max-age=60") end @@ -237,6 +264,7 @@ end class StaticEncodingTest < StaticTest def setup + super @root = "#{FIXTURE_LOAD_PATH}/公共" @app = ActionDispatch::Static.new(DummyApp, @root, "public, max-age=60") end diff --git a/actionpack/test/journey/route_test.rb b/actionpack/test/journey/route_test.rb index 21d867aca0..9616f036b3 100644 --- a/actionpack/test/journey/route_test.rb +++ b/actionpack/test/journey/route_test.rb @@ -25,6 +25,14 @@ module ActionDispatch end end + def test_path_requirements_override_defaults + strexp = Router::Strexp.build(':name', { name: /love/ }, ['/']) + path = Path::Pattern.new strexp + defaults = { name: 'tender' } + route = Route.new('name', nil, path, nil, defaults) + assert_equal /love/, route.requirements[:name] + end + def test_ip_address path = Path::Pattern.from_string '/messages/:id(.:format)' route = Route.new("name", nil, path, {:ip => '192.168.1.1'}, diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index 19c61b5914..a134e343cc 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -401,6 +401,33 @@ module ActionDispatch assert_equal({:id => 1, :relative_url_root => nil}, params) end + def test_generate_missing_keys_no_matches_different_format_keys + path = Path::Pattern.from_string '/:controller/:action/:name' + @router.routes.add_route @app, path, {}, {}, {} + primarty_parameters = { + :id => 1, + :controller => "tasks", + :action => "show", + :relative_url_root => nil + } + redirection_parameters = { + 'action'=>'show', + } + missing_key = 'name' + missing_parameters ={ + missing_key => "task_1" + } + request_parameters = primarty_parameters.merge(redirection_parameters).merge(missing_parameters) + + message = "No route matches #{Hash[request_parameters.sort_by{|k,v|k.to_s}].inspect} missing required keys: #{[missing_key.to_sym].inspect}" + + error = assert_raises(ActionController::UrlGenerationError) do + @formatter.generate( + nil, request_parameters, request_parameters) + end + assert_equal message, error.message + end + def test_generate_uses_recall_if_needed path = Path::Pattern.from_string '/:controller(/:action(/:id))' @router.routes.add_route @app, path, {}, {}, {} |