diff options
Diffstat (limited to 'actionpack')
32 files changed, 442 insertions, 157 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index d473ab427d..6b73b29ace 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,34 @@ +* Update default rendering policies when the controller action did + not explicitly indicate a response. + + For API controllers, the implicit render always renders "204 No Content" + and does not account for any templates. + + For other controllers, the following conditions are checked: + + First, if a template exists for the controller action, it is rendered. + This template lookup takes into account the action name, locales, format, + variant, template handlers, etc. (see +render+ for details). + + Second, if other templates exist for the controller action but is not in + the right format (or variant, etc.), an <tt>ActionController::UnknownFormat</tt> + is raised. The list of available templates is assumed to be a complete + enumeration of all the possible formats (or variants, etc.); that is, + having only HTML and JSON templates indicate that the controller action is + not meant to handle XML requests. + + Third, if the current request is an "interactive" browser request (the user + navigated here by entering the URL in the address bar, submiting a form, + clicking on a link, etc. as opposed to an XHR or non-browser API request), + <tt>ActionView::UnknownFormat</tt> is raised to display a helpful error + message. + + Finally, it falls back to the same "204 No Content" behavior as API controllers. + + *Godfrey Chan*, *Jon Moss*, *Kasper Timm Hansen*, *Mike Clark*, *Matthew Draper* + +## Rails 5.0.0.beta3 (February 24, 2016) ## + * Add application/gzip as a default mime type. *Mehmet Emin İNAÇ* @@ -37,13 +68,13 @@ end end ``` - + Passing `as: :json` to integration test request helpers will set the format, content type and encode the parameters as JSON. - + Then on the response side, `parsed_body` will parse the body according to the content type the response has. - + Currently JSON is the only supported MIME type. Add your own with `ActionDispatch::IntegrationTest.register_encoder`. diff --git a/actionpack/lib/abstract_controller.rb b/actionpack/lib/abstract_controller.rb index 56c4033387..1e57cbaac4 100644 --- a/actionpack/lib/abstract_controller.rb +++ b/actionpack/lib/abstract_controller.rb @@ -6,6 +6,7 @@ module AbstractController extend ActiveSupport::Autoload autoload :Base + autoload :Caching autoload :Callbacks autoload :Collector autoload :DoubleRenderError, "abstract_controller/rendering" @@ -15,4 +16,9 @@ module AbstractController autoload :Translation autoload :AssetPaths autoload :UrlFor + + def self.eager_load! + super + AbstractController::Caching.eager_load! + end end diff --git a/actionpack/lib/abstract_controller/caching.rb b/actionpack/lib/abstract_controller/caching.rb new file mode 100644 index 0000000000..0dea50889a --- /dev/null +++ b/actionpack/lib/abstract_controller/caching.rb @@ -0,0 +1,62 @@ +module AbstractController + module Caching + extend ActiveSupport::Concern + extend ActiveSupport::Autoload + + eager_autoload do + autoload :Fragments + end + + module ConfigMethods + def cache_store + config.cache_store + end + + def cache_store=(store) + config.cache_store = ActiveSupport::Cache.lookup_store(store) + end + + private + def cache_configured? + perform_caching && cache_store + end + end + + include ConfigMethods + include AbstractController::Caching::Fragments + + included do + extend ConfigMethods + + config_accessor :default_static_extension + self.default_static_extension ||= '.html' + + 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 view_cache_dependencies + self.class._view_cache_dependencies.map { |dep| instance_exec(&dep) }.compact + end + + protected + # Convenience accessor. + def cache(key, options = {}, &block) + if cache_configured? + cache_store.fetch(ActiveSupport::Cache.expand_cache_key(key, :controller), options, &block) + else + yield + end + end + end +end diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/abstract_controller/caching/fragments.rb index b9ad51a9cf..3257a731ed 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/abstract_controller/caching/fragments.rb @@ -1,4 +1,4 @@ -module ActionController +module AbstractController module Caching # Fragment caching is used for caching various blocks within # views without caching the entire action as a whole. This is @@ -135,13 +135,8 @@ module ActionController end def instrument_fragment_cache(name, key) # :nodoc: - payload = { - controller: controller_name, - action: action_name, - key: key - } - - ActiveSupport::Notifications.instrument("#{name}.action_controller", payload) { yield } + payload = instrument_payload(key) + ActiveSupport::Notifications.instrument("#{name}.#{instrument_name}", payload) { yield } end end end diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 40f33a9de0..62f5905205 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -9,12 +9,15 @@ module ActionController autoload :API autoload :Base - autoload :Caching autoload :Metal autoload :Middleware autoload :Renderer autoload :FormBuilder + eager_autoload do + autoload :Caching + end + autoload_under "metal" do autoload :ConditionalGet autoload :Cookies @@ -47,11 +50,6 @@ module ActionController autoload :TestCase, 'action_controller/test_case' autoload :TemplateAssertions, 'action_controller/test_case' - - def self.eager_load! - super - ActionController::Caching.eager_load! - end end # Common Active Support usage in Action Controller diff --git a/actionpack/lib/action_controller/caching.rb b/actionpack/lib/action_controller/caching.rb index 0b8fa2ea09..a9a8508abc 100644 --- a/actionpack/lib/action_controller/caching.rb +++ b/actionpack/lib/action_controller/caching.rb @@ -1,6 +1,3 @@ -require 'fileutils' -require 'uri' - module ActionController # \Caching is a cheap way of speeding up slow applications by keeping the result of # calculations, renderings, and database calls around for subsequent requests. @@ -23,65 +20,25 @@ module ActionController # config.action_controller.cache_store = :mem_cache_store, Memcached::Rails.new('localhost:11211') # config.action_controller.cache_store = MyOwnStore.new('parameter') module Caching - extend ActiveSupport::Concern extend ActiveSupport::Autoload - - eager_autoload do - autoload :Fragments - end - - module ConfigMethods - def cache_store - config.cache_store - end - - def cache_store=(store) - config.cache_store = ActiveSupport::Cache.lookup_store(store) - end - - private - def cache_configured? - perform_caching && cache_store - end - end - - include AbstractController::Callbacks - - include ConfigMethods - include Fragments + extend ActiveSupport::Concern included do - extend ConfigMethods - - config_accessor :default_static_extension - self.default_static_extension ||= '.html' - - 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) + include AbstractController::Caching end - module ClassMethods - def view_cache_dependency(&dependency) - self._view_cache_dependencies += [dependency] - end - end + private - def view_cache_dependencies - self.class._view_cache_dependencies.map { |dep| instance_exec(&dep) }.compact - end + def instrument_payload(key) + { + controller: controller_name, + action: action_name, + key: key + } + end - protected - # Convenience accessor. - def cache(key, options = {}, &block) - if cache_configured? - cache_store.fetch(ActiveSupport::Cache.expand_cache_key(key, :controller), options, &block) - else - yield - end + def instrument_name + "action_controller" end end end diff --git a/actionpack/lib/action_controller/metal/basic_implicit_render.rb b/actionpack/lib/action_controller/metal/basic_implicit_render.rb index 6c6f8381ff..cef65a362c 100644 --- a/actionpack/lib/action_controller/metal/basic_implicit_render.rb +++ b/actionpack/lib/action_controller/metal/basic_implicit_render.rb @@ -1,5 +1,5 @@ module ActionController - module BasicImplicitRender + module BasicImplicitRender # :nodoc: def send_action(method, *args) super.tap { default_render unless performed? } end diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index 17fcc2fa02..6b540d42c7 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -1,29 +1,80 @@ +require 'active_support/core_ext/string/strip' + module ActionController + # Handles implicit rendering for a controller action when it did not + # explicitly indicate an appropiate response via methods such as +render+, + # +respond_to+, +redirect+ or +head+. + # + # For API controllers, the implicit render always renders "204 No Content" + # and does not account for any templates. + # + # For other controllers, the following conditions are checked: + # + # First, if a template exists for the controller action, it is rendered. + # This template lookup takes into account the action name, locales, format, + # variant, template handlers, etc. (see +render+ for details). + # + # Second, if other templates exist for the controller action but is not in + # the right format (or variant, etc.), an <tt>ActionController::UnknownFormat</tt> + # is raised. The list of available templates is assumed to be a complete + # enumeration of all the possible formats (or variants, etc.); that is, + # having only HTML and JSON templates indicate that the controller action is + # not meant to handle XML requests. + # + # Third, if the current request is an "interactive" browser request (the user + # navigated here by entering the URL in the address bar, submiting a form, + # clicking on a link, etc. as opposed to an XHR or non-browser API request), + # <tt>ActionView::UnknownFormat</tt> is raised to display a helpful error + # message. + # + # Finally, it falls back to the same "204 No Content" behavior as API controllers. module ImplicitRender + # :stopdoc: include BasicImplicitRender - # Renders the template corresponding to the controller action, if it exists. - # The action name, format, and variant are all taken into account. - # For example, the "new" action with an HTML format and variant "phone" - # would try to render the <tt>new.html+phone.erb</tt> template. - # - # If no template is found <tt>ActionController::BasicImplicitRender</tt>'s implementation is called, unless - # a block is passed. In that case, it will override the super implementation. - # - # default_render do - # head 404 # No template was found - # end def default_render(*args) if template_exists?(action_name.to_s, _prefixes, variants: request.variant) render(*args) - else - if block_given? - yield(*args) - else - logger.info "No template found for #{self.class.name}\##{action_name}, rendering head :no_content" if logger - super + elsif any_templates?(action_name.to_s, _prefixes) + message = "#{self.class.name}\##{action_name} does not know how to respond " \ + "to this request. There are other templates available for this controller " \ + "action but none of them were suitable for this request.\n\n" \ + "This usually happens when the client requested an unsupported format " \ + "(e.g. requesting HTML content from a JSON endpoint or vice versa), but " \ + "it might also be failing due to other constraints, such as locales or" \ + "variants.\n" + + if request.formats.any? + message << "\nRequested format(s): #{request.formats.join(", ")}" end + + if request.variant.any? + message << "\nRequested variant(s): #{request.variant.join(", ")}" + end + + raise ActionController::UnknownFormat, message + elsif interactive_browser_request? + message = "You did not define any templates for #{self.class.name}\##{action_name}. " \ + "This is not necessarily a problem (e.g. you might be building an API endpoint " \ + "that does not require any templates), and the controller would usually respond " \ + "with `head :no_content` for your convenience.\n\n" \ + "However, you appear to have navigated here from an interactive browser request – " \ + "such as by navigating to this URL directly, clicking on a link or submitting a form. " \ + "Rendering a `head :no_content` in this case could have resulted in unexpected UI " \ + "behavior in the browser.\n\n" \ + "If you expected the `head :no_content` response, you do not need to take any " \ + "actions – requests coming from an XHR (AJAX) request or other non-browser clients " \ + "will receive the \"204 No Content\" response as expected.\n\n" \ + "If you did not expect this behavior, you can resolve this error by adding a " \ + "template for this controller action (usually `#{action_name}.html.erb`) or " \ + "otherwise indicate the appropriate response in the action using `render`, " \ + "`redirect_to`, `head`, etc.\n" + + raise ActionController::UnknownFormat, message + else + logger.info "No template found for #{self.class.name}\##{action_name}, rendering head :no_content" if logger + super end end @@ -32,5 +83,11 @@ module ActionController "default_render" end end + + private + + def interactive_browser_request? + request.format == Mime[:html] && !request.xhr? + end end end diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 25ec3cf5b6..a01110d474 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -579,7 +579,7 @@ module ActionController end def inspect - "<#{self.class} #{@parameters}>" + "<#{self.class} #{@parameters} permitted: #{@permitted}>" end def method_missing(method_sym, *args, &block) diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 0c4b661214..700317614f 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -52,7 +52,7 @@ module ActionController self.session = session self.session_options = TestSession::DEFAULT_OPTIONS @custom_param_parsers = { - Mime[:xml] => lambda { |raw_post| Hash.from_xml(raw_post)['hash'] } + xml: lambda { |raw_post| Hash.from_xml(raw_post)['hash'] } } end @@ -105,7 +105,7 @@ module ActionController when :url_encoded_form data = non_path_parameters.to_query else - @custom_param_parsers[content_mime_type] = ->(_) { non_path_parameters } + @custom_param_parsers[content_mime_type.symbol] = ->(_) { non_path_parameters } data = non_path_parameters.to_query end end diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index cca7376ffa..ff5031d7d5 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -1,22 +1,31 @@ module ActionDispatch module Http module Parameters + extend ActiveSupport::Concern + PARAMETERS_KEY = 'action_dispatch.request.path_parameters' DEFAULT_PARSERS = { - Mime[:json] => lambda { |raw_post| + Mime[:json].symbol => -> (raw_post) { data = ActiveSupport::JSON.decode(raw_post) data.is_a?(Hash) ? data : {:_json => data} } } - def self.included(klass) - class << klass - attr_accessor :parameter_parsers + included do + class << self + attr_reader :parameter_parsers end - klass.parameter_parsers = DEFAULT_PARSERS + self.parameter_parsers = DEFAULT_PARSERS end + + module ClassMethods + def parameter_parsers=(parsers) # :nodoc: + @parameter_parsers = parsers.transform_keys { |key| key.respond_to?(:symbol) ? key.symbol : key } + end + end + # Returns both GET and POST \parameters in a single hash. def parameters params = get_header("action_dispatch.request.parameters") @@ -51,7 +60,7 @@ module ActionDispatch def parse_formatted_parameters(parsers) return yield if content_length.zero? - strategy = parsers.fetch(content_mime_type) { return yield } + strategy = parsers.fetch(content_mime_type.symbol) { return yield } begin strategy.call(raw_post) diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 5427425ef7..316a9f08b7 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -403,6 +403,10 @@ module ActionDispatch def commit_flash end + def ssl? + super || scheme == 'wss'.freeze + end + private def check_method(name) HTTP_METHOD_LOOKUP[name] || raise(ActionController::UnknownHttpMethod, "#{name}, accepted HTTP methods are #{HTTP_METHODS[0...-1].join(', ')}, and #{HTTP_METHODS[-1]}") diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index 35c2b1b86e..cfd6681dd1 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -3,7 +3,7 @@ module ActionDispatch class Route # :nodoc: attr_reader :app, :path, :defaults, :name, :precedence - attr_reader :constraints + attr_reader :constraints, :internal alias :conditions :constraints module VerbMatchers @@ -55,7 +55,7 @@ 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, required_defaults, defaults, request_method_match, precedence) + def initialize(name, app, path, constraints, required_defaults, defaults, request_method_match, precedence, internal = false) @name = name @app = app @path = path @@ -70,6 +70,7 @@ module ActionDispatch @decorated_ast = nil @precedence = precedence @path_formatter = @path.build_formatter + @internal = internal end def ast @@ -81,7 +82,7 @@ module ActionDispatch end def requirements # :nodoc: - # needed for rails `rake routes` + # needed for rails `rails routes` @defaults.merge(path.requirements).delete_if { |_,v| /.+?/ == v } diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index c2a4f46e67..5841c978af 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -37,6 +37,7 @@ module ActionDispatch # The +parsers+ argument can take Hash of parsers where key is identifying # content mime type, and value is a lambda that is going to process data. def self.new(app, parsers = {}) + parsers = parsers.transform_keys { |key| key.respond_to?(:symbol) ? key.symbol : key } ActionDispatch::Request.parameter_parsers = ActionDispatch::Request::DEFAULT_PARSERS.merge(parsers) app end diff --git a/actionpack/lib/action_dispatch/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb index 735b5939dd..711d8b016a 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -23,7 +23,7 @@ module ActionDispatch # preload lists is `18.weeks`. # * `subdomains`: Set to `true` to tell the browser to apply these settings # to all subdomains. This protects your cookies from interception by a - # vulnerable site on a subdomain. Defaults to `false`. + # vulnerable site on a subdomain. Defaults to `true`. # * `preload`: Advertise that this site may be included in browsers' # preloaded HSTS lists. HSTS protects your site on every visit *except the # first visit* since it hasn't seen your HSTS header yet. To close this @@ -49,7 +49,7 @@ module ActionDispatch if options[:host] || options[:port] ActiveSupport::Deprecation.warn <<-end_warning.strip_heredoc The `:host` and `:port` options are moving within `:redirect`: - `config.ssl_options = { redirect: { host: …, port: … }}`. + `config.ssl_options = { redirect: { host: …, port: … } }`. end_warning @redirect = options.slice(:host, :port) else @@ -57,6 +57,17 @@ module ActionDispatch end @secure_cookies = secure_cookies + + if hsts != true && hsts != false && hsts[:subdomains].nil? + hsts[:subdomains] = false + + ActiveSupport::Deprecation.warn <<-end_warning.strip_heredoc + In Rails 5.1, The `:subdomains` option of HSTS config will be treated as true if + unspecified. Set `config.ssl_options = { hsts: { subdomains: false } }` to opt out + of this behavior. + end_warning + end + @hsts_header = build_hsts_header(normalize_hsts_options(hsts)) end diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index 983f1daeb3..5d30a545a2 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -41,7 +41,7 @@ module ActionDispatch end def internal? - controller.to_s =~ %r{\Arails/(info|mailers|welcome)} + internal end def engine? @@ -51,7 +51,7 @@ module ActionDispatch ## # This class is just used for displaying route information when someone - # executes `rake routes` or looks at the RoutingError page. + # executes `rails routes` or looks at the RoutingError page. # People should not use this class. class RoutesInspector # :nodoc: def initialize(routes) diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index afbaa45d20..16b430c36e 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -107,6 +107,7 @@ module ActionDispatch @ast = ast @anchor = anchor @via = via + @internal = options[:internal] path_params = ast.find_all(&:symbol?).map(&:to_sym) @@ -148,7 +149,8 @@ module ActionDispatch required_defaults, defaults, request_method, - precedence) + precedence, + @internal) route end diff --git a/actionpack/lib/action_pack/gem_version.rb b/actionpack/lib/action_pack/gem_version.rb index 778c5482d3..157f401f54 100644 --- a/actionpack/lib/action_pack/gem_version.rb +++ b/actionpack/lib/action_pack/gem_version.rb @@ -8,7 +8,7 @@ module ActionPack MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "beta2" + PRE = "beta3" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end diff --git a/actionpack/test/controller/force_ssl_test.rb b/actionpack/test/controller/force_ssl_test.rb index 22f1cc7c22..03a9c9ae78 100644 --- a/actionpack/test/controller/force_ssl_test.rb +++ b/actionpack/test/controller/force_ssl_test.rb @@ -322,3 +322,12 @@ class RedirectToSSLTest < ActionController::TestCase assert_equal 'ihaz', response.body end end + +class ForceSSLControllerLevelTest < ActionController::TestCase + def test_no_redirect_websocket_ssl_request + request.env['rack.url_scheme'] = 'wss' + request.env['Upgrade'] = 'websocket' + get :cheeseburger + assert_response 200 + end +end diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index ea50f05f4d..6277407ff7 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -390,7 +390,7 @@ class IntegrationTestUsesCorrectClass < ActionDispatch::IntegrationTest reset! %w( get post head patch put delete ).each do |verb| - assert_nothing_raised("'#{verb}' should use integration test methods") { __send__(verb, '/') } + assert_nothing_raised { __send__(verb, '/') } end end end diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index 76e2d3ff43..d0c7b2e06a 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -160,7 +160,14 @@ class RespondToController < ActionController::Base end end - def variant_with_implicit_rendering + def variant_with_implicit_template_rendering + # This has exactly one variant template defined in the file system (+mobile.html.erb), + # which raises the regular MissingTemplate error for other variants. + end + + def variant_without_implicit_template_rendering + # This differs from the above in that it does not have any templates defined in the file + # system, which triggers the ImplicitRender (204 No Content) behavior. end def variant_with_format_and_custom_render @@ -272,6 +279,8 @@ class RespondToController < ActionController::Base end class RespondToControllerTest < ActionController::TestCase + NO_CONTENT_WARNING = "No template found for RespondToController#variant_without_implicit_template_rendering, rendering head :no_content" + def setup super @request.host = "www.example.com" @@ -616,30 +625,69 @@ class RespondToControllerTest < ActionController::TestCase end def test_invalid_variant + assert_raises(ActionController::UnknownFormat) do + get :variant_with_implicit_template_rendering, params: { v: :invalid } + end + end + + def test_variant_not_set_regular_unknown_format + assert_raises(ActionController::UnknownFormat) do + get :variant_with_implicit_template_rendering + end + end + + def test_variant_with_implicit_template_rendering + get :variant_with_implicit_template_rendering, params: { v: :mobile } + assert_equal "text/html", @response.content_type + assert_equal "mobile", @response.body + end + + def test_variant_without_implicit_rendering_from_browser + assert_raises(ActionController::UnknownFormat) do + get :variant_without_implicit_template_rendering, params: { v: :does_not_matter } + end + end + + def test_variant_variant_not_set_and_without_implicit_rendering_from_browser + assert_raises(ActionController::UnknownFormat) do + get :variant_without_implicit_template_rendering + end + end + + def test_variant_without_implicit_rendering_from_xhr logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new old_logger, ActionController::Base.logger = ActionController::Base.logger, logger - get :variant_with_implicit_rendering, params: { v: :invalid } + get :variant_without_implicit_template_rendering, xhr: true, params: { v: :does_not_matter } assert_response :no_content - assert_equal 1, logger.logged(:info).select{ |s| s =~ /No template found/ }.size, "Implicit head :no_content not logged" + + assert_equal 1, logger.logged(:info).select{ |s| s == NO_CONTENT_WARNING }.size, "Implicit head :no_content not logged" ensure ActionController::Base.logger = old_logger end - def test_variant_not_set_regular_template_missing - get :variant_with_implicit_rendering + def test_variant_without_implicit_rendering_from_api + logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new + old_logger, ActionController::Base.logger = ActionController::Base.logger, logger + + get :variant_without_implicit_template_rendering, format: 'json', params: { v: :does_not_matter } assert_response :no_content + + assert_equal 1, logger.logged(:info).select{ |s| s == NO_CONTENT_WARNING }.size, "Implicit head :no_content not logged" + ensure + ActionController::Base.logger = old_logger end - def test_variant_with_implicit_rendering - get :variant_with_implicit_rendering, params: { v: :implicit } + def test_variant_variant_not_set_and_without_implicit_rendering_from_xhr + logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new + old_logger, ActionController::Base.logger = ActionController::Base.logger, logger + + get :variant_without_implicit_template_rendering, xhr: true assert_response :no_content - end - def test_variant_with_implicit_template_rendering - get :variant_with_implicit_rendering, params: { v: :mobile } - assert_equal "text/html", @response.content_type - assert_equal "mobile", @response.body + assert_equal 1, logger.logged(:info).select { |s| s == NO_CONTENT_WARNING }.size, "Implicit head :no_content not logged" + ensure + ActionController::Base.logger = old_logger end def test_variant_with_format_and_custom_render @@ -778,24 +826,3 @@ class RespondToControllerTest < ActionController::TestCase assert_equal "phone", @response.body end end - -class RespondToWithBlockOnDefaultRenderController < ActionController::Base - def show - default_render do - render body: 'default_render yielded' - end - end -end - -class RespondToWithBlockOnDefaultRenderControllerTest < ActionController::TestCase - def setup - super - @request.host = "www.example.com" - end - - def test_default_render_uses_block_when_no_template_exists - get :show - assert_equal "default_render yielded", @response.body - assert_equal "text/plain", @response.content_type - end -end diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb index 4ef5bed30d..cea265f9ab 100644 --- a/actionpack/test/controller/parameters/accessors_test.rb +++ b/actionpack/test/controller/parameters/accessors_test.rb @@ -4,6 +4,8 @@ require 'active_support/core_ext/hash/transform_values' class ParametersAccessorsTest < ActiveSupport::TestCase setup do + ActionController::Parameters.permit_all_parameters = false + @params = ActionController::Parameters.new( person: { age: '32', @@ -176,12 +178,20 @@ class ParametersAccessorsTest < ActiveSupport::TestCase assert(@params != false) end - test "inspect shows both class name and parameters" do + test "inspect shows both class name, parameters and permitted flag" do assert_equal( '<ActionController::Parameters {"person"=>{"age"=>"32", '\ - '"name"=>{"first"=>"David", "last"=>"Heinemeier Hansson"}, ' \ - '"addresses"=>[{"city"=>"Chicago", "state"=>"Illinois"}]}}>', + '"name"=>{"first"=>"David", "last"=>"Heinemeier Hansson"}, ' \ + '"addresses"=>[{"city"=>"Chicago", "state"=>"Illinois"}]}} permitted: false>', @params.inspect ) end + + test "inspect prints updated permitted flag in the output" do + assert_match(/permitted: false/, @params.inspect) + + @params.permit! + + assert_match(/permitted: true/, @params.inspect) + end end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 60c6518c62..83d7405e4d 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -26,6 +26,9 @@ end class ImplicitRenderTestController < ActionController::Base def empty_action end + + def empty_action_with_template + end end class TestController < ActionController::Base @@ -537,10 +540,28 @@ end class ImplicitRenderTest < ActionController::TestCase tests ImplicitRenderTestController - def test_implicit_no_content_response - get :empty_action + def test_implicit_no_content_response_as_browser + assert_raises(ActionController::UnknownFormat) do + get :empty_action + end + end + + def test_implicit_no_content_response_as_xhr + get :empty_action, xhr: true assert_response :no_content end + + def test_implicit_success_response_with_right_format + get :empty_action_with_template + assert_equal "<h1>Empty action rendered this implicitly.</h1>\n", @response.body + assert_response :success + end + + def test_implicit_unknown_format_response + assert_raises(ActionController::UnknownFormat) do + get :empty_action_with_template, format: 'json' + end + end end class HeadRenderTest < ActionController::TestCase diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index c645af88d7..f7dcbc1984 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -133,11 +133,11 @@ class PerFormTokensController < ActionController::Base self.per_form_csrf_tokens = true def index - render inline: "<%= form_tag (params[:form_path] || '/per_form_tokens/post_one'), method: (params[:form_method] || :post) %>" + render inline: "<%= form_tag (params[:form_path] || '/per_form_tokens/post_one'), method: params[:form_method] %>" end def button_to - render inline: "<%= button_to 'Button', (params[:form_path] || '/per_form_tokens/post_one'), method: (params[:form_method] || :post) %>" + render inline: "<%= button_to 'Button', (params[:form_path] || '/per_form_tokens/post_one'), method: params[:form_method] %>" end def post_one @@ -710,6 +710,20 @@ class PerFormTokensControllerTest < ActionController::TestCase end end + test "Accepts proper token for implicit post method on button_to tag" do + get :button_to + + form_token = assert_presence_and_fetch_form_csrf_token + + assert_matches_session_token_on_server form_token, 'post' + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one' + assert_nothing_raised do + post :post_one, params: { custom_authenticity_token: form_token } + end + end + %w{delete post patch}.each do |verb| test "Accepts proper token for #{verb} method on button_to tag" do get :button_to, params: { form_method: verb } diff --git a/actionpack/test/controller/webservice_test.rb b/actionpack/test/controller/webservice_test.rb index 6d377c4691..daf17558aa 100644 --- a/actionpack/test/controller/webservice_test.rb +++ b/actionpack/test/controller/webservice_test.rb @@ -99,7 +99,7 @@ class WebServiceTest < ActionDispatch::IntegrationTest def test_parsing_json_doesnot_rescue_exception req = Class.new(ActionDispatch::Request) do def params_parsers - { Mime[:json] => Proc.new { |data| raise Interrupt } } + { json: Proc.new { |data| raise Interrupt } } end def content_length; get_header('rack.input').length; end diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index 64801bff39..3655c7f570 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -150,6 +150,34 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest ) end + test "parses json params after custom json mime type registered" do + begin + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w(application/vnd.api+json) + assert_parses( + {"user" => {"username" => "meinac"}, "username" => "meinac"}, + "{\"username\": \"meinac\"}", { 'CONTENT_TYPE' => 'application/json' } + ) + ensure + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) + end + end + + test "parses json params after custom json mime type registered with synonym" do + begin + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w(application/vnd.api+json) + assert_parses( + {"user" => {"username" => "meinac"}, "username" => "meinac"}, + "{\"username\": \"meinac\"}", { 'CONTENT_TYPE' => 'application/vnd.api+json' } + ) + ensure + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) + end + end + private def assert_parses(expected, actual, headers = {}) with_test_routing(UsersController) do diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 8b3849cb7a..cd385982d9 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -445,4 +445,12 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest assert_equal('application/xml; charset=utf-16', @response.headers['Content-Type']) end + + test "we can set strong ETag by directly adding it as header" do + @response = ActionDispatch::Response.create + @response.add_header "ETag", '"202cb962ac59075b964b07152d234b70"' + + assert_equal('"202cb962ac59075b964b07152d234b70"', @response.etag) + assert_equal('"202cb962ac59075b964b07152d234b70"', @response.headers['ETag']) + end end diff --git a/actionpack/test/dispatch/routing/inspector_test.rb b/actionpack/test/dispatch/routing/inspector_test.rb index f72a87b994..fd85cc6e9f 100644 --- a/actionpack/test/dispatch/routing/inspector_test.rb +++ b/actionpack/test/dispatch/routing/inspector_test.rb @@ -389,6 +389,29 @@ module ActionDispatch ], output end + def test_displaying_routes_for_internal_engines + engine = Class.new(Rails::Engine) do + def self.inspect + "Blog::Engine" + end + end + engine.routes.draw do + get '/cart', to: 'cart#show' + post '/cart', to: 'cart#create' + patch '/cart', to: 'cart#update' + end + + output = draw do + get '/custom/assets', to: 'custom_assets#show' + mount engine => "/blog", as: "blog", internal: true + end + + assert_equal [ + " Prefix Verb URI Pattern Controller#Action", + "custom_assets GET /custom/assets(.:format) custom_assets#show", + ], output + end + end end end diff --git a/actionpack/test/dispatch/ssl_test.rb b/actionpack/test/dispatch/ssl_test.rb index c66a0e6a7a..18ff894b31 100644 --- a/actionpack/test/dispatch/ssl_test.rb +++ b/actionpack/test/dispatch/ssl_test.rb @@ -7,7 +7,7 @@ class SSLTest < ActionDispatch::IntegrationTest def build_app(headers: {}, ssl_options: {}) headers = HEADERS.merge(headers) - ActionDispatch::SSL.new lambda { |env| [200, headers, []] }, ssl_options + ActionDispatch::SSL.new lambda { |env| [200, headers, []] }, ssl_options.reverse_merge(hsts: { subdomains: true }) end end @@ -98,15 +98,16 @@ end class StrictTransportSecurityTest < SSLTest EXPECTED = 'max-age=15552000' + EXPECTED_WITH_SUBDOMAINS = 'max-age=15552000; includeSubDomains' - def assert_hsts(expected, url: 'https://example.org', hsts: {}, headers: {}) + def assert_hsts(expected, url: 'https://example.org', hsts: { subdomains: true }, headers: {}) self.app = build_app ssl_options: { hsts: hsts }, headers: headers get url assert_equal expected, response.headers['Strict-Transport-Security'] end test 'enabled by default' do - assert_hsts EXPECTED + assert_hsts EXPECTED_WITH_SUBDOMAINS end test 'not sent with http:// responses' do @@ -126,11 +127,15 @@ class StrictTransportSecurityTest < SSLTest end test ':expires sets max-age' do - assert_hsts 'max-age=500', hsts: { expires: 500 } + assert_deprecated do + assert_hsts 'max-age=500', hsts: { expires: 500 } + end end test ':expires supports AS::Duration arguments' do - assert_hsts 'max-age=31557600', hsts: { expires: 1.year } + assert_deprecated do + assert_hsts 'max-age=31557600', hsts: { expires: 1.year } + end end test 'include subdomains' do @@ -142,11 +147,15 @@ class StrictTransportSecurityTest < SSLTest end test 'opt in to browser preload lists' do - assert_hsts "#{EXPECTED}; preload", hsts: { preload: true } + assert_deprecated do + assert_hsts "#{EXPECTED}; preload", hsts: { preload: true } + end end test 'opt out of browser preload lists' do - assert_hsts EXPECTED, hsts: { preload: false } + assert_deprecated do + assert_hsts EXPECTED, hsts: { preload: false } + end end end diff --git a/actionpack/test/fixtures/implicit_render_test/empty_action_with_mobile_variant.html+mobile.erb b/actionpack/test/fixtures/implicit_render_test/empty_action_with_mobile_variant.html+mobile.erb new file mode 100644 index 0000000000..ded99ba52d --- /dev/null +++ b/actionpack/test/fixtures/implicit_render_test/empty_action_with_mobile_variant.html+mobile.erb @@ -0,0 +1 @@ +mobile diff --git a/actionpack/test/fixtures/implicit_render_test/empty_action_with_template.html.erb b/actionpack/test/fixtures/implicit_render_test/empty_action_with_template.html.erb new file mode 100644 index 0000000000..dd294f8cf6 --- /dev/null +++ b/actionpack/test/fixtures/implicit_render_test/empty_action_with_template.html.erb @@ -0,0 +1 @@ +<h1>Empty action rendered this implicitly.</h1> diff --git a/actionpack/test/fixtures/respond_to/variant_with_implicit_rendering.html+mobile.erb b/actionpack/test/fixtures/respond_to/variant_with_implicit_template_rendering.html+mobile.erb index 317801ad30..317801ad30 100644 --- a/actionpack/test/fixtures/respond_to/variant_with_implicit_rendering.html+mobile.erb +++ b/actionpack/test/fixtures/respond_to/variant_with_implicit_template_rendering.html+mobile.erb |