diff options
Diffstat (limited to 'actionpack')
20 files changed, 133 insertions, 32 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 0ae96441ce..4ab0857a66 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,11 @@ +* 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 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/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 2273406948..c492b7fb64 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.gsub('"'.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.gsub('"'.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/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 367b736035..663a969f72 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,20 @@ 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 diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index e30c9c5ade..c98e937423 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -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_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/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index fdd1bc4e69..9a92b690c7 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -28,7 +28,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 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/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/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/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/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..f153030675 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -2,6 +2,16 @@ require 'abstract_unit' require 'zlib' module StaticTests + def setup + @default_internal_encoding = Encoding.default_internal + @default_external_encoding = Encoding.default_external + end + + def teardown + Encoding.default_internal = @default_internal_encoding + Encoding.default_external = @default_external_encoding + end + def test_serves_dynamic_content assert_equal "Hello, World!", get("/nofile").body end @@ -10,6 +20,16 @@ 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 + Encoding.default_internal = "Windows-31J" + Encoding.default_external = "Windows-31J" + 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 +228,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 +258,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/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, {}, {}, {} |