diff options
Diffstat (limited to 'actionpack')
42 files changed, 355 insertions, 138 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 152ec3700b..a13d9e1078 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,28 @@ +* Introduce a new error page to when the implict render page is accessed in the browser. + + Now instead of showing an error page that with exception and backtraces we now show only + one informative page. + + *Vinicius Stock* + +* Introduce ActionDispatch::DebugExceptions.register_interceptor + + Exception aware plugin authors can use the newly introduced + `.register_interceptor` method to get the processed exception, instead of + monkey patching DebugExceptions. + + ActionDispatch::DebugExceptions.register_interceptor do |request, exception| + HypoteticalPlugin.capture_exception(request, exception) + end + + *Genadi Samokovarov* + +* Output only one Content-Security-Policy nonce header value per request. + + Fixes #32597. + + *Andrey Novikov*, *Andrew White* + * Move default headers configuration into their own module that can be included in controllers. *Kevin Deisz* diff --git a/actionpack/Rakefile b/actionpack/Rakefile index 4dd7c59ce8..e99eb1723a 100644 --- a/actionpack/Rakefile +++ b/actionpack/Rakefile @@ -28,7 +28,7 @@ namespace :test do end task :lines do - load File.expand_path("..", __dir__) + "/tools/line_statistics" + load File.expand_path("../tools/line_statistics", __dir__) files = FileList["lib/**/*.rb"] CodeTools::LineStatistics.new(files).print_loc end diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 3378d6db0f..2e565d5d44 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -78,7 +78,7 @@ module ActionController # # You can retrieve it again through the same hash: # - # Hello #{session[:person]} + # "Hello #{session[:person]}" # # For removing objects from the session, you can either assign a single key to +nil+: # diff --git a/actionpack/lib/action_controller/metal/exceptions.rb b/actionpack/lib/action_controller/metal/exceptions.rb index 58db4a1458..ce9eb209fe 100644 --- a/actionpack/lib/action_controller/metal/exceptions.rb +++ b/actionpack/lib/action_controller/metal/exceptions.rb @@ -50,4 +50,7 @@ module ActionController class UnknownFormat < ActionControllerError #:nodoc: end + + class MissingExactTemplate < UnknownFormat #:nodoc: + end end diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index ac0c127cdc..d3bb58f48b 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -41,18 +41,8 @@ module ActionController raise ActionController::UnknownFormat, message elsif interactive_browser_request? - message = "#{self.class.name}\##{action_name} is missing a template " \ - "for this request format and variant.\n\n" \ - "request.formats: #{request.formats.map(&:to_s).inspect}\n" \ - "request.variant: #{request.variant.inspect}\n\n" \ - "NOTE! For XHR/Ajax or API requests, this action would normally " \ - "respond with 204 No Content: an empty white screen. Since you're " \ - "loading it in a web browser, we assume that you expected to " \ - "actually render a template, not nothing, so we're showing an " \ - "error to be extra-clear. If you expect 204 No Content, carry on. " \ - "That's what you'll get from an XHR or API request. Give it a shot." - - raise ActionController::UnknownFormat, message + message = "#{self.class.name}\##{action_name} is missing a template for request formats: #{request.formats.map(&:to_s).join(',')}" + raise ActionController::MissingExactTemplate, message else logger.info "No template found for #{self.class.name}\##{action_name}, rendering head :no_content" if logger super diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index fc9cf8aaff..4bae795438 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -275,7 +275,7 @@ module ActionController #:nodoc: # Check for cross-origin JavaScript responses. def non_xhr_javascript_response? # :doc: - content_type =~ %r(\Atext/javascript) && !request.xhr? + content_type =~ %r(\A(?:text|application)/javascript) && !request.xhr? end AUTHENTICITY_TOKEN_LENGTH = 32 @@ -400,9 +400,14 @@ module ActionController #:nodoc: end def xor_byte_strings(s1, s2) # :doc: - s2_bytes = s2.bytes - s1.each_byte.with_index { |c1, i| s2_bytes[i] ^= c1 } - s2_bytes.pack("C*") + s2 = s2.dup + size = s1.bytesize + i = 0 + while i < size + s2.setbyte(i, s1.getbyte(i) ^ s2.getbyte(i)) + i += 1 + end + s2 end # The form's authenticity parameter. Override to provide your own. diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 4789252550..46c0e80194 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -374,7 +374,7 @@ module ActionController # Person.new(params) # => #<Person id: nil, name: "Francesco"> def permit! each_pair do |key, value| - Array.wrap(value).each do |v| + Array.wrap(value).flatten.each do |v| v.permit! if v.respond_to? :permit! end end @@ -560,12 +560,14 @@ module ActionController # Returns a parameter for the given +key+. If the +key+ # can't be found, there are several options: With no other arguments, # it will raise an <tt>ActionController::ParameterMissing</tt> error; - # if more arguments are given, then that will be returned; if a block + # if a second argument is given, then that is returned (converted to an + # instance of ActionController::Parameters if possible); if a block # is given, then that will be run and its result returned. # # params = ActionController::Parameters.new(person: { name: "Francesco" }) # params.fetch(:person) # => <ActionController::Parameters {"name"=>"Francesco"} permitted: false> # params.fetch(:none) # => ActionController::ParameterMissing: param is missing or the value is empty: none + # params.fetch(:none, {}) # => <ActionController::Parameters {} permitted: false> # params.fetch(:none, "Francesco") # => "Francesco" # params.fetch(:none) { "Francesco" } # => "Francesco" def fetch(key, *args) diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 798d142755..33b0bcbefe 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -109,7 +109,7 @@ module ActionController when :xml data = non_path_parameters.to_xml when :url_encoded_form - data = non_path_parameters.to_query + data = Rack::Utils.build_nested_query(non_path_parameters) else @custom_param_parsers[content_mime_type.symbol] = ->(_) { non_path_parameters } data = non_path_parameters.to_query @@ -460,10 +460,6 @@ module ActionController def process(action, method: "GET", params: {}, session: nil, body: nil, flash: {}, format: nil, xhr: false, as: nil) check_required_ivars - if body - @request.set_header "RAW_POST_DATA", body - end - http_method = method.to_s.upcase @html_document = nil @@ -478,6 +474,10 @@ module ActionController @response.request = @request @controller.recycle! + if body + @request.set_header "RAW_POST_DATA", body + end + @request.set_header "REQUEST_METHOD", http_method if as @@ -604,6 +604,8 @@ module ActionController env.delete "action_dispatch.request.query_parameters" env.delete "action_dispatch.request.request_parameters" env["rack.input"] = StringIO.new + env.delete "CONTENT_LENGTH" + env.delete "RAW_POST_DATA" env end diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb index c1f80a1ffc..35041fd072 100644 --- a/actionpack/lib/action_dispatch/http/content_security_policy.rb +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -21,13 +21,8 @@ module ActionDispatch #:nodoc: return response if policy_present?(headers) if policy = request.content_security_policy - if policy.directives["script-src"] - if nonce = request.content_security_policy_nonce - policy.directives["script-src"] << "'nonce-#{nonce}'" - end - end - - headers[header_name(request)] = policy.build(request.controller_instance) + nonce = request.content_security_policy_nonce + headers[header_name(request)] = policy.build(request.controller_instance, nonce) end response @@ -131,12 +126,15 @@ module ActionDispatch #:nodoc: manifest_src: "manifest-src", media_src: "media-src", object_src: "object-src", + prefetch_src: "prefetch-src", script_src: "script-src", style_src: "style-src", worker_src: "worker-src" }.freeze - private_constant :MAPPINGS, :DIRECTIVES + NONCE_DIRECTIVES = %w[script-src].freeze + + private_constant :MAPPINGS, :DIRECTIVES, :NONCE_DIRECTIVES attr_reader :directives @@ -205,8 +203,8 @@ module ActionDispatch #:nodoc: end end - def build(context = nil) - build_directives(context).compact.join("; ") + def build(context = nil, nonce = nil) + build_directives(context, nonce).compact.join("; ") end private @@ -229,10 +227,14 @@ module ActionDispatch #:nodoc: end end - def build_directives(context) + def build_directives(context, nonce) @directives.map do |directive, sources| if sources.is_a?(Array) - "#{directive} #{build_directive(sources, context).join(' ')}" + if nonce && nonce_directive?(directive) + "#{directive} #{build_directive(sources, context).join(' ')} 'nonce-#{nonce}'" + else + "#{directive} #{build_directive(sources, context).join(' ')}" + end elsif sources directive else @@ -261,5 +263,9 @@ module ActionDispatch #:nodoc: raise RuntimeError, "Unexpected content security policy source: #{source.inspect}" end end + + def nonce_directive?(directive) + NONCE_DIRECTIVES.include?(directive) + end end end diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb index ec86b8bc47..ec012ad02d 100644 --- a/actionpack/lib/action_dispatch/http/filter_parameters.rb +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -9,8 +9,8 @@ module ActionDispatch # sub-hashes of the params hash to filter. Filtering only certain sub-keys # from a hash is possible by using the dot notation: 'credit_card.number'. # If a block is given, each key and value of the params hash and all - # sub-hashes is passed to it, where the value or the key can be replaced using - # String#replace or similar method. + # sub-hashes are passed to it, where the value or the key can be replaced using + # String#replace or similar methods. # # env["action_dispatch.parameter_filter"] = [:password] # => replaces the value to all keys matching /password/i with "[FILTERED]" diff --git a/actionpack/lib/action_dispatch/journey/nfa/simulator.rb b/actionpack/lib/action_dispatch/journey/nfa/simulator.rb index 8efe48d91c..002f6feb97 100644 --- a/actionpack/lib/action_dispatch/journey/nfa/simulator.rb +++ b/actionpack/lib/action_dispatch/journey/nfa/simulator.rb @@ -25,8 +25,6 @@ module ActionDispatch state = tt.eclosure(0) until input.eos? sym = input.scan(%r([/.?]|[^/.?]+)) - - # FIXME: tt.eclosure is not needed for the GTG state = tt.eclosure(tt.move(state, sym)) end diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 511306eb0e..33edad8bd9 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -50,10 +50,18 @@ module ActionDispatch end end - def initialize(app, routes_app = nil, response_format = :default) + cattr_reader :interceptors, instance_accessor: false, default: [] + + def self.register_interceptor(object = nil, &block) + interceptor = object || block + interceptors << interceptor + end + + def initialize(app, routes_app = nil, response_format = :default, interceptors = self.class.interceptors) @app = app @routes_app = routes_app @response_format = response_format + @interceptors = interceptors end def call(env) @@ -67,12 +75,26 @@ module ActionDispatch response rescue Exception => exception + invoke_interceptors(request, exception) raise exception unless request.show_exceptions? render_exception(request, exception) end private + def invoke_interceptors(request, exception) + backtrace_cleaner = request.get_header("action_dispatch.backtrace_cleaner") + wrapper = ExceptionWrapper.new(backtrace_cleaner, exception) + + @interceptors.each do |interceptor| + begin + interceptor.call(request, exception) + rescue Exception + log_error(request, wrapper) + end + end + end + def render_exception(request, exception) backtrace_cleaner = request.get_header("action_dispatch.backtrace_cleaner") wrapper = ExceptionWrapper.new(backtrace_cleaner, exception) diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index d1b4508378..f05c69137b 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -12,6 +12,7 @@ module ActionDispatch "ActionController::UnknownHttpMethod" => :method_not_allowed, "ActionController::NotImplemented" => :not_implemented, "ActionController::UnknownFormat" => :not_acceptable, + "ActionController::MissingExactTemplate" => :not_acceptable, "ActionController::InvalidAuthenticityToken" => :unprocessable_entity, "ActionController::InvalidCrossOriginRequest" => :unprocessable_entity, "ActionDispatch::Http::Parameters::ParseError" => :bad_request, @@ -22,11 +23,12 @@ module ActionDispatch ) cattr_accessor :rescue_templates, default: Hash.new("diagnostics").merge!( - "ActionView::MissingTemplate" => "missing_template", - "ActionController::RoutingError" => "routing_error", - "AbstractController::ActionNotFound" => "unknown_action", - "ActiveRecord::StatementInvalid" => "invalid_statement", - "ActionView::Template::Error" => "template_error" + "ActionView::MissingTemplate" => "missing_template", + "ActionController::RoutingError" => "routing_error", + "AbstractController::ActionNotFound" => "unknown_action", + "ActiveRecord::StatementInvalid" => "invalid_statement", + "ActionView::Template::Error" => "template_error", + "ActionController::MissingExactTemplate" => "missing_exact_template", ) attr_reader :backtrace_cleaner, :exception, :line_number, :file diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index acd999444a..8130bfe2e7 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -69,7 +69,7 @@ module ActionDispatch headers["Vary"] = "Accept-Encoding" if gzip_path - return [status, headers, body] + [status, headers, body] ensure request.path_info = path end diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_exact_template.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_exact_template.html.erb new file mode 100644 index 0000000000..76ab1691b5 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_exact_template.html.erb @@ -0,0 +1,19 @@ +<header> + <h1>No template for interactive request</h1> +</header> + +<div id="container"> + <h2><%= h @exception.message %></h2> + + <p class="summary"> + <strong>NOTE!</strong><br> + Unless told otherwise, Rails expects an action to render a template with the same name,<br> + contained in a folder named after its controller. + + If this controller is an API responding with 204 (No Content), <br> + which does not require a template, + then this error will occur when trying to access it via browser,<br> + since we expect an HTML template + to be rendered for such requests. If that's the case, carry on. + </p> +</div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_exact_template.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_exact_template.text.erb new file mode 100644 index 0000000000..fcdbe6069d --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_exact_template.text.erb @@ -0,0 +1,3 @@ +Missing exact template + +<%= @exception.message %> diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index a29a5a04ef..1134279a7f 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -35,7 +35,7 @@ module ActionDispatch if @raise_on_name_error raise else - return [404, { "X-Cascade" => "pass" }, []] + [404, { "X-Cascade" => "pass" }, []] end end diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb index 2c8cee3a9b..d2685e0452 100644 --- a/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb +++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb @@ -80,7 +80,7 @@ module ActionDispatch end def inline_base64(path) - Base64.encode64(path).gsub("\n", "") + Base64.strict_encode64(path) end def failed? diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 7171b6942c..f0398dc7b1 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -189,6 +189,12 @@ module ActionDispatch # merged into the Rack env hash. # - +env+: Additional env to pass, as a Hash. The headers will be # merged into the Rack env hash. + # - +xhr+: Set to `true` if you want to make and Ajax request. + # Adds request headers characteristic of XMLHttpRequest e.g. HTTP_X_REQUESTED_WITH. + # The headers will be merged into the Rack env hash. + # - +as+: Used for encoding the request with different content type. + # Supports `:json` by default and will set the approriate request headers. + # The headers will be merged into the Rack env hash. # # This method is rarely used directly. Use +#get+, +#post+, or other standard # HTTP methods in integration tests. +#process+ is only required when using a diff --git a/actionpack/test/abstract/callbacks_test.rb b/actionpack/test/abstract/callbacks_test.rb index fdc09bd951..4512ea27b3 100644 --- a/actionpack/test/abstract/callbacks_test.rb +++ b/actionpack/test/abstract/callbacks_test.rb @@ -154,7 +154,7 @@ module AbstractController test "when :except is specified, an after action is not triggered on that action" do @controller.process(:index) - assert !@controller.instance_variable_defined?("@authenticated") + assert_not @controller.instance_variable_defined?("@authenticated") end end @@ -198,7 +198,7 @@ module AbstractController test "when :except is specified with an array, an after action is not triggered on that action" do @controller.process(:index) - assert !@controller.instance_variable_defined?("@authenticated") + assert_not @controller.instance_variable_defined?("@authenticated") end end diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 504c77b8ef..763df3a776 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -290,13 +290,13 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase def test_template_objects_exist process :assign_this - assert !@controller.instance_variable_defined?(:"@hi") + assert_not @controller.instance_variable_defined?(:"@hi") assert @controller.instance_variable_get(:"@howdy") end def test_template_objects_missing process :nothing - assert !@controller.instance_variable_defined?(:@howdy) + assert_not @controller.instance_variable_defined?(:@howdy) end def test_empty_flash @@ -366,7 +366,7 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase process :redirect_external assert_predicate @response, :redirect? assert_match(/rubyonrails/, @response.redirect_url) - assert !/perloffrails/.match(@response.redirect_url) + assert_no_match(/perloffrails/, @response.redirect_url) end def test_redirection diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index a8f726f181..6fe036dd15 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -94,14 +94,14 @@ class FragmentCachingTest < ActionController::TestCase def test_fragment_exist_with_caching_enabled @store.write("views/name", "value") assert @controller.fragment_exist?("name") - assert !@controller.fragment_exist?("other_name") + assert_not @controller.fragment_exist?("other_name") end def test_fragment_exist_with_caching_disabled @controller.perform_caching = false @store.write("views/name", "value") - assert !@controller.fragment_exist?("name") - assert !@controller.fragment_exist?("other_name") + assert_not @controller.fragment_exist?("name") + assert_not @controller.fragment_exist?("other_name") end def test_write_fragment_with_caching_enabled @@ -144,7 +144,7 @@ class FragmentCachingTest < ActionController::TestCase buffer = "generated till now -> ".html_safe buffer << view_context.send(:fragment_for, "expensive") { fragment_computed = true } - assert !fragment_computed + assert_not fragment_computed assert_equal "generated till now -> fragment content", buffer end diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 2b16a555bb..425a6e25cc 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -787,7 +787,7 @@ class FilterTest < ActionController::TestCase assert_equal %w( ensure_login find_user ), @controller.instance_variable_get(:@ran_filter) test_process(ConditionalSkippingController, "login") - assert !@controller.instance_variable_defined?("@ran_after_action") + assert_not @controller.instance_variable_defined?("@ran_after_action") test_process(ConditionalSkippingController, "change_password") assert_equal %w( clean_up ), @controller.instance_variable_get("@ran_after_action") end diff --git a/actionpack/test/controller/flash_hash_test.rb b/actionpack/test/controller/flash_hash_test.rb index 6c3ac26de1..e3ec5bb7fc 100644 --- a/actionpack/test/controller/flash_hash_test.rb +++ b/actionpack/test/controller/flash_hash_test.rb @@ -44,7 +44,7 @@ module ActionDispatch @hash["foo"] = "bar" @hash.delete "foo" - assert !@hash.key?("foo") + assert_not @hash.key?("foo") assert_nil @hash["foo"] end @@ -53,7 +53,7 @@ module ActionDispatch assert_equal({ "foo" => "bar" }, @hash.to_hash) @hash.to_hash["zomg"] = "aaron" - assert !@hash.key?("zomg") + assert_not @hash.key?("zomg") assert_equal({ "foo" => "bar" }, @hash.to_hash) end diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb index 560157dc61..3f211cd60d 100644 --- a/actionpack/test/controller/http_digest_authentication_test.rb +++ b/actionpack/test/controller/http_digest_authentication_test.rb @@ -202,7 +202,7 @@ class HttpDigestAuthenticationTest < ActionController::TestCase test "validate_digest_response should fail with nil returning password_procedure" do @request.env["HTTP_AUTHORIZATION"] = encode_credentials(username: nil, password: nil) - assert !ActionController::HttpAuthentication::Digest.validate_digest_response(@request, "SuperSecret") { nil } + assert_not ActionController::HttpAuthentication::Digest.validate_digest_response(@request, "SuperSecret") { nil } end test "authentication request with request-uri ending in '/'" do diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 8a49d7822e..9cdf04b886 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -135,7 +135,7 @@ class IntegrationTestTest < ActiveSupport::TestCase session1 = @test.open_session { |sess| } session2 = @test.open_session # implicit session - assert !session1.equal?(session2) + assert_not session1.equal?(session2) end # RSpec mixes Matchers (which has a #method_missing) into diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index f9ffd5f54c..771eccb29b 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -658,13 +658,13 @@ class RespondToControllerTest < ActionController::TestCase end def test_variant_without_implicit_rendering_from_browser - assert_raises(ActionController::UnknownFormat) do + assert_raises(ActionController::MissingExactTemplate) 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 + assert_raises(ActionController::MissingExactTemplate) do get :variant_without_implicit_template_rendering end end diff --git a/actionpack/test/controller/parameters/nested_parameters_permit_test.rb b/actionpack/test/controller/parameters/nested_parameters_permit_test.rb index ccc6bf9807..1403e224c0 100644 --- a/actionpack/test/controller/parameters/nested_parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/nested_parameters_permit_test.rb @@ -5,7 +5,7 @@ require "action_controller/metal/strong_parameters" class NestedParametersPermitTest < ActiveSupport::TestCase def assert_filtered_out(params, key) - assert !params.has_key?(key), "key #{key.inspect} has not been filtered out" + assert_not params.has_key?(key), "key #{key.inspect} has not been filtered out" end test "permitted nested parameters" do diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index e60003dc64..d2fa0aa16e 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -6,7 +6,7 @@ require "action_controller/metal/strong_parameters" class ParametersPermitTest < ActiveSupport::TestCase def assert_filtered_out(params, key) - assert !params.has_key?(key), "key #{key.inspect} has not been filtered out" + assert_not params.has_key?(key), "key #{key.inspect} has not been filtered out" end setup do @@ -136,7 +136,7 @@ class ParametersPermitTest < ActiveSupport::TestCase test "key: it is not assigned if not present in params" do params = ActionController::Parameters.new(name: "Joe") permitted = params.permit(:id) - assert !permitted.has_key?(:id) + assert_not permitted.has_key?(:id) end test "key to empty array: empty arrays pass" do @@ -353,12 +353,15 @@ class ParametersPermitTest < ActiveSupport::TestCase assert_equal "Jonas", @params[:person][:family][:brother] end - test "permit is recursive" do + test "permit! is recursive" do + @params[:nested_array] = [[{ x: 2, y: 3 }, { x: 21, y: 42 }]] @params.permit! assert_predicate @params, :permitted? assert_predicate @params[:person], :permitted? assert_predicate @params[:person][:name], :permitted? assert_predicate @params[:person][:addresses][0], :permitted? + assert_predicate @params[:nested_array][0][0], :permitted? + assert_predicate @params[:nested_array][0][1], :permitted? end test "permitted takes a default value when Parameters.permit_all_parameters is set" do diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index fc21543049..24c5761e41 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -650,7 +650,7 @@ class ImplicitRenderTest < ActionController::TestCase tests ImplicitRenderTestController def test_implicit_no_content_response_as_browser - assert_raises(ActionController::UnknownFormat) do + assert_raises(ActionController::MissingExactTemplate) do get :empty_action end end diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 7a02c27c99..ea94a3e048 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -521,6 +521,11 @@ module RequestForgeryProtectionTests get :negotiate_same_origin end + assert_cross_origin_blocked do + @request.accept = "application/javascript" + get :negotiate_same_origin + end + assert_cross_origin_not_blocked { get :same_origin_js, xhr: true } assert_cross_origin_not_blocked { get :same_origin_js, xhr: true, format: "js" } assert_cross_origin_not_blocked do diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index 259f3b8855..9d0a8b4f00 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -1288,14 +1288,14 @@ class RouteSetTest < ActiveSupport::TestCase end def test_routing_traversal_does_not_load_extra_classes - assert !Object.const_defined?("Profiler__"), "Profiler should not be loaded" + assert_not Object.const_defined?("Profiler__"), "Profiler should not be loaded" set.draw do get "/profile" => "profile#index" end request_path_params("/profile") rescue nil - assert !Object.const_defined?("Profiler__"), "Profiler should not be loaded" + assert_not Object.const_defined?("Profiler__"), "Profiler should not be loaded" end def test_recognize_with_conditions_and_format diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index d1122abba6..d4620d085a 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -220,7 +220,15 @@ XML params = Hash[:page, { name: "page name" }, "some key", 123] post :render_raw_post, params: params.dup - assert_equal params.to_query, @response.body + assert_equal Rack::Utils.build_nested_query(params), @response.body + end + + def test_params_round_trip + params = {"foo"=>{"contents"=>[{"name"=>"gorby", "id"=>"123"}, {"name"=>"puff", "d"=>"true"}]}} + post :test_params, params: params.dup + + controller_info = { "controller" => "test_case_test/test", "action" => "test_params" } + assert_equal params.merge(controller_info), JSON.parse(@response.body) end def test_body_stream @@ -228,7 +236,7 @@ XML post :render_body, params: params.dup - assert_equal params.to_query, @response.body + assert_equal Rack::Utils.build_nested_query(params), @response.body end def test_document_body_and_params_with_post @@ -681,6 +689,22 @@ XML assert_equal "baz", @request.filtered_parameters[:foo] end + def test_raw_post_reset_between_post_requests + post :no_op, params: { foo: "bar" } + assert_equal "foo=bar", @request.raw_post + + post :no_op, params: { foo: "baz" } + assert_equal "foo=baz", @request.raw_post + end + + def test_content_length_reset_after_post_request + post :no_op, params: { foo: "bar" } + assert_not_equal 0, @request.content_length + + get :no_op + assert_equal 0, @request.content_length + end + def test_path_is_kept_after_the_request get :test_params, params: { id: "foo" } assert_equal "/test_case_test/test/test_params/foo", @request.path diff --git a/actionpack/test/dispatch/content_security_policy_test.rb b/actionpack/test/dispatch/content_security_policy_test.rb index 95fce39dad..4f9a4ff2bd 100644 --- a/actionpack/test/dispatch/content_security_policy_test.rb +++ b/actionpack/test/dispatch/content_security_policy_test.rb @@ -116,6 +116,12 @@ class ContentSecurityPolicyTest < ActiveSupport::TestCase @policy.object_src false assert_no_match %r{object-src}, @policy.build + @policy.prefetch_src :self + assert_match %r{prefetch-src 'self'}, @policy.build + + @policy.prefetch_src false + assert_no_match %r{prefetch-src}, @policy.build + @policy.script_src :self assert_match %r{script-src 'self'}, @policy.build @@ -200,7 +206,7 @@ class ContentSecurityPolicyTest < ActiveSupport::TestCase end def test_dynamic_directives - request = Struct.new(:host).new("www.example.com") + request = ActionDispatch::Request.new("HTTP_HOST" => "www.example.com") controller = Struct.new(:request).new(request) @policy.script_src -> { request.host } @@ -209,7 +215,9 @@ class ContentSecurityPolicyTest < ActiveSupport::TestCase def test_mixed_static_and_dynamic_directives @policy.script_src :self, -> { "foo.com" }, "bar.com" - assert_equal "script-src 'self' foo.com bar.com", @policy.build(Object.new) + request = ActionDispatch::Request.new({}) + controller = Struct.new(:request).new(request) + assert_equal "script-src 'self' foo.com bar.com", @policy.build(controller) end def test_invalid_directive_source @@ -241,6 +249,73 @@ class ContentSecurityPolicyTest < ActiveSupport::TestCase end end +class DefaultContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest + class PolicyController < ActionController::Base + def index + head :ok + end + end + + ROUTES = ActionDispatch::Routing::RouteSet.new + ROUTES.draw do + scope module: "default_content_security_policy_integration_test" do + get "/", to: "policy#index" + end + end + + POLICY = ActionDispatch::ContentSecurityPolicy.new do |p| + p.default_src :self + p.script_src :https + end + + class PolicyConfigMiddleware + def initialize(app) + @app = app + end + + def call(env) + env["action_dispatch.content_security_policy"] = POLICY + env["action_dispatch.content_security_policy_nonce_generator"] = proc { "iyhD0Yc0W+c=" } + env["action_dispatch.content_security_policy_report_only"] = false + env["action_dispatch.show_exceptions"] = false + + @app.call(env) + end + end + + APP = build_app(ROUTES) do |middleware| + middleware.use PolicyConfigMiddleware + middleware.use ActionDispatch::ContentSecurityPolicy::Middleware + end + + def app + APP + end + + def test_adds_nonce_to_script_src_content_security_policy_only_once + get "/" + get "/" + assert_policy "default-src 'self'; script-src https: 'nonce-iyhD0Yc0W+c='" + end + + private + + def assert_policy(expected, report_only: false) + assert_response :success + + if report_only + expected_header = "Content-Security-Policy-Report-Only" + unexpected_header = "Content-Security-Policy" + else + expected_header = "Content-Security-Policy" + unexpected_header = "Content-Security-Policy-Report-Only" + end + + assert_nil response.headers[unexpected_header] + assert_equal expected, response.headers[expected_header] + end +end + class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest class PolicyController < ActionController::Base content_security_policy only: :inline do |p| diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 94cff10fe4..aba778fad6 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -65,8 +65,8 @@ class CookieJarTest < ActiveSupport::TestCase end def test_key_methods - assert !request.cookie_jar.key?(:foo) - assert !request.cookie_jar.has_key?("foo") + assert_not request.cookie_jar.key?(:foo) + assert_not request.cookie_jar.has_key?("foo") request.cookie_jar[:foo] = :bar assert request.cookie_jar.key?(:foo) diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 60acba0616..045567ff83 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -3,6 +3,8 @@ require "abstract_unit" class DebugExceptionsTest < ActionDispatch::IntegrationTest + InterceptedErrorInstance = StandardError.new + class Boomer attr_accessor :closed @@ -36,6 +38,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest raise RuntimeError when %r{/method_not_allowed} raise ActionController::MethodNotAllowed + when %r{/intercepted_error} + raise InterceptedErrorInstance when %r{/unknown_http_method} raise ActionController::UnknownHttpMethod when %r{/not_implemented} @@ -76,9 +80,13 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end end + Interceptor = proc { |request, exception| request.set_header("int", exception) } + BadInterceptor = proc { |request, exception| raise "bad" } RoutesApp = Struct.new(:routes).new(SharedTestRoutes) ProductionApp = ActionDispatch::DebugExceptions.new(Boomer.new(false), RoutesApp) DevelopmentApp = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp) + InterceptedApp = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :default, [Interceptor]) + BadInterceptedApp = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :default, [BadInterceptor]) test "skip diagnosis if not showing detailed exceptions" do @app = ProductionApp @@ -499,4 +507,20 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end end end + + test "invoke interceptors before rendering" do + @app = InterceptedApp + get "/intercepted_error", headers: { "action_dispatch.show_exceptions" => true } + + assert_equal InterceptedErrorInstance, request.get_header("int") + end + + test "bad interceptors doesn't debug exceptions" do + @app = BadInterceptedApp + + get "/puke", headers: { "action_dispatch.show_exceptions" => true } + + assert_response 500 + assert_match(/puke/, body) + end end diff --git a/actionpack/test/dispatch/executor_test.rb b/actionpack/test/dispatch/executor_test.rb index 8eb6450385..5b8be39b6d 100644 --- a/actionpack/test/dispatch/executor_test.rb +++ b/actionpack/test/dispatch/executor_test.rb @@ -81,7 +81,7 @@ class ExecutorTest < ActiveSupport::TestCase running = false body.close - assert !running + assert_not running end def test_complete_callbacks_are_called_on_close @@ -89,7 +89,7 @@ class ExecutorTest < ActiveSupport::TestCase executor.to_complete { completed = true } body = call_and_return_body - assert !completed + assert_not completed body.close assert completed @@ -116,7 +116,7 @@ class ExecutorTest < ActiveSupport::TestCase call_and_return_body.close assert result - assert !defined?(@in_shared_context) # it's not in the test itself + assert_not defined?(@in_shared_context) # it's not in the test itself end private diff --git a/actionpack/test/dispatch/mime_type_test.rb b/actionpack/test/dispatch/mime_type_test.rb index 6167ea46df..fa264417e1 100644 --- a/actionpack/test/dispatch/mime_type_test.rb +++ b/actionpack/test/dispatch/mime_type_test.rb @@ -180,8 +180,8 @@ class MimeTypeTest < ActiveSupport::TestCase assert Mime[:js] =~ "text/javascript" assert Mime[:js] =~ "application/javascript" assert Mime[:js] !~ "text/html" - assert !(Mime[:js] !~ "text/javascript") - assert !(Mime[:js] !~ "application/javascript") + assert_not (Mime[:js] !~ "text/javascript") + assert_not (Mime[:js] !~ "application/javascript") assert Mime[:html] =~ "application/xhtml+xml" end end diff --git a/actionpack/test/dispatch/reloader_test.rb b/actionpack/test/dispatch/reloader_test.rb index e529229fae..edc4cd62a3 100644 --- a/actionpack/test/dispatch/reloader_test.rb +++ b/actionpack/test/dispatch/reloader_test.rb @@ -115,7 +115,7 @@ class ReloaderTest < ActiveSupport::TestCase reloader.to_complete { completed = true } body = call_and_return_body - assert !completed + assert_not completed body.close assert completed @@ -129,7 +129,7 @@ class ReloaderTest < ActiveSupport::TestCase prepared = false body.close - assert !prepared + assert_not prepared end def test_complete_callbacks_are_called_on_exceptions diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 4c8d528507..0f37d074af 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -158,7 +158,7 @@ class ResponseTest < ActiveSupport::TestCase @response.status = c.to_s @response.set_header "Content-Length", "0" _, headers, _ = @response.to_a - assert !headers.has_key?("Content-Length"), "#{c} must not have a Content-Length header field" + assert_not headers.has_key?("Content-Length"), "#{c} must not have a Content-Length header field" end end @@ -177,7 +177,7 @@ class ResponseTest < ActiveSupport::TestCase @response = ActionDispatch::Response.new @response.status = c.to_s _, headers, _ = @response.to_a - assert !headers.has_key?("Content-Type"), "#{c} should not have Content-Type header" + assert_not headers.has_key?("Content-Type"), "#{c} should not have Content-Type header" end [200, 302, 404, 500].each do |c| @@ -191,7 +191,7 @@ class ResponseTest < ActiveSupport::TestCase test "does not include Status header" do @response.status = "200 OK" _, headers, _ = @response.to_a - assert !headers.has_key?("Status") + assert_not headers.has_key?("Status") end test "response code" do diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index fe314e26b1..5efbe5b553 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3153,7 +3153,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest after = has_named_route?(:hello) end - assert !before, "expected to not have named route :hello before route definition" + assert_not before, "expected to not have named route :hello before route definition" assert after, "expected to have named route :hello after route definition" end @@ -3166,7 +3166,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end - assert !respond_to?(:routes_no_collision_path) + assert_not respond_to?(:routes_no_collision_path) end def test_controller_name_with_leading_slash_raise_error diff --git a/actionpack/test/journey/route/definition/scanner_test.rb b/actionpack/test/journey/route/definition/scanner_test.rb index 070886c7df..bcbe4388c3 100644 --- a/actionpack/test/journey/route/definition/scanner_test.rb +++ b/actionpack/test/journey/route/definition/scanner_test.rb @@ -10,61 +10,64 @@ module ActionDispatch @scanner = Scanner.new end - # /page/:id(/:action)(.:format) - def test_tokens - [ - ["/", [[:SLASH, "/"]]], - ["*omg", [[:STAR, "*omg"]]], - ["/page", [[:SLASH, "/"], [:LITERAL, "page"]]], - ["/page!", [[:SLASH, "/"], [:LITERAL, "page!"]]], - ["/page$", [[:SLASH, "/"], [:LITERAL, "page$"]]], - ["/page&", [[:SLASH, "/"], [:LITERAL, "page&"]]], - ["/page'", [[:SLASH, "/"], [:LITERAL, "page'"]]], - ["/page*", [[:SLASH, "/"], [:LITERAL, "page*"]]], - ["/page+", [[:SLASH, "/"], [:LITERAL, "page+"]]], - ["/page,", [[:SLASH, "/"], [:LITERAL, "page,"]]], - ["/page;", [[:SLASH, "/"], [:LITERAL, "page;"]]], - ["/page=", [[:SLASH, "/"], [:LITERAL, "page="]]], - ["/page@", [[:SLASH, "/"], [:LITERAL, "page@"]]], - ['/page\:', [[:SLASH, "/"], [:LITERAL, "page:"]]], - ['/page\(', [[:SLASH, "/"], [:LITERAL, "page("]]], - ['/page\)', [[:SLASH, "/"], [:LITERAL, "page)"]]], - ["/~page", [[:SLASH, "/"], [:LITERAL, "~page"]]], - ["/pa-ge", [[:SLASH, "/"], [:LITERAL, "pa-ge"]]], - ["/:page", [[:SLASH, "/"], [:SYMBOL, ":page"]]], - ["/(:page)", [ + CASES = [ + ["/", [[:SLASH, "/"]]], + ["*omg", [[:STAR, "*omg"]]], + ["/page", [[:SLASH, "/"], [:LITERAL, "page"]]], + ["/page!", [[:SLASH, "/"], [:LITERAL, "page!"]]], + ["/page$", [[:SLASH, "/"], [:LITERAL, "page$"]]], + ["/page&", [[:SLASH, "/"], [:LITERAL, "page&"]]], + ["/page'", [[:SLASH, "/"], [:LITERAL, "page'"]]], + ["/page*", [[:SLASH, "/"], [:LITERAL, "page*"]]], + ["/page+", [[:SLASH, "/"], [:LITERAL, "page+"]]], + ["/page,", [[:SLASH, "/"], [:LITERAL, "page,"]]], + ["/page;", [[:SLASH, "/"], [:LITERAL, "page;"]]], + ["/page=", [[:SLASH, "/"], [:LITERAL, "page="]]], + ["/page@", [[:SLASH, "/"], [:LITERAL, "page@"]]], + ['/page\:', [[:SLASH, "/"], [:LITERAL, "page:"]]], + ['/page\(', [[:SLASH, "/"], [:LITERAL, "page("]]], + ['/page\)', [[:SLASH, "/"], [:LITERAL, "page)"]]], + ["/~page", [[:SLASH, "/"], [:LITERAL, "~page"]]], + ["/pa-ge", [[:SLASH, "/"], [:LITERAL, "pa-ge"]]], + ["/:page", [[:SLASH, "/"], [:SYMBOL, ":page"]]], + ["/(:page)", [ + [:SLASH, "/"], + [:LPAREN, "("], + [:SYMBOL, ":page"], + [:RPAREN, ")"], + ]], + ["(/:action)", [ + [:LPAREN, "("], [:SLASH, "/"], + [:SYMBOL, ":action"], + [:RPAREN, ")"], + ]], + ["(())", [[:LPAREN, "("], + [:LPAREN, "("], [:RPAREN, ")"], [:RPAREN, ")"]]], + ["(.:format)", [ [:LPAREN, "("], - [:SYMBOL, ":page"], + [:DOT, "."], + [:SYMBOL, ":format"], [:RPAREN, ")"], ]], - ["(/:action)", [ - [:LPAREN, "("], - [:SLASH, "/"], - [:SYMBOL, ":action"], - [:RPAREN, ")"], - ]], - ["(())", [[:LPAREN, "("], - [:LPAREN, "("], [:RPAREN, ")"], [:RPAREN, ")"]]], - ["(.:format)", [ - [:LPAREN, "("], - [:DOT, "."], - [:SYMBOL, ":format"], - [:RPAREN, ")"], - ]], - ].each do |str, expected| - @scanner.scan_setup str - assert_tokens expected, @scanner + ] + + CASES.each do |pattern, expected_tokens| + test "Scanning `#{pattern}`" do + @scanner.scan_setup pattern + assert_tokens expected_tokens, @scanner, pattern end end - def assert_tokens(tokens, scanner) - toks = [] - while tok = scanner.next_token - toks << tok + private + + def assert_tokens(expected_tokens, scanner, pattern) + actual_tokens = [] + while token = scanner.next_token + actual_tokens << token + end + assert_equal expected_tokens, actual_tokens, "Wrong tokens for `#{pattern}`" end - assert_equal tokens, toks - end end end end |